diff --git a/src/Phaseolies/Cache/CacheStore.php b/src/Phaseolies/Cache/CacheStore.php index 246093cb..3e3d0e8a 100644 --- a/src/Phaseolies/Cache/CacheStore.php +++ b/src/Phaseolies/Cache/CacheStore.php @@ -2,18 +2,19 @@ namespace Phaseolies\Cache; +use DateTime; use Symfony\Component\Cache\Adapter\AdapterInterface; -use Psr\SimpleCache\CacheInterface; use Phaseolies\Cache\Lock\AtomicLock; +use Symfony\Contracts\Cache\CacheInterface as ContractsCacheInterface; -class CacheStore implements CacheInterface +class CacheStore implements IncrementableCacheInterface { /** * The cache adapter instance. * * @var AdapterInterface */ - protected $adapter; + protected AdapterInterface $adapter; /** * The cache prefix @@ -32,7 +33,7 @@ class CacheStore implements CacheInterface public function __construct(AdapterInterface $adapter, ?string $prefix = null) { $this->adapter = $adapter; - $this->prefix = $prefix ?? config('caching.prefix'); + $this->prefix = (string) ($prefix ?? config('caching.prefix')); } /** @@ -46,6 +47,19 @@ protected function prefixedKey(string $key): string return $this->prefix . $key; } + /** + * Validate a cache key and return the prefixed adapter key. + * + * @param mixed $key + * @return string + */ + protected function prefixedValidatedKey($key): string + { + $key = $this->normalizeKey($key); + + return $this->prefixedKey($key); + } + /** * Get the cache data by key * @@ -56,8 +70,7 @@ protected function prefixedKey(string $key): string #[\Override] public function get($key, $default = null): mixed { - $key = $this->prefixedKey($key); - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); $item = $this->adapter->getItem($key); return $item->isHit() ? $item->get() : $default; @@ -74,8 +87,7 @@ public function get($key, $default = null): mixed #[\Override] public function set($key, $value, $ttl = null): bool { - $key = $this->prefixedKey($key); - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); $item = $this->adapter->getItem($key); $item->set($value); @@ -96,9 +108,7 @@ public function set($key, $value, $ttl = null): bool #[\Override] public function delete($key): bool { - $key = $this->prefixedKey($key); - - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); return $this->adapter->deleteItem($key); } @@ -111,7 +121,7 @@ public function delete($key): bool #[\Override] public function clear(): bool { - return $this->adapter->clear(); + return $this->adapter->clear($this->prefix); } /** @@ -124,14 +134,14 @@ public function clear(): bool #[\Override] public function getMultiple($keys, $default = null): iterable { - if (!is_iterable($keys)) { - throw new \InvalidArgumentException('Keys must be an array or traversable'); - } + $keys = $this->normalizeKeyList($keys); + $prefixedKeys = []; - $prefixedKeys = array_map(fn($key) => $this->prefixedKey($key), (array)$keys); - $validatedKeys = $this->validateKeys($prefixedKeys); + foreach ($keys as $key) { + $prefixedKeys[] = $this->prefixedValidatedKey($key); + } - $items = $this->adapter->getItems($validatedKeys); + $items = $this->adapter->getItems($prefixedKeys); $results = []; foreach ($items as $key => $item) { @@ -139,6 +149,10 @@ public function getMultiple($keys, $default = null): iterable $results[$originalKey] = $item->isHit() ? $item->get() : $default; } + foreach ($keys as $key) { + $results[$key] ??= $default; + } + return $results; } @@ -152,16 +166,13 @@ public function getMultiple($keys, $default = null): iterable #[\Override] public function setMultiple($values, $ttl = null): bool { - if (!is_iterable($values)) { - throw new \InvalidArgumentException('Values must be an array or traversable'); - } + $values = $this->normalizeValueMap($values); $success = true; $ttl = $this->convertTtlToSeconds($ttl); foreach ($values as $key => $value) { - $prefixedKey = $this->prefixedKey($key); - $this->validateKey($prefixedKey); + $prefixedKey = $this->prefixedValidatedKey($key); $item = $this->adapter->getItem($prefixedKey); $item->set($value); @@ -184,14 +195,14 @@ public function setMultiple($values, $ttl = null): bool #[\Override] public function deleteMultiple($keys): bool { - if (!is_iterable($keys)) { - throw new \InvalidArgumentException('Keys must be an array or traversable'); - } + $keys = $this->normalizeKeyList($keys); + $prefixedKeys = []; - $prefixedKeys = array_map(fn($key) => $this->prefixedKey($key), (array)$keys); - $validatedKeys = $this->validateKeys($prefixedKeys); + foreach ($keys as $key) { + $prefixedKeys[] = $this->prefixedValidatedKey($key); + } - return $this->adapter->deleteItems($validatedKeys); + return $this->adapter->deleteItems($prefixedKeys); } /** @@ -203,9 +214,7 @@ public function deleteMultiple($keys): bool #[\Override] public function has($key): bool { - $key = $this->prefixedKey($key); - - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); return $this->adapter->hasItem($key); } @@ -219,25 +228,25 @@ public function has($key): bool */ public function increment($key, $value = 1): int|bool { - $key = $this->prefixedKey($key); - $this->validateKey($key); - $item = $this->adapter->getItem($key); + $key = $this->prefixedValidatedKey($key); - if (!$item->isHit()) { - return false; - } + return $this->withKeyLock($key, function () use ($key, $value) { + $item = $this->adapter->getItem($key); - $current = (int)$item->get(); - $new = $current + $value; - $item->set($new); + if (!$item->isHit()) { + return false; + } - // Preserve existing expiration - if ($item->getMetadata()['expiry'] ?? null) { - $item->expiresAt(\DateTime::createFromFormat('U', $item->getMetadata()['expiry'])); - } + $current = (int) $item->get(); + $new = $current + $value; + $item->set($new); + + if ($item->getMetadata()['expiry'] ?? null) { + $item->expiresAt(DateTime::createFromFormat('U', (string) $item->getMetadata()['expiry'])); + } - $this->adapter->save($item); - return $new; + return $this->adapter->save($item) ? $new : false; + }); } /** @@ -249,25 +258,25 @@ public function increment($key, $value = 1): int|bool */ public function decrement($key, $value = 1): int|bool { - $key = $this->prefixedKey($key); - $this->validateKey($key); - $item = $this->adapter->getItem($key); + $key = $this->prefixedValidatedKey($key); - if (!$item->isHit()) { - return false; - } + return $this->withKeyLock($key, function () use ($key, $value) { + $item = $this->adapter->getItem($key); + + if (!$item->isHit()) { + return false; + } - $current = (int)$item->get(); - $new = $current - $value; - $item->set($new); + $current = (int) $item->get(); + $new = $current - $value; + $item->set($new); - // Preserve existing expiration - if ($item->getMetadata()['expiry'] ?? null) { - $item->expiresAt(\DateTime::createFromFormat('U', $item->getMetadata()['expiry'])); - } + if ($item->getMetadata()['expiry'] ?? null) { + $item->expiresAt(DateTime::createFromFormat('U', (string) $item->getMetadata()['expiry'])); + } - $this->adapter->save($item); - return $new; + return $this->adapter->save($item) ? $new : false; + }); } /** @@ -280,21 +289,39 @@ public function decrement($key, $value = 1): int|bool */ public function add($key, $value, $ttl = null): bool { - $key = $this->prefixedKey($key); - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); + $seconds = $this->convertTtlToSeconds($ttl); - if ($this->adapter->hasItem($key)) { - return false; - } + if ($this->adapter instanceof ContractsCacheInterface) { + $created = false; - $item = $this->adapter->getItem($key); - $item->set($value); + $this->adapter->get($key, function ($item) use (&$created, $value, $seconds) { + $created = true; - if ($ttl !== null) { - $item->expiresAfter($this->convertTtlToSeconds($ttl)); + if ($seconds !== null) { + $item->expiresAfter($seconds); + } + + return $value; + }); + + return $created; } - return $this->adapter->save($item); + return $this->withKeyLock($key, function () use ($key, $value, $seconds) { + if ($this->adapter->hasItem($key)) { + return false; + } + + $item = $this->adapter->getItem($key); + $item->set($value); + + if ($seconds !== null) { + $item->expiresAfter($seconds); + } + + return $this->adapter->save($item); + }); } /** @@ -306,8 +333,7 @@ public function add($key, $value, $ttl = null): bool */ public function forever($key, $value): bool { - $key = $this->prefixedKey($key); - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); $item = $this->adapter->getItem($key); $item->set($value); @@ -324,8 +350,7 @@ public function forever($key, $value): bool */ public function forget($key): bool { - $key = $this->prefixedKey($key); - $this->validateKey($key); + $key = $this->prefixedValidatedKey($key); if (!$this->adapter->hasItem($key)) { return false; @@ -350,7 +375,11 @@ protected function validateKey($key): void )); } - if (preg_match('/[{}()\/\\\\@]/', $key)) { + if ($key === '') { + throw new \InvalidArgumentException('Cache key must not be empty'); + } + + if (preg_match('/[{}()\/\\\\@\:]/', $key)) { throw new \InvalidArgumentException(sprintf( 'Invalid key: "%s". The key contains one or more characters reserved for future extension', $key @@ -394,6 +423,119 @@ protected function convertTtlToSeconds($ttl): ?int return (int) $ttl; } + /** + * Normalize an individual cache key. + * + * @param mixed $key + * @return string + */ + protected function normalizeKey($key): string + { + $this->validateKey($key); + + return $key; + } + + /** + * Normalize an iterable of cache keys into a sequential array of strings. + * + * @param mixed $keys + * @return array + */ + protected function normalizeKeyList($keys): array + { + if ($keys instanceof \Traversable) { + $keys = iterator_to_array($keys, false); + } elseif (!is_array($keys)) { + throw new \InvalidArgumentException('Keys must be an array or traversable'); + } + + $normalized = []; + + foreach ($keys as $key) { + $normalized[] = $this->normalizeKey($key); + } + + return $normalized; + } + + /** + * Normalize an iterable of key/value pairs into an array. + * + * @param mixed $values + * @return array + */ + protected function normalizeValueMap($values): array + { + if ($values instanceof \Traversable) { + $values = iterator_to_array($values, true); + } elseif (!is_array($values)) { + throw new \InvalidArgumentException('Values must be an array or traversable'); + } + + $normalized = []; + + foreach ($values as $key => $value) { + $normalized[$this->normalizeKey($key)] = $value; + } + + return $normalized; + } + + /** + * Execute a callback while holding a process-local lock for the key. + * + * @template T + * @param string $key + * @param \Closure(): T $callback + * @return T + */ + protected function withKeyLock(string $key, \Closure $callback): mixed + { + $directory = $this->lockDirectory(); + + if (!is_dir($directory)) { + mkdir($directory, 0777, true); + } + + $path = $directory . DIRECTORY_SEPARATOR . sha1($key) . '.lock'; + $handle = fopen($path, 'c+'); + + if ($handle === false) { + throw new \RuntimeException(sprintf('Unable to open cache lock file: %s', $path)); + } + + try { + if (!flock($handle, LOCK_EX)) { + throw new \RuntimeException(sprintf('Unable to acquire cache lock: %s', $path)); + } + + return $callback(); + } finally { + flock($handle, LOCK_UN); + fclose($handle); + } + } + + /** + * Resolve the directory used for process-local cache locks. + * + * @return string + */ + protected function lockDirectory(): string + { + if (function_exists('storage_path')) { + try { + return storage_path('framework/cache/locks'); + } catch (\Throwable) { + // Fall back to the system temp directory when the application container + // is not fully bootstrapped, e.g. in isolated unit tests. + } + } + + return sys_get_temp_dir() . DIRECTORY_SEPARATOR . 'doppar-cache-locks'; + } + /** * Get the current adapter * @@ -414,10 +556,8 @@ public function getAdapter(): AdapterInterface */ public function stash(string $key, $ttl, \Closure $callback): mixed { - $value = $this->get($key); - - if (!is_null($value)) { - return $value; + if ($this->has($key)) { + return $this->get($key); } $value = $callback(); @@ -436,10 +576,8 @@ public function stash(string $key, $ttl, \Closure $callback): mixed */ public function stashForever(string $key, \Closure $callback): mixed { - $value = $this->get($key); - - if (!is_null($value)) { - return $value; + if ($this->has($key)) { + return $this->get($key); } $value = $callback(); diff --git a/src/Phaseolies/Cache/IncrementableCacheInterface.php b/src/Phaseolies/Cache/IncrementableCacheInterface.php new file mode 100644 index 00000000..3184acb6 --- /dev/null +++ b/src/Phaseolies/Cache/IncrementableCacheInterface.php @@ -0,0 +1,27 @@ +cache = $cache; } @@ -39,16 +38,21 @@ public function attempt(string $key, int $maxAttempts, int $decaySeconds): RateL { $timerKey = $key . '_timer'; $now = time(); + $resetAt = $now + $decaySeconds; try { - if (!$this->cache->has($key)) { - $this->cache->set($key, 1, $decaySeconds); - $this->cache->set($timerKey, $now + $decaySeconds, $decaySeconds); + if ($this->cache->add($key, 1, $decaySeconds)) { + $this->cache->add($timerKey, $resetAt, $decaySeconds); $hits = 1; } else { $hits = $this->cache->increment($key); - if ($hits <= $maxAttempts) { - $this->cache->set($timerKey, $now + $decaySeconds, $decaySeconds); + + if ($hits === false) { + $this->cache->set($key, 1, $decaySeconds); + $this->cache->set($timerKey, $resetAt, $decaySeconds); + $hits = 1; + } elseif ($hits <= $maxAttempts) { + $this->cache->set($timerKey, $resetAt, $decaySeconds); } } @@ -127,17 +131,25 @@ public function hit(string $key, int $decaySeconds): int { $timerKey = $key . '_timer'; $now = time(); + $resetAt = $now + $decaySeconds; try { - if (!$this->cache->has($key)) { + if ($this->cache->add($key, 1, $decaySeconds)) { + $this->cache->add($timerKey, $resetAt, $decaySeconds); + return 1; + } + + $hits = $this->cache->increment($key); + + if ($hits === false) { $this->cache->set($key, 1, $decaySeconds); - $this->cache->set($timerKey, $now + $decaySeconds, $decaySeconds); + $this->cache->set($timerKey, $resetAt, $decaySeconds); return 1; - } else { - $hits = $this->cache->increment($key); - $this->cache->set($timerKey, $now + $decaySeconds, $decaySeconds); - return $hits; } + + $this->cache->set($timerKey, $resetAt, $decaySeconds); + + return $hits; } catch (InvalidArgumentException $e) { throw $e; } diff --git a/src/Phaseolies/Helpers/helpers.php b/src/Phaseolies/Helpers/helpers.php index ff264d83..5da7c743 100644 --- a/src/Phaseolies/Helpers/helpers.php +++ b/src/Phaseolies/Helpers/helpers.php @@ -343,6 +343,30 @@ function config(string|array $key, ?string $default = null): mixed } } +if (!function_exists('cache')) { + /** + * Get the cache store instance, retrieve an item, or store multiple items + * + * @param string|array|null $key + * @param mixed $default + * @return mixed + */ + function cache(string|array|null $key = null, mixed $default = null): mixed + { + $store = app('cache'); + + if (is_null($key)) { + return $store; + } + + if (is_array($key)) { + return $store->setMultiple($key, $default); + } + + return $store->get($key, $default); + } +} + if (!function_exists('is_auth')) { /** * Check if the user is authenticated. diff --git a/src/Phaseolies/Providers/CacheServiceProvider.php b/src/Phaseolies/Providers/CacheServiceProvider.php index a464729f..ce17c845 100644 --- a/src/Phaseolies/Providers/CacheServiceProvider.php +++ b/src/Phaseolies/Providers/CacheServiceProvider.php @@ -9,6 +9,7 @@ use Psr\SimpleCache\CacheInterface; use Phaseolies\Providers\ServiceProvider; use Phaseolies\Cache\CacheStore; +use Phaseolies\Cache\IncrementableCacheInterface; class CacheServiceProvider extends ServiceProvider { @@ -26,6 +27,8 @@ public function register(): void { $adapter = $this->createAdapter(config('caching.default', 'file')); $cacheStore = new CacheStore($adapter, config('caching.prefix')); + $this->app->singleton(CacheStore::class, fn() => $cacheStore); + $this->app->singleton(IncrementableCacheInterface::class, fn() => $cacheStore); $this->app->singleton(CacheInterface::class, fn() => $cacheStore); $this->app->singleton('cache', fn() => $cacheStore); } diff --git a/src/Phaseolies/Providers/RateLimiterServiceProvider.php b/src/Phaseolies/Providers/RateLimiterServiceProvider.php index 30f02def..bc5d206c 100644 --- a/src/Phaseolies/Providers/RateLimiterServiceProvider.php +++ b/src/Phaseolies/Providers/RateLimiterServiceProvider.php @@ -2,8 +2,8 @@ namespace Phaseolies\Providers; -use Psr\SimpleCache\CacheInterface; use Phaseolies\Providers\ServiceProvider; +use Phaseolies\Cache\IncrementableCacheInterface; use Phaseolies\Cache\RateLimiter; class RateLimiterServiceProvider extends ServiceProvider @@ -16,7 +16,7 @@ class RateLimiterServiceProvider extends ServiceProvider public function register(): void { $this->app->singleton(RateLimiter::class, function ($app) { - return new RateLimiter($app->make(CacheInterface::class)); + return new RateLimiter($app->make(IncrementableCacheInterface::class)); }); } diff --git a/tests/CacheStoreTest.php b/tests/CacheStoreTest.php index 69323e31..b9dfe15c 100644 --- a/tests/CacheStoreTest.php +++ b/tests/CacheStoreTest.php @@ -2,6 +2,8 @@ namespace Tests\Unit; +use ArrayIterator; +use Phaseolies\DI\Container; use Symfony\Component\Cache\Adapter\ArrayAdapter; use Phaseolies\Cache\Lock\AtomicLock; use Phaseolies\Cache\CacheStore; @@ -17,6 +19,14 @@ protected function setUp(): void { $this->adapter = new ArrayAdapter(); $this->cache = new CacheStore($this->adapter, 'test_'); + $container = new Container(); + $container->instance('cache', $this->cache); + Container::setInstance($container); + } + + protected function tearDown(): void + { + Container::forgetInstance(); } public function testGetReturnsDefaultForMissingKey() @@ -25,6 +35,30 @@ public function testGetReturnsDefaultForMissingKey() $this->assertEquals('default_value', $result); } + public function testCacheHelperReturnsBoundStore() + { + $this->assertSame($this->cache, cache()); + } + + public function testCacheHelperGetsValues() + { + $this->cache->set('helper_key', 'helper_value'); + + $this->assertSame('helper_value', cache('helper_key')); + $this->assertSame('fallback', cache('missing_helper_key', 'fallback')); + } + + public function testCacheHelperStoresMultipleValues() + { + $this->assertTrue(cache([ + 'helper_multi_1' => 'value1', + 'helper_multi_2' => 'value2', + ], 60)); + + $this->assertSame('value1', $this->cache->get('helper_multi_1')); + $this->assertSame('value2', $this->cache->get('helper_multi_2')); + } + public function testSetAndGet() { $this->assertTrue($this->cache->set('test_key', 'test_value')); @@ -60,6 +94,20 @@ public function testClear() $this->assertFalse($this->cache->has('key2')); } + public function testClearOnlyRemovesItemsForCurrentPrefix() + { + $sharedAdapter = new ArrayAdapter(); + $primary = new CacheStore($sharedAdapter, 'alpha_'); + $secondary = new CacheStore($sharedAdapter, 'beta_'); + + $primary->set('shared', 'alpha'); + $secondary->set('shared', 'beta'); + + $this->assertTrue($primary->clear()); + $this->assertNull($primary->get('shared')); + $this->assertSame('beta', $secondary->get('shared')); + } + public function testGetMultiple() { $this->cache->set('key1', 'value1'); @@ -73,6 +121,20 @@ public function testGetMultiple() ], $result); } + public function testGetMultipleAcceptsTraversableKeys() + { + $this->cache->set('key1', 'value1'); + $this->cache->set('key2', 'value2'); + + $result = $this->cache->getMultiple(new ArrayIterator(['key1', 'key2', 'key3']), 'default'); + + $this->assertEquals([ + 'key1' => 'value1', + 'key2' => 'value2', + 'key3' => 'default', + ], $result); + } + public function testSetMultiple() { $values = [ @@ -85,6 +147,18 @@ public function testSetMultiple() $this->assertEquals('value2', $this->cache->get('multi2')); } + public function testSetMultipleAcceptsTraversableValues() + { + $values = new ArrayIterator([ + 'multi1' => 'value1', + 'multi2' => 'value2', + ]); + + $this->assertTrue($this->cache->setMultiple($values)); + $this->assertEquals('value1', $this->cache->get('multi1')); + $this->assertEquals('value2', $this->cache->get('multi2')); + } + public function testDeleteMultiple() { $this->cache->set('key1', 'value1'); @@ -97,6 +171,18 @@ public function testDeleteMultiple() $this->assertTrue($this->cache->has('key3')); } + public function testDeleteMultipleAcceptsTraversableKeys() + { + $this->cache->set('key1', 'value1'); + $this->cache->set('key2', 'value2'); + $this->cache->set('key3', 'value3'); + + $this->assertTrue($this->cache->deleteMultiple(new ArrayIterator(['key1', 'key2']))); + $this->assertFalse($this->cache->has('key1')); + $this->assertFalse($this->cache->has('key2')); + $this->assertTrue($this->cache->has('key3')); + } + public function testHas() { $this->assertFalse($this->cache->has('some_key')); @@ -200,6 +286,48 @@ public function testStashForever() $this->assertEquals('forever_value', $this->cache->get('forever_stash')); } + public function testStashDoesNotRecomputeCachedNullValues() + { + $calls = 0; + + $first = $this->cache->stash('nullable', 60, function () use (&$calls) { + $calls++; + + return null; + }); + + $second = $this->cache->stash('nullable', 60, function () use (&$calls) { + $calls++; + + return 'recomputed'; + }); + + $this->assertNull($first); + $this->assertNull($second); + $this->assertSame(1, $calls); + } + + public function testStashForeverDoesNotRecomputeCachedNullValues() + { + $calls = 0; + + $first = $this->cache->stashForever('nullable_forever', function () use (&$calls) { + $calls++; + + return null; + }); + + $second = $this->cache->stashForever('nullable_forever', function () use (&$calls) { + $calls++; + + return 'recomputed'; + }); + + $this->assertNull($first); + $this->assertNull($second); + $this->assertSame(1, $calls); + } + public function testStashWhen() { $callback = function () { @@ -223,6 +351,18 @@ public function testInvalidKeyThrowsException() $this->cache->get('invalid/key'); } + public function testEmptyKeyThrowsException() + { + $this->expectException(\InvalidArgumentException::class); + $this->cache->get(''); + } + + public function testReservedColonKeyThrowsException() + { + $this->expectException(\InvalidArgumentException::class); + $this->cache->get('invalid:key'); + } + public function testPrefixIsApplied() { $this->cache->set('prefixed', 'value'); @@ -509,11 +649,12 @@ public function testAtomicLockBlockWaitsUntilAvailable() // Release the first lock after a short delay in another thread simulation // Since we can’t sleep in test too long, we manually simulate expiry sleep(1); - $this->adapter->deleteItem('lock_test'); + $this->adapter->deleteItem('test_lock_test'); $this->assertTrue($lock2->block(2)); - $this->assertFalse($this->adapter->hasItem('lock_test')); + $this->assertTrue($this->adapter->hasItem('test_lock_test')); $this->assertTrue($lock2->release()); + $this->assertFalse($this->adapter->hasItem('test_lock_test')); } public function testLockOwnerGeneration() diff --git a/tests/Requests/RateLimiterTest.php b/tests/Requests/RateLimiterTest.php index 04c908b1..adb958f2 100644 --- a/tests/Requests/RateLimiterTest.php +++ b/tests/Requests/RateLimiterTest.php @@ -2,22 +2,22 @@ namespace Tests\Unit\Requests; -use Phaseolies\Cache\RateLimiter; +use Phaseolies\Cache\IncrementableCacheInterface; use Phaseolies\Cache\RateLimit; -use Psr\SimpleCache\CacheInterface; -use Psr\SimpleCache\InvalidArgumentException; -use PHPUnit\Framework\TestCase; +use Phaseolies\Cache\RateLimiter; use PHPUnit\Framework\Attributes\AllowMockObjectsWithoutExpectations; +use PHPUnit\Framework\TestCase; +use Psr\SimpleCache\InvalidArgumentException; #[AllowMockObjectsWithoutExpectations] class RateLimiterTest extends TestCase { - protected $cache; - protected $limiter; + protected IncrementableCacheInterface $cache; + protected RateLimiter $limiter; protected function setUp(): void { - $this->cache = $this->createMock(CacheInterface::class); + $this->cache = $this->createMock(IncrementableCacheInterface::class); $this->limiter = new RateLimiter($this->cache); } @@ -33,36 +33,31 @@ public function testAttemptWithNewKey() $decaySeconds = 60; $now = time(); - // Mock has() to return false for both key checks - $this->cache->expects($this->exactly(1)) - ->method('has') - ->willReturnCallback(function ($arg) use ($key) { - TestCase::assertTrue(in_array($arg, [$key, $key . '_timer'])); - }) - ->willReturn(false); - - // Mock set() calls $this->cache->expects($this->exactly(2)) - ->method('set') + ->method('add') ->willReturnCallback(function ($keyArg, $valueArg, $ttlArg) use ($key, $decaySeconds, $now) { static $call = 0; $call++; if ($call === 1) { - // First expected call: [$key, 1, $decaySeconds] - TestCase::assertEquals($key, $keyArg); - TestCase::assertEquals(1, $valueArg); - TestCase::assertEquals($decaySeconds, $ttlArg); - } elseif ($call === 2) { - // Second expected call: [$key . '_timer', $now + $decaySeconds, $decaySeconds] - TestCase::assertEquals($key . '_timer', $keyArg); - TestCase::assertEquals($now + $decaySeconds, $valueArg); - TestCase::assertEquals($decaySeconds, $ttlArg); + TestCase::assertSame($key, $keyArg); + TestCase::assertSame(1, $valueArg); + TestCase::assertSame($decaySeconds, $ttlArg); + + return true; } + TestCase::assertSame($key . '_timer', $keyArg); + TestCase::assertSame($now + $decaySeconds, $valueArg); + TestCase::assertSame($decaySeconds, $ttlArg); + return true; - }) - ->willReturn(true); + }); + + $this->cache->expects($this->once()) + ->method('get') + ->with($key . '_timer') + ->willReturn($now + $decaySeconds); $result = $this->limiter->attempt($key, $maxAttempts, $decaySeconds); @@ -72,6 +67,84 @@ public function testAttemptWithNewKey() $this->assertEquals($now + $decaySeconds, $result->resetAt); } + public function testAttemptWithExistingKeyUsesIncrementPath() + { + $key = 'test_key'; + $maxAttempts = 5; + $decaySeconds = 60; + $now = time(); + + $this->cache->expects($this->once()) + ->method('add') + ->with($key, 1, $decaySeconds) + ->willReturn(false); + + $this->cache->expects($this->once()) + ->method('increment') + ->with($key) + ->willReturn(3); + + $this->cache->expects($this->once()) + ->method('set') + ->with($key . '_timer', $now + $decaySeconds, $decaySeconds) + ->willReturn(true); + + $this->cache->expects($this->once()) + ->method('get') + ->with($key . '_timer') + ->willReturn($now + $decaySeconds); + + $result = $this->limiter->attempt($key, $maxAttempts, $decaySeconds); + + $this->assertSame(2, $result->remaining); + } + + public function testAttemptRecoversWhenIncrementReturnsFalse() + { + $key = 'test_key'; + $maxAttempts = 5; + $decaySeconds = 60; + $now = time(); + + $this->cache->expects($this->once()) + ->method('add') + ->with($key, 1, $decaySeconds) + ->willReturn(false); + + $this->cache->expects($this->once()) + ->method('increment') + ->with($key) + ->willReturn(false); + + $this->cache->expects($this->exactly(2)) + ->method('set') + ->willReturnCallback(function ($keyArg, $valueArg, $ttlArg) use ($key, $decaySeconds, $now) { + static $call = 0; + $call++; + + if ($call === 1) { + TestCase::assertSame($key, $keyArg); + TestCase::assertSame(1, $valueArg); + } else { + TestCase::assertSame($key . '_timer', $keyArg); + TestCase::assertSame($now + $decaySeconds, $valueArg); + } + + TestCase::assertSame($decaySeconds, $ttlArg); + + return true; + }); + + $this->cache->expects($this->once()) + ->method('get') + ->with($key . '_timer') + ->willReturn($now + $decaySeconds); + + $result = $this->limiter->attempt($key, $maxAttempts, $decaySeconds); + + $this->assertSame($maxAttempts - 1, $result->remaining); + } + public function testTooManyAttempts() { $key = 'test_key'; @@ -125,32 +198,55 @@ public function testHitWithNewKey() $decaySeconds = 60; $now = time(); - $this->cache->method('has') - ->with($key) - ->willReturn(false); - $this->cache->expects($this->exactly(2)) - ->method('set') + ->method('add') ->willReturnCallback(function ($keyArg, $valueArg, $ttlArg) use ($key, $decaySeconds, $now) { static $call = 0; $call++; if ($call === 1) { - TestCase::assertEquals($key, $keyArg); - TestCase::assertEquals(1, $valueArg); - TestCase::assertEquals($decaySeconds, $ttlArg); - } elseif ($call === 2) { - TestCase::assertEquals($key . '_timer', $keyArg); - TestCase::assertEquals($now + $decaySeconds, $valueArg); - TestCase::assertEquals($decaySeconds, $ttlArg); + TestCase::assertSame($key, $keyArg); + TestCase::assertSame(1, $valueArg); + TestCase::assertSame($decaySeconds, $ttlArg); + + return true; } - }) - ->willReturn(true); + + TestCase::assertSame($key . '_timer', $keyArg); + TestCase::assertSame($now + $decaySeconds, $valueArg); + TestCase::assertSame($decaySeconds, $ttlArg); + + return true; + }); $result = $this->limiter->hit($key, $decaySeconds); $this->assertEquals(1, $result); } + public function testHitWithExistingKeyUsesIncrement() + { + $key = 'test_key'; + $decaySeconds = 60; + $now = time(); + + $this->cache->expects($this->once()) + ->method('add') + ->with($key, 1, $decaySeconds) + ->willReturn(false); + + $this->cache->expects($this->once()) + ->method('increment') + ->with($key) + ->willReturn(4); + + $this->cache->expects($this->once()) + ->method('set') + ->with($key . '_timer', $now + $decaySeconds, $decaySeconds) + ->willReturn(true); + + $this->assertSame(4, $this->limiter->hit($key, $decaySeconds)); + } + public function testClear() { $key = 'test_key'; @@ -166,8 +262,9 @@ public function testClear() } elseif ($call === 2) { TestCase::assertEquals($key . '_timer', $keyArg); } - }) - ->willReturn(true); + + return true; + }); $this->limiter->clear($key); } @@ -214,8 +311,8 @@ public function testCacheExceptionHandling() $key = 'test_key'; $exception = new class extends \Exception implements InvalidArgumentException {}; - $this->cache->method('has') - ->with($key) + $this->cache->method('add') + ->with($key, 1, 60) ->willThrowException($exception); $this->expectException(InvalidArgumentException::class);