From acfd5b37e7cad5e28ac213cc0fcc8d087138cd67 Mon Sep 17 00:00:00 2001 From: Maks Oleksyuk Date: Sun, 19 Apr 2026 15:23:05 +0300 Subject: [PATCH 1/2] fix: skip rate limit for cache hits --- src/Traits/HasRateLimits.php | 24 +++++ tests/Feature/CacheHitRateLimitTest.php | 125 ++++++++++++++++++++++++ 2 files changed, 149 insertions(+) create mode 100644 tests/Feature/CacheHitRateLimitTest.php diff --git a/src/Traits/HasRateLimits.php b/src/Traits/HasRateLimits.php index eba3f7d..658fc3f 100644 --- a/src/Traits/HasRateLimits.php +++ b/src/Traits/HasRateLimits.php @@ -9,6 +9,7 @@ use Saloon\Http\Response; use Saloon\Enums\PipeOrder; use Saloon\Http\PendingRequest; +use Saloon\Http\Faking\MockResponse; use Saloon\RateLimitPlugin\Limit; use Saloon\RateLimitPlugin\Helpers\LimitHelper; use Saloon\RateLimitPlugin\Contracts\RateLimitStore; @@ -47,6 +48,15 @@ public function bootHasRateLimits(PendingRequest $pendingRequest): void // the request from being processed. $pendingRequest->middleware()->onRequest(function (PendingRequest $pendingRequest): void { + // Skip the pre-request limit check for cache hits. Cache middleware registers + // with PipeOrder::FIRST, so hasFakeResponse() is already true here (null order). + // Plain FakeResponse = cache hit; MockResponse = mock client (should still check). + $fakeResponse = $pendingRequest->getFakeResponse(); + + if ($fakeResponse !== null && ! $fakeResponse instanceof MockResponse) { + return; + } + $limit = $this->getExceededLimit(); if ($limit instanceof Limit) { @@ -55,6 +65,20 @@ public function bootHasRateLimits(PendingRequest $pendingRequest): void }); $pendingRequest->middleware()->onResponse(function (Response $response): Response { + // Skip rate limit counting for cache hits — they never reach the API. + // + // We cannot use $response->isCached() here because the cache plugin registers + // setCached(true) via onResponse(PipeOrder::FIRST) during the request middleware + // phase, which places it *after* this handler in the pipeline (same PipeOrder, + // later insertion). So isCached() is always false at this point. + // + // Instead: cache hits produce a plain FakeResponse, while MockClient produces a + // MockResponse (extends FakeResponse) and sets isMocked() before the pipeline runs. + // Checking hasFakeResponse() && !isMocked() isolates cache hits only. + if ($response->getPendingRequest()->hasFakeResponse() && ! $response->isMocked()) { + return $response; + } + $limitThatWasExceeded = null; $store = $this->rateLimitStore(); diff --git a/tests/Feature/CacheHitRateLimitTest.php b/tests/Feature/CacheHitRateLimitTest.php new file mode 100644 index 0000000..2e4bb31 --- /dev/null +++ b/tests/Feature/CacheHitRateLimitTest.php @@ -0,0 +1,125 @@ +middleware()->onRequest( + callable: function (PendingRequest $pendingRequest): FakeResponse { + return new FakeResponse(['cached' => true], 200); + }, + order: PipeOrder::FIRST, + ); +} + +test('cache hits do not count against the rate limit', function () { + $store = new MemoryStore; + + $connector = new TestConnector($store, [ + Limit::allow(3)->everyMinute(), + ]); + + withCacheMiddleware($connector); + + // Send 10 requests — all served from "cache". Limit of 3 should never be reached. + for ($i = 0; $i < 10; $i++) { + $connector->send(new UserRequest); + } + + expect($connector->hasReachedRateLimit())->toBeFalse(); +}); + +test('cache hits do not increment the hit counter in the store', function () { + $store = new MemoryStore; + + $connector = new TestConnector($store, [ + Limit::allow(3)->everyMinute(), + ]); + + withCacheMiddleware($connector); + + $connector->send(new UserRequest); + $connector->send(new UserRequest); + $connector->send(new UserRequest); + + // No real hits recorded — store stays empty + expect($store->getStore())->toBeEmpty(); +}); + +test('real api responses still count against the rate limit', function () { + $store = new MemoryStore; + + $connector = new TestConnector($store, [ + Limit::allow(3)->everyMinute(), + ]); + + $connector->withMockClient(new MockClient([ + UserRequest::class => new MockResponse(['name' => 'Sam'], 200), + ])); + + $connector->send(new UserRequest); + $connector->send(new UserRequest); + $connector->send(new UserRequest); + + expect($connector->hasReachedRateLimit())->toBeTrue(); +}); + +test('cache hits do not throw when the rate limit is already exhausted by real requests', function () { + $store = new MemoryStore; + + $connector = new TestConnector($store, [ + Limit::allow(3)->everyMinute(), + ]); + + // Exhaust the limit with 3 real (mock) requests + $connector->withMockClient(new MockClient([ + UserRequest::class => new MockResponse(['name' => 'Sam'], 200), + ])); + + $connector->send(new UserRequest); + $connector->send(new UserRequest); + $connector->send(new UserRequest); + + expect($connector->hasReachedRateLimit())->toBeTrue(); + + // Cache middleware must use PipeOrder::FIRST so it fires before the rate limit + // onRequest check — mirroring how the real cache plugin registers its middleware. + withCacheMiddleware($connector); + + expect(fn () => $connector->send(new UserRequest)) + ->not->toThrow(\Saloon\RateLimitPlugin\Exceptions\RateLimitReachedException::class); +}); + +test('only real requests count toward the limit when mixed with cache hits', function () { + $store = new MemoryStore; + + // 1 real request via mock client + $connector = new TestConnector($store, [Limit::allow(3)->everyMinute()]); + $connector->withMockClient(new MockClient([ + UserRequest::class => new MockResponse(['name' => 'Sam'], 200), + ])); + $connector->send(new UserRequest); // hits = 1 + + // 5 cache hits via same store — should not add to the count + withCacheMiddleware($connector); + for ($i = 0; $i < 5; $i++) { + $connector->send(new UserRequest); + } + + // Only 1 real hit, limit of 3 not reached + expect($connector->hasReachedRateLimit())->toBeFalse(); +}); From 8046789aac4d39d11c57e1f88301a1d80a1b43d5 Mon Sep 17 00:00:00 2001 From: maks-oleksyuk <90793591+maks-oleksyuk@users.noreply.github.com> Date: Sun, 19 Apr 2026 12:24:29 +0000 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=AA=84=20Code=20Style=20Fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/Traits/HasRateLimits.php | 2 +- tests/Feature/CacheHitRateLimitTest.php | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Traits/HasRateLimits.php b/src/Traits/HasRateLimits.php index 658fc3f..3dbcd9d 100644 --- a/src/Traits/HasRateLimits.php +++ b/src/Traits/HasRateLimits.php @@ -9,8 +9,8 @@ use Saloon\Http\Response; use Saloon\Enums\PipeOrder; use Saloon\Http\PendingRequest; -use Saloon\Http\Faking\MockResponse; use Saloon\RateLimitPlugin\Limit; +use Saloon\Http\Faking\MockResponse; use Saloon\RateLimitPlugin\Helpers\LimitHelper; use Saloon\RateLimitPlugin\Contracts\RateLimitStore; use Saloon\RateLimitPlugin\Helpers\RetryAfterHelper; diff --git a/tests/Feature/CacheHitRateLimitTest.php b/tests/Feature/CacheHitRateLimitTest.php index 2e4bb31..9767c24 100644 --- a/tests/Feature/CacheHitRateLimitTest.php +++ b/tests/Feature/CacheHitRateLimitTest.php @@ -4,13 +4,13 @@ use Saloon\Enums\PipeOrder; use Saloon\Http\PendingRequest; -use Saloon\Http\Faking\FakeResponse; +use Saloon\RateLimitPlugin\Limit; use Saloon\Http\Faking\MockClient; +use Saloon\Http\Faking\FakeResponse; use Saloon\Http\Faking\MockResponse; -use Saloon\RateLimitPlugin\Limit; use Saloon\RateLimitPlugin\Stores\MemoryStore; -use Saloon\RateLimitPlugin\Tests\Fixtures\Connectors\TestConnector; use Saloon\RateLimitPlugin\Tests\Fixtures\Requests\UserRequest; +use Saloon\RateLimitPlugin\Tests\Fixtures\Connectors\TestConnector; // Simulate what saloonphp/cache-plugin does: a request middleware with PipeOrder::FIRST // returns a plain FakeResponse (not MockResponse). This mirrors CacheMiddleware exactly —