From c094b88944c0d437af70afde6b2cb19142b1f073 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 7 Aug 2026 10:28:29 -0400 Subject: [PATCH 01/15] Add CompareAndSwapInterface and fix races in atomic operations Introduce CompareAndSwapInterface with setIfAbsent, deleteIfEquals and expireIfEquals: conditional writes the engine resolves in one indivisible step, which is what a correct distributed lock needs. Implemented by RedisCacheEngine (SET NX EX plus Lua) and MemcachedEngine (native add plus cas). Kept separate from AtomicOperationInterface so existing implementors are untouched. Fix three real races in the existing atomic operations: - Memcached seeded a missing key with get()===false then set() in increment, decrement and add. The increment itself is atomic but the seeding is not, so a slower caller's set(0) lands after a faster caller's increment and reissues the same value. Twenty concurrent increments ended at 8. Now seeded with Memcached::add(). - Redis add() converted a string key into a list with GET, DEL and re-push, unprotected, so concurrent callers deleted a list the others were rebuilding. Twenty concurrent appends duplicated the seed value 16 times. The conversion is now a compare-guarded script. - Redis increment and decrement applied the TTL as a separate EXPIRE, leaving a counter with no expiry if the process died in between. Both now run as one script, and add() finally honours the $ttl it had been accepting and discarding. Also fix FileSystem, where the atomic operations passed a relative TTL where an absolute timestamp was expected, so anything written with a TTL expired in 1970; and where the expiry file was dropped before the lock was taken. FileSystem does not implement CompareAndSwapInterface: flock binds to an inode and the engine unlinks on delete, so two processes can hold the lock on two different inodes at the same path. Tests fork twenty real processes at a synchronised barrier; each fix has a case that fails without it. --- CHANGELOG-7.0.md | 192 +++++++++++++++++++++++++ README.md | 2 + docs/atomic-operations.md | 19 +++ docs/compare-and-swap.md | 141 ++++++++++++++++++ src/CompareAndSwapInterface.php | 65 +++++++++ src/Psr16/FileSystemCacheEngine.php | 31 ++-- src/Psr16/MemcachedEngine.php | 123 ++++++++++++---- src/Psr16/RedisCacheEngine.php | 212 ++++++++++++++++++++++----- tests/CachePSR16Test.php | 32 +++++ tests/CompareAndSwapTest.php | 164 +++++++++++++++++++++ tests/ConcurrencyTest.php | 213 ++++++++++++++++++++++++++++ 11 files changed, 1115 insertions(+), 79 deletions(-) create mode 100644 CHANGELOG-7.0.md create mode 100644 docs/compare-and-swap.md create mode 100644 src/CompareAndSwapInterface.php create mode 100644 tests/CompareAndSwapTest.php create mode 100644 tests/ConcurrencyTest.php diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md new file mode 100644 index 0000000..579be28 --- /dev/null +++ b/CHANGELOG-7.0.md @@ -0,0 +1,192 @@ +# Changelog - Version 7.0 + +## Overview + +Version 7.0 adds `CompareAndSwapInterface`, the conditional-write primitive needed to build correct +distributed locks on top of a cache engine, and repairs three genuine race conditions in the +existing atomic operations. + +The races were not theoretical. A forked-process test reproduces each one against 6.x: twenty +concurrent `increment()` calls on Memcached ended at **8** instead of 20, and twenty concurrent +`add()` calls on Redis duplicated the seed value **16 times** while dropping appends. + +## Breaking Changes + +| Area | Before (6.x) | After (7.0) | Description | +|------|--------------|-------------|-------------| +| **Redis atomic operations** | `INCR`/`RPUSH` + separate `EXPIRE` | Lua scripts (`EVAL`) | `increment()`, `decrement()` and `add()` are now single scripts so the value and its TTL cannot be split apart. **Requires Lua scripting to be enabled on the Redis server**; some managed offerings restrict `EVAL`. | +| **Redis `add()` TTL** | `$ttl` accepted and silently ignored | `$ttl` applied | A list built with `add($key, $value, 60)` now actually expires. Code that relied on the list living forever despite passing a TTL will see it expire. | +| **FileSystem atomic TTL** | raw `$ttl` written as the expiry timestamp | converted with `addToNow()` | `increment($key, 1, 60)` wrote `60` into the expiry file — a moment in 1970 — so the entry was already expired when written. It now means 60 seconds from now, and the value survives as intended. | + +## New Features + +### CompareAndSwapInterface + +A new interface for conditional writes that the storage engine resolves in one indivisible step: + +- `setIfAbsent(string $key, mixed $value, DateInterval|int|null $ttl = null): bool` — stores only if + the key is free; exactly one of any number of concurrent callers gets `true`. The TTL is applied + in the same step as the write, so a process that dies mid-operation cannot leave a key with no + expiry behind. +- `deleteIfEquals(string $key, mixed $value): bool` — deletes only while the key still holds your + value, so an owner whose TTL quietly lapsed cannot destroy the entry someone else has taken over. +- `expireIfEquals(string $key, mixed $value, DateInterval|int|null $ttl): bool` — extends the TTL + under the same guard. + +**Implemented by:** `RedisCacheEngine` (`SET NX EX` plus Lua for the guarded operations) and +`MemcachedEngine` (native `add()` plus `cas()`). + +`FileSystemCacheEngine` deliberately does **not** implement it. `flock()` attaches to an inode +rather than a path, and this engine unlinks the file on delete, so two processes can hold "the +lock" on two different inodes at the same path. The file system is not a reliable substrate for +mutual exclusion and the engine does not pretend otherwise. + +This is a separate interface rather than an addition to `AtomicOperationInterface`, so nothing that +already implements the latter breaks. Probe with `instanceof` and degrade explicitly: + +```php +if (!$cache instanceof \ByJG\Cache\CompareAndSwapInterface) { + throw new RuntimeException('This engine cannot guarantee mutual exclusion'); +} +``` + +### Documentation + +- New [Compare and Swap](docs/compare-and-swap.md) guide, including a worked lock example and an + explicit section on what the interface does *not* give you (no fairness, no reentrancy, the TTL + is still a guess). +- [Atomic Operations](docs/atomic-operations.md) now documents the TTL semantics and what happens + when `add()` is called on a key previously written by `set()`. + +## Bug Fixes + +### Memcached: non-atomic seeding in every atomic operation + +`increment()`, `decrement()` and `add()` all initialised a missing key with `get() === false` +followed by `set()`. Memcached's own `increment()` is atomic, but those two preparatory calls are +not, and the interleaving loses updates: + +``` +P1: get() === false P2: get() === false +P1: set(0) +P1: increment() -> 1 + P2: set(0) <- resets the counter + P2: increment() -> 1 <- the same value issued twice +``` + +All three now seed with `Memcached::add()`, which the server resolves in a single step: exactly one +caller creates the key and the rest move on. + +`add()` was additionally hardened — it no longer dereferences the CAS token when the key expired +mid-loop, and it claims an absent key with `add()` instead of `set()`. + +### Redis: `add()` corrupted the list when converting from a plain value + +The first `add()` to a key written by `set()` has to turn a string into a list. That was done as +`GET` → `DEL` → re-push → `RPUSH` with no protection, so concurrent callers each deleted a list the +others were mid-way through rebuilding. The conversion is now a compare-guarded script that only +rewrites the key while it still holds the value that was read; losing that compare means another +process already converted it. + +### Redis: TTL applied as a separate command + +`increment()` and `decrement()` set the expiry with a follow-up `EXPIRE`. A crash between the two +left a counter with no expiry at all. Both now apply it inside the script. + +### FileSystem: expiry dropped outside the lock + +`putContents()` deleted the `.ttl` file before acquiring the lock, leaving the value briefly +immortal — a reader arriving in that window saw an entry that should already have expired. The +delete now happens under the lock. + +## Testing + +- `tests/CompareAndSwapTest.php` — 18 tests across both engines covering expiry, non-owner + rejection and the stale-owner-versus-new-owner case. +- `tests/ConcurrencyTest.php` — forks twenty real processes at a synchronised barrier. Sequential + tests cannot prove atomicity; these fail against 6.x and pass on 7.0. +- A TTL regression test added to `tests/CachePSR16Test.php` covering every engine that implements + `AtomicOperationInterface`. + +Note for anyone writing similar tests: forked children inherit the parent's Memcached socket and +close it as they exit, killing the parent's connection. Reconnect in the parent before asserting. + +## Migration Path from 6.x to 7.0 + +### Step 1: Confirm Lua scripting is available on your Redis server + +`increment()`, `decrement()` and `add()` now use `EVAL`. This is enabled by default in Redis, but +some managed and proxied deployments restrict it. Verify with: + +```bash +redis-cli EVAL "return 1" 0 # should print (integer) 1 +``` + +If `EVAL` is unavailable in your environment, stay on 6.x or open an issue. + +### Step 2: Update the dependency + +```bash +composer require byjg/cache-engine:^7.0 +composer update +``` + +### Step 3: Review any use of `add()` with a TTL on Redis + +The TTL was previously ignored, so lists built this way never expired. They now do. If you were +relying on the old behaviour, drop the TTL argument: + +```php +$cache->add('my-key', 'value', 3600); // now expires after an hour +$cache->add('my-key', 'value'); // never expires, as 6.x behaved +``` + +### Step 4: Review any use of atomic operations with a TTL on FileSystem + +These were broken and returned already-expired entries, so working code is unlikely to depend on +them. If you worked around the bug by omitting the TTL, you can now pass it. + +### Step 5: Adopt compare-and-swap where you were hand-rolling a lock + +If you have code shaped like this, replace it — it has a race between the two calls: + +```php +// Before - two operations with a gap between them +if (!$cache->has('lock')) { + $cache->set('lock', $token, 30); +} + +// After - one indivisible operation +if ($cache->setIfAbsent('lock', $token, 30)) { + try { + // ... + } finally { + $cache->deleteIfEquals('lock', $token); + } +} +``` + +### Common Migration Issues + +**Issue**: `NOSCRIPT` or "unknown command EVAL" errors from Redis +**Solution**: Lua scripting is disabled or proxied away in your deployment. See Step 1. + +**Issue**: Cached lists built with `add()` now disappear +**Solution**: You are passing a TTL that was previously ignored. See Step 3. + +**Issue**: `$cache->setIfAbsent()` does not exist +**Solution**: The engine does not implement `CompareAndSwapInterface`. Only `RedisCacheEngine` and +`MemcachedEngine` do. Probe with `instanceof` before calling. + +## Notes + +- `AtomicOperationInterface` is unchanged; existing implementations continue to work untouched. +- No changes to the PSR-6 or PSR-16 interface implementations. +- Existing cache data remains compatible across versions. +- PHP requirement is unchanged at `>=8.3 <8.6`. + +## Links + +- [Full Commit History](https://github.com/byjg/php-cache-engine/compare/6.0.1...7.0.0) +- [Documentation](https://github.com/byjg/php-cache-engine/tree/master/docs) +- [Report Issues](https://github.com/byjg/php-cache-engine/issues) diff --git a/README.md b/README.md index c297706..cc60f8a 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ A powerful, versatile cache implementation providing both PSR-6 and PSR-16 inter - **PSR-6 Cache Pool interface** - More verbose caching with fine-grained control - **Multiple storage backends** - Choose from memory, file system, Redis, Memcached and more - **Atomic operations** - Support for increment, decrement and add operations in compatible engines +- **Compare and swap** - Conditional writes for correct distributed locking on Redis and Memcached - **Garbage collection** - Automatic cleanup of expired items - **PSR-11 container support** - Retrieve cache keys via dependency container - **Logging capabilities** - PSR-3 compatible logging of cache operations @@ -68,6 +69,7 @@ $value = $item->get(); ### Advanced Features - [Atomic Operations](docs/atomic-operations.md) +- [Compare and Swap](docs/compare-and-swap.md) - [Garbage Collection](docs/garbage-collection.md) - [Logging](docs/setup-log-handler.md) - [PSR-11 Container Usage](docs/psr11-usage.md) diff --git a/docs/atomic-operations.md b/docs/atomic-operations.md index 542b8cf..8dc2539 100644 --- a/docs/atomic-operations.md +++ b/docs/atomic-operations.md @@ -24,6 +24,13 @@ The engines that support atomic operations implement the `AtomicOperationInterfa - FileSystemCacheEngine - TmpfsCacheEngine (inherits from FileSystemCacheEngine) +These operations read and modify a value in one step. If what you need instead is a *conditional* +write — "store this only if the key is free", "delete this only if it is still mine" — see +[Compare and Swap](compare-and-swap.md). + +All three operations accept an optional TTL, expressed in seconds from now (or as a `DateInterval`), +and apply it as part of the same operation. + ## Increment The increment operation is used to increment a value by a given number. @@ -58,3 +65,15 @@ $cache->add('my-key', 'value3'); print_r($cache->get('my-key')); // ['value1', 'value2', 'value3'] ``` +If the key already holds a plain value written by `set()`, the first `add()` converts it into a +list and keeps the original value as the first element: + +```php +set('my-key', 'value1'); +$cache->add('my-key', 'value2'); + +print_r($cache->get('my-key')); // ['value1', 'value2'] +``` + + diff --git a/docs/compare-and-swap.md b/docs/compare-and-swap.md new file mode 100644 index 0000000..1257460 --- /dev/null +++ b/docs/compare-and-swap.md @@ -0,0 +1,141 @@ +--- +sidebar_position: 12 +--- + +# Compare and Swap + +Compare-and-swap means: read the current state, compare it against what you expect, and only write +when it matches — all resolved by the storage engine in a single indivisible step. + +The reason this needs its own interface is that the obvious way of writing it is wrong: + +```php +has('lock')) { // <- another process can write here + $cache->set('lock', $token, 30); +} +``` + +Between the `has()` and the `set()` there is a gap. Two processes can both find the key absent and +both write, and both walk away believing they own it. No amount of re-reading afterwards closes +that gap. Compare-and-swap removes it by never splitting the check from the write. + +The engines that support it implement `CompareAndSwapInterface`. + +**Engines that support compare and swap:** + +- RedisCacheEngine +- MemcachedEngine + +FileSystemCacheEngine does **not** implement this interface. See +[the note at the bottom](#why-not-filesystemcacheengine). + +## setIfAbsent + +Stores the value only if the key currently holds nothing. Exactly one of any number of concurrent +callers gets `true`. A key whose TTL has already elapsed counts as absent. + +```php +setIfAbsent('my-lock', $token, 30)) { + // This process, and only this process, owns 'my-lock' for the next 30 seconds +} +``` + +The TTL is applied in the same step as the write. This matters: an engine that wrote the value +first and set the expiry afterwards would leave a key with no expiry at all if the process died in +between — a lock nobody can ever release. + +## deleteIfEquals + +Deletes the key only if it still holds exactly the value you pass. + +```php +deleteIfEquals('my-lock', $token); +``` + +Use this instead of `delete()` whenever the key might have changed hands. If your TTL quietly +expired while you were still working, another process may already hold the key — a plain `delete()` +would destroy *their* lock. `deleteIfEquals()` refuses and returns `false`. + +## expireIfEquals + +Rewrites the TTL, but only while the key still holds your value. This lets a long-running owner +keep its claim alive without any risk of extending a claim that has already passed to someone else. + +```php +expireIfEquals('my-lock', $token, 30)) { + // We lost the lock. Stop - somebody else is doing this work now. + break; + } +} +``` + +Passing `null` as the TTL removes the expiry, making the key permanent. + +## Putting it together + +```php +setIfAbsent('rebuild-report', $token, 60)) { + return; // Another worker already has it +} + +try { + rebuildTheReport(); +} finally { + $cache->deleteIfEquals('rebuild-report', $token); +} +``` + +The random token is what makes the release safe. Without it there is no way to tell your own lock +apart from the one a later process took over after your TTL lapsed. + +## Detecting support + +Not every engine can offer these guarantees. Probe before relying on them, and degrade explicitly +rather than silently: + +```php +putContents($this->fixKey($key), $value, $ttl, function ($currentValue, $value) { + // addToNow() is what turns a relative TTL into the absolute timestamp the .ttl file holds. + // Passing the raw $ttl through wrote "60" as the expiry, i.e. a moment in 1970, so any + // counter created with a TTL was already expired by the time it was written. + return $this->putContents($this->fixKey($key), $value, $this->addToNow($ttl), function ($currentValue, $value) { return intval($currentValue) + $value; }); } @@ -287,7 +292,7 @@ public function increment(string $key, int $value = 1, DateInterval|int|null $tt #[Override] public function decrement(string $key, int $value = 1, DateInterval|int|null $ttl = null): int { - return $this->putContents($this->fixKey($key), $value, $ttl, function ($currentValue, $value) { + return $this->putContents($this->fixKey($key), $value, $this->addToNow($ttl), function ($currentValue, $value) { return intval($currentValue) - $value; }); } @@ -295,7 +300,7 @@ public function decrement(string $key, int $value = 1, DateInterval|int|null $tt #[Override] public function add(string $key, $value, DateInterval|int|null $ttl = null): array { - return $this->putContents($this->fixKey($key), $value, $ttl, function ($currentValue, $value) { + return $this->putContents($this->fixKey($key), $value, $this->addToNow($ttl), function ($currentValue, $value) { if (empty($currentValue)) { return [$value]; } diff --git a/src/Psr16/MemcachedEngine.php b/src/Psr16/MemcachedEngine.php index 71200ab..b6d328f 100644 --- a/src/Psr16/MemcachedEngine.php +++ b/src/Psr16/MemcachedEngine.php @@ -3,6 +3,7 @@ namespace ByJG\Cache\Psr16; use ByJG\Cache\AtomicOperationInterface; +use ByJG\Cache\CompareAndSwapInterface; use ByJG\Cache\Exception\InvalidArgumentException; use ByJG\Cache\Exception\StorageErrorException; use DateInterval; @@ -12,8 +13,13 @@ use Psr\Log\LoggerInterface; use Psr\Log\NullLogger; -class MemcachedEngine extends BaseCacheEngine implements AtomicOperationInterface +class MemcachedEngine extends BaseCacheEngine implements AtomicOperationInterface, CompareAndSwapInterface { + /** + * Memcached expires an item immediately when handed a negative expiration. That is the only + * way to retire a key through cas(), since the extension exposes no compare-and-delete. + */ + private const EXPIRE_NOW = -1; /** * @@ -231,16 +237,13 @@ public function increment(string $key, int $value = 1, DateInterval|int|null $tt { $this->lazyLoadMemCachedServers(); - $ttl = $this->convertToSeconds($ttl); - - if ($this->memCached->get($this->fixKey($key)) === false) { - $this->memCached->set($this->fixKey($key), 0, is_null($ttl) ? 0 : $ttl); - } + $fixKey = $this->fixKey($key); + $this->seed($fixKey, 0, $ttl); - $result = $this->memCached->increment($this->fixKey($key), $value); + $result = $this->memCached->increment($fixKey, $value); $this->logger->info("[Memcached] Increment '$key' result " . $this->memCached->getResultCode()); if ($this->memCached->getResultCode() !== Memcached::RES_SUCCESS) { - $this->logger->error("[Memcached] Set '$key' failed with status " . $this->memCached->getResultCode()); + $this->logger->error("[Memcached] Increment '$key' failed with status " . $this->memCached->getResultCode()); } return $result; @@ -257,21 +260,34 @@ public function decrement(string $key, int $value = 1, DateInterval|int|null $tt { $this->lazyLoadMemCachedServers(); - $ttl = $this->convertToSeconds($ttl); - - if ($this->memCached->get($this->fixKey($key)) === false) { - $this->memCached->set($this->fixKey($key), 0, is_null($ttl) ? 0 : $ttl); - } + $fixKey = $this->fixKey($key); + $this->seed($fixKey, 0, $ttl); - $result = $this->memCached->decrement($this->fixKey($key), $value); + $result = $this->memCached->decrement($fixKey, $value); $this->logger->info("[Memcached] Decrement '$key' result " . $this->memCached->getResultCode()); if ($this->memCached->getResultCode() !== Memcached::RES_SUCCESS) { - $this->logger->error("[Memcached] Set '$key' failed with status " . $this->memCached->getResultCode()); + $this->logger->error("[Memcached] Decrement '$key' failed with status " . $this->memCached->getResultCode()); } return $result; } + /** + * Create the key only if it is absent, so a counter can be started without a race. + * + * increment()/decrement() fail on a missing key, which forces every caller to initialise it + * first. Doing that with get()-then-set() is what used to break: two processes could both read + * the key as absent, and the slower one's set(0) would land AFTER the faster one's increment, + * resetting the counter and handing out the same value twice. add() is resolved server-side in + * a single step, so exactly one caller creates the key and everybody else silently moves on. + */ + private function seed(string $fixKey, mixed $initial, DateInterval|int|null $ttl): void + { + $ttl = $this->convertToSeconds($ttl); + + $this->memCached->add($fixKey, $initial, is_null($ttl) ? 0 : $ttl); + } + /** * @throws NotFoundExceptionInterface * @throws InvalidArgumentException @@ -284,29 +300,80 @@ public function add(string $key, $value, DateInterval|int|null $ttl = null): arr $this->lazyLoadMemCachedServers(); $ttl = $this->convertToSeconds($ttl); + $expiration = is_null($ttl) ? 0 : $ttl; $fixKey = $this->fixKey($key); - if ($this->memCached->get($fixKey) === false) { - $this->memCached->set($fixKey, [], is_null($ttl) ? 0 : $ttl); - } - - do { + while (true) { $data = $this->memCached->get($fixKey, null, Memcached::GET_EXTENDED); - $casToken = $data['cas']; - $currentValue = $data['value']; - if ($currentValue === false) { - $currentValue = []; + // Absent: claim it with add(), which only succeeds for the first caller to get there. + // Whoever loses simply goes round again and finds the list the winner created. + if ($data === false) { + if ($this->memCached->add($fixKey, [$value], $expiration)) { + return [$value]; + } + continue; } + $currentValue = $data['value']; if (!is_array($currentValue)) { $currentValue = [$currentValue]; } - $currentValue[] = $value; - $success = $this->memCached->cas($casToken, $fixKey, $currentValue, is_null($ttl) ? 0 : $ttl); - } while (!$success); - return $currentValue; + // cas() writes only while the item is untouched since the read above; if another + // append slipped in, the token is stale, the write is refused and we retry on top of it. + if ($this->memCached->cas($data['cas'], $fixKey, $currentValue, $expiration)) { + return $currentValue; + } + } + } + + #[\Override] + public function setIfAbsent(string $key, mixed $value, DateInterval|int|null $ttl = null): bool + { + $this->lazyLoadMemCachedServers(); + + $ttl = $this->convertToSeconds($ttl); + + // Memcached's own add() is the primitive this whole interface is named after. + return $this->memCached->add($this->fixKey($key), $value, is_null($ttl) ? 0 : $ttl); + } + + #[\Override] + public function deleteIfEquals(string $key, mixed $value): bool + { + // No compare-and-delete exists, so retire the item by expiring it in the same cas() that + // proves we are still looking at the value we expect. + return $this->casIfEquals($key, $value, self::EXPIRE_NOW); + } + + #[\Override] + public function expireIfEquals(string $key, mixed $value, DateInterval|int|null $ttl): bool + { + $ttl = $this->convertToSeconds($ttl); + + return $this->casIfEquals($key, $value, is_null($ttl) ? 0 : $ttl); + } + + /** + * Rewrite the item with a new expiration, but only while it still holds $value. + * + * The cas token read here is invalidated server-side by any competing write, so a caller whose + * item was replaced between the get() and the cas() is refused rather than silently clobbering + * the new owner's value. + */ + private function casIfEquals(string $key, mixed $value, int $expiration): bool + { + $this->lazyLoadMemCachedServers(); + + $fixKey = $this->fixKey($key); + $data = $this->memCached->get($fixKey, null, Memcached::GET_EXTENDED); + + if ($data === false || $data['value'] != $value) { + return false; + } + + return $this->memCached->cas($data['cas'], $fixKey, $data['value'], $expiration); } } diff --git a/src/Psr16/RedisCacheEngine.php b/src/Psr16/RedisCacheEngine.php index 20b9d26..92f7aee 100644 --- a/src/Psr16/RedisCacheEngine.php +++ b/src/Psr16/RedisCacheEngine.php @@ -3,7 +3,9 @@ namespace ByJG\Cache\Psr16; use ByJG\Cache\AtomicOperationInterface; +use ByJG\Cache\CompareAndSwapInterface; use ByJG\Cache\Exception\InvalidArgumentException; +use ByJG\Cache\Exception\StorageErrorException; use DateInterval; use Psr\Container\ContainerExceptionInterface; use Psr\Container\NotFoundExceptionInterface; @@ -12,8 +14,66 @@ use Redis; use RedisException; -class RedisCacheEngine extends BaseCacheEngine implements AtomicOperationInterface +class RedisCacheEngine extends BaseCacheEngine implements AtomicOperationInterface, CompareAndSwapInterface { + /** + * Every multi-step operation below runs as a Lua script instead of a sequence of commands. + * Redis executes a script to completion before serving any other client, so the read and the + * write it performs cannot be interleaved - which is exactly the guarantee the callers need. + */ + private const LUA_INCREMENT_BY = <<<'LUA' + local result = redis.call('INCRBY', KEYS[1], ARGV[1]) + if tonumber(ARGV[2]) > 0 then + redis.call('EXPIRE', KEYS[1], ARGV[2]) + end + return result + LUA; + + /** Appends to the list and hands back the result the caller would otherwise have to re-read. */ + private const LUA_APPEND = <<<'LUA' + if redis.call('TYPE', KEYS[1])['ok'] == 'string' then + return false + end + redis.call('RPUSH', KEYS[1], ARGV[1]) + if tonumber(ARGV[2]) > 0 then + redis.call('EXPIRE', KEYS[1], ARGV[2]) + end + return redis.call('LRANGE', KEYS[1], 0, -1) + LUA; + + /** Rewrites a string key as a list, but only while it still holds the value we inspected. */ + private const LUA_EXPLODE_STRING = <<<'LUA' + if redis.call('GET', KEYS[1]) ~= ARGV[1] then + return 0 + end + redis.call('DEL', KEYS[1]) + for i = 2, #ARGV do + redis.call('RPUSH', KEYS[1], ARGV[i]) + end + return 1 + LUA; + + private const LUA_DELETE_IF_EQUALS = <<<'LUA' + if redis.call('GET', KEYS[1]) == ARGV[1] then + return redis.call('DEL', KEYS[1]) + end + return 0 + LUA; + + private const LUA_EXPIRE_IF_EQUALS = <<<'LUA' + if redis.call('GET', KEYS[1]) ~= ARGV[1] then + return 0 + end + if tonumber(ARGV[2]) > 0 then + redis.call('EXPIRE', KEYS[1], ARGV[2]) + else + redis.call('PERSIST', KEYS[1]) + end + return 1 + LUA; + + /** Bound on the string-to-list conversion retry; only a pathological writer ever gets close. */ + private const MAX_CONVERSION_ATTEMPTS = 10; /** * @@ -72,6 +132,25 @@ protected function fixKey(string $key): string return "cache:$key"; } + /** + * Redis stores scalars verbatim and everything else serialized. get(), add() and the + * compare-and-swap operations must agree on this representation, otherwise a CAS comparison + * would be made against a string the caller never actually wrote. + */ + protected function encode(mixed $value): mixed + { + return is_object($value) || is_array($value) ? serialize($value) : $value; + } + + protected function decode(mixed $value): mixed + { + if (is_string($value) && preg_match('/^[Oa]:\d+:["{]/', $value)) { + return unserialize($value); + } + + return $value; + } + /** * @param string $key * @param mixed $default @@ -90,10 +169,7 @@ public function get(string $key, mixed $default = null): mixed $type = $this->redis->type($fixKey); if ($type === Redis::REDIS_STRING) { - $value = $this->redis->get($fixKey); - if (is_string($value) && preg_match('/^[Oa]:\d+:["{]/', $value)) { - $value = unserialize($value); - } + $value = $this->decode($this->redis->get($fixKey)); } else if ($type === Redis::REDIS_LIST) { $value = $this->redis->lRange($fixKey, 0, -1); } else { @@ -102,9 +178,7 @@ public function get(string $key, mixed $default = null): mixed if (is_array($value)) { foreach ($value as $k => $v) { - if (is_string($v) && preg_match('/^[Oa]:\d+:["{]/', $v)) { - $value[$k] = unserialize($v); - } + $value[$k] = $this->decode($v); } } @@ -200,55 +274,117 @@ public function isAvailable(): bool #[\Override] public function increment(string $key, int $value = 1, DateInterval|int|null $ttl = null): int { - $this->lazyLoadRedisServer(); + return $this->incrementBy($key, $value, $ttl); + } - $result = $this->redis->incr($this->fixKey($key), $value); + #[\Override] + public function decrement(string $key, int $value = 1, DateInterval|int|null $ttl = null): int + { + return $this->incrementBy($key, -$value, $ttl); + } - if ($ttl) { - $this->redis->expire($this->fixKey($key), $this->convertToSeconds($ttl)); - } + /** + * INCRBY covers both directions. Applying the TTL inside the script matters: as two separate + * commands, a process that died in between would leave the counter with no expiry at all. + */ + private function incrementBy(string $key, int $delta, DateInterval|int|null $ttl): int + { + return (int)$this->script(self::LUA_INCREMENT_BY, $key, [$delta, $this->ttlInSeconds($ttl)]); + } + + /** + * Run one of the scripts above against a single key. + * + * Every script here takes exactly one key and reads the rest of its inputs from ARGV, so the + * lazy connect and the eval() shape are the same each time and belong in one place. + */ + private function script(string $lua, string $key, array $args = []): mixed + { + $this->lazyLoadRedisServer(); - return $result; + return $this->redis->eval($lua, array_merge([$this->fixKey($key)], $args), 1); } + /** + * @throws StorageErrorException + */ #[\Override] - public function decrement(string $key, int $value = 1, DateInterval|int|null $ttl = null): int + public function add(string $key, $value, DateInterval|int|null $ttl = null): array { - $this->lazyLoadRedisServer(); + $args = [$this->encode($value), $this->ttlInSeconds($ttl)]; + + // A key previously written by set() is a plain string and has to become a list before it + // can be appended to. The append itself never contends - the loop only exists so a caller + // that loses that one-shot conversion to a concurrent writer can pick up and carry on. + for ($attempt = 0; $attempt < self::MAX_CONVERSION_ATTEMPTS; $attempt++) { + $list = $this->script(self::LUA_APPEND, $key, $args); + + if (is_array($list)) { + return array_map(fn($item) => $this->decode($item), $list); + } + + $this->convertStringToList($key); + } - $result = $this->redis->decr($this->fixKey($key), $value); + throw new StorageErrorException("Could not append to '$key': the key kept changing type"); + } - if ($ttl) { - $this->redis->expire($this->fixKey($key), $this->convertToSeconds($ttl)); + /** + * Turn a string key into the list add() appends to, without ever exposing a moment where the + * key is missing. The script rewrites it only while it still holds the value we just read, so + * losing the race means another process already converted it and there is nothing left to do. + */ + private function convertStringToList(string $key): void + { + $current = $this->redis->get($this->fixKey($key)); + if ($current === false) { + return; // Expired or deleted meanwhile; the next append creates a fresh list. } - return $result; + $decoded = $this->decode($current); + $elements = is_array($decoded) ? array_values($decoded) : [$decoded]; + + $this->script( + self::LUA_EXPLODE_STRING, + $key, + array_merge([$current], array_map(fn($item) => $this->encode($item), $elements)) + ); } #[\Override] - public function add(string $key, $value, DateInterval|int|null $ttl = null): array + public function setIfAbsent(string $key, mixed $value, DateInterval|int|null $ttl = null): bool { $this->lazyLoadRedisServer(); - $fixKey = $this->fixKey($key); - $type = $this->redis->type($fixKey); + $seconds = $this->ttlInSeconds($ttl); + // SET NX writes the value and its expiry as one command - there is no window in which the + // key exists without a TTL, so a crash here cannot leave the key behind forever. + $options = $seconds > 0 ? ['nx', 'ex' => $seconds] : ['nx']; - if ($type === Redis::REDIS_STRING) { - $currValue = $this->redis->get($fixKey); - if (is_string($currValue) && preg_match('/^[Oa]:\d+:["{]/', $currValue)) { - $currValue = unserialize($currValue); - } - if (is_object($currValue)) { - $currValue = [$currValue]; - } - $this->redis->del($fixKey); - foreach ((array)$currValue as $items) { - $this->add($key, $items); - } - } + return $this->redis->set($this->fixKey($key), $this->encode($value), $options) !== false; + } + + #[\Override] + public function deleteIfEquals(string $key, mixed $value): bool + { + return (bool)$this->script(self::LUA_DELETE_IF_EQUALS, $key, [$this->encode($value)]); + } + + #[\Override] + public function expireIfEquals(string $key, mixed $value, DateInterval|int|null $ttl): bool + { + return (bool)$this->script( + self::LUA_EXPIRE_IF_EQUALS, + $key, + [$this->encode($value), $this->ttlInSeconds($ttl)] + ); + } - $result = $this->redis->rPush($fixKey, is_object($value) || is_array($value) ? serialize($value) : $value); + /** Lua has no notion of "no TTL", so the absence of one is passed down as a plain zero. */ + private function ttlInSeconds(DateInterval|int|null $ttl): int + { + $seconds = $this->convertToSeconds($ttl); - return $result ? $this->get($key) : []; + return is_int($seconds) ? $seconds : 0; } } diff --git a/tests/CachePSR16Test.php b/tests/CachePSR16Test.php index fa64c0a..2b99e2a 100644 --- a/tests/CachePSR16Test.php +++ b/tests/CachePSR16Test.php @@ -396,4 +396,36 @@ public function testAtomicAdd(BaseCacheEngine $cacheEngine) $this->markTestIncomplete('Does not support atomic add or it is native'); } } + + /** + * The TTL argument of the atomic operations has to mean the same thing it means everywhere + * else: seconds from now. FileSystem used to hand it straight to the expiry file, which stores + * an absolute timestamp, so a value written with a TTL of 60 expired in January 1970 and the + * key was already unreadable by the time the caller looked at it. + * + * @param BaseCacheEngine $cacheEngine + * @throws \Psr\SimpleCache\InvalidArgumentException + */ + #[DataProvider('CachePoolProvider')] + public function testAtomicOperationsHonourTheTtl(BaseCacheEngine $cacheEngine) + { + $this->cacheEngine = $cacheEngine; + + if ($cacheEngine->isAvailable() && ($cacheEngine instanceof AtomicOperationInterface)) { + $this->assertEquals(1, $cacheEngine->increment('ttl-counter', 1, 30)); + $this->assertEquals(2, $cacheEngine->increment('ttl-counter', 1, 30)); + $this->assertEquals(2, $cacheEngine->get('ttl-counter'), 'A 30s TTL must still be alive'); + + $this->assertEquals(['x'], $cacheEngine->add('ttl-list', 'x', 30)); + $this->assertEquals(['x'], $cacheEngine->get('ttl-list'), 'A 30s TTL must still be alive'); + + $cacheEngine->increment('expiring-counter', 1, 1); + $cacheEngine->add('expiring-list', 'x', 1); + sleep(2); + $this->assertNull($cacheEngine->get('expiring-counter'), 'A 1s TTL must have elapsed'); + $this->assertNull($cacheEngine->get('expiring-list'), 'A 1s TTL must have elapsed'); + } else { + $this->markTestIncomplete('Does not support atomic operations or it is native'); + } + } } diff --git a/tests/CompareAndSwapTest.php b/tests/CompareAndSwapTest.php new file mode 100644 index 0000000..5815260 --- /dev/null +++ b/tests/CompareAndSwapTest.php @@ -0,0 +1,164 @@ + [new RedisCacheEngine()], + 'memcached' => [new MemcachedEngine()], + ]; + } + + #[\Override] + protected function tearDown(): void + { + foreach ($this->started as $engine) { + $engine->delete('cas-key'); + } + $this->started = []; + } + + /** + * Skips instead of failing so the suite still runs without the docker-compose services up, + * which is how the rest of this test suite treats an unreachable backend. + */ + private function engineOrSkip(BaseCacheEngine $engine): CompareAndSwapInterface + { + if (!$engine->isAvailable()) { + $this->markTestSkipped(get_class($engine) . ' is not available'); + } + + $this->assertInstanceOf(CompareAndSwapInterface::class, $engine); + + $this->started[] = $engine; + $engine->delete('cas-key'); + + return $engine; + } + + #[DataProvider('engineProvider')] + public function testSetIfAbsentSucceedsOnlyForTheFirstCaller(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $this->assertTrue($cas->setIfAbsent('cas-key', 'token-a', 30)); + $this->assertFalse($cas->setIfAbsent('cas-key', 'token-b', 30)); + $this->assertEquals('token-a', $engine->get('cas-key'), 'The loser must not have overwritten the winner'); + } + + #[DataProvider('engineProvider')] + public function testSetIfAbsentSucceedsAgainOnceTheTtlHasPassed(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $this->assertTrue($cas->setIfAbsent('cas-key', 'token-a', 1)); + $this->assertFalse($cas->setIfAbsent('cas-key', 'token-b', 1)); + + sleep(2); + + $this->assertTrue($cas->setIfAbsent('cas-key', 'token-b', 30)); + $this->assertEquals('token-b', $engine->get('cas-key')); + } + + #[DataProvider('engineProvider')] + public function testSetIfAbsentAppliesTheTtlWithTheWrite(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $cas->setIfAbsent('cas-key', 'token-a', 1); + sleep(2); + + $this->assertNull($engine->get('cas-key'), 'The key must expire on its own, with no second command'); + } + + #[DataProvider('engineProvider')] + public function testDeleteIfEqualsOnlyRemovesTheMatchingValue(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $cas->setIfAbsent('cas-key', 'token-a', 30); + + $this->assertFalse($cas->deleteIfEquals('cas-key', 'token-b')); + $this->assertEquals('token-a', $engine->get('cas-key'), 'A non-matching caller must not delete'); + + $this->assertTrue($cas->deleteIfEquals('cas-key', 'token-a')); + $this->assertNull($engine->get('cas-key')); + } + + #[DataProvider('engineProvider')] + public function testDeleteIfEqualsOnAMissingKeyReturnsFalse(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $this->assertFalse($cas->deleteIfEquals('cas-key', 'token-a')); + } + + /** + * The scenario the interface exists for: an owner whose TTL quietly lapsed must not be able to + * delete the key that somebody else has legitimately taken over in the meantime. + */ + #[DataProvider('engineProvider')] + public function testAnExpiredOwnerCannotDeleteTheNewOwnersValue(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $cas->setIfAbsent('cas-key', 'stale-owner', 1); + sleep(2); + $this->assertTrue($cas->setIfAbsent('cas-key', 'new-owner', 30)); + + $this->assertFalse($cas->deleteIfEquals('cas-key', 'stale-owner')); + $this->assertFalse($cas->expireIfEquals('cas-key', 'stale-owner', 30)); + $this->assertEquals('new-owner', $engine->get('cas-key')); + } + + #[DataProvider('engineProvider')] + public function testExpireIfEqualsExtendsTheLifetimeOfAMatchingValue(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $cas->setIfAbsent('cas-key', 'token-a', 1); + $this->assertTrue($cas->expireIfEquals('cas-key', 'token-a', 30)); + + sleep(2); + + $this->assertEquals('token-a', $engine->get('cas-key'), 'The original 1s expiry must have been replaced'); + } + + #[DataProvider('engineProvider')] + public function testExpireIfEqualsIsRejectedForANonMatchingValue(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $cas->setIfAbsent('cas-key', 'token-a', 1); + $this->assertFalse($cas->expireIfEquals('cas-key', 'token-b', 30)); + + sleep(2); + + $this->assertNull($engine->get('cas-key'), 'The rejected refresh must not have extended anything'); + } + + #[DataProvider('engineProvider')] + public function testSetIfAbsentRoundTripsNonScalarValues(BaseCacheEngine $engine): void + { + $cas = $this->engineOrSkip($engine); + + $this->assertTrue($cas->setIfAbsent('cas-key', ['a' => 1, 'b' => 2], 30)); + $this->assertEquals(['a' => 1, 'b' => 2], $engine->get('cas-key')); + + $this->assertFalse($cas->deleteIfEquals('cas-key', ['a' => 1])); + $this->assertTrue($cas->deleteIfEquals('cas-key', ['a' => 1, 'b' => 2])); + } +} diff --git a/tests/ConcurrencyTest.php b/tests/ConcurrencyTest.php new file mode 100644 index 0000000..e65ff64 --- /dev/null +++ b/tests/ConcurrencyTest.php @@ -0,0 +1,213 @@ + [RedisCacheEngine::class], + 'memcached' => [MemcachedEngine::class], + ]; + } + + #[\Override] + protected function setUp(): void + { + if (!function_exists('pcntl_fork')) { + $this->markTestSkipped('pcntl is required to test concurrent access'); + } + } + + /** + * Runs $child in CHILDREN separate processes, released as simultaneously as we can manage, and + * returns their exit codes. The barrier matters: without it the first child would routinely + * finish before the last one was even forked, and nothing would ever contend. + * + * @return int[] + */ + private function fork(callable $child): array + { + $startAt = microtime(true) + 0.5; + $pids = []; + + for ($i = 0; $i < self::CHILDREN; $i++) { + $pid = pcntl_fork(); + $this->assertNotSame(-1, $pid, 'Could not fork'); + + if ($pid === 0) { + $status = 1; + try { + usleep((int)max(0.0, ($startAt - microtime(true)) * 1_000_000.0)); + $status = $child($i) ? 0 : 1; + } finally { + exit($status); + } + } + + $pids[] = $pid; + } + + $codes = []; + foreach ($pids as $pid) { + pcntl_waitpid($pid, $status); + $codes[] = pcntl_wifexited($status) ? pcntl_wexitstatus($status) : -1; + } + + return $codes; + } + + /** + * Always hands back a freshly connected engine, and must be called again after fork() to read + * back results: the children inherit the parent's open socket and close it as they exit, which + * leaves the parent's own connection dead. That is a property of forking, not of the engine. + */ + private function engineOrSkip(string $engineClass): BaseCacheEngine + { + $engine = new $engineClass(); + if (!$engine->isAvailable()) { + $this->markTestSkipped("$engineClass is not available"); + } + + return $engine; + } + + /** + * The defining property of setIfAbsent: N processes race for an absent key, exactly one wins. + * A has()-then-set() implementation lets several win here. + */ + #[DataProvider('engineProvider')] + public function testExactlyOneProcessWinsSetIfAbsent(string $engineClass): void + { + $this->engineOrSkip($engineClass)->delete('race-lock'); + + $codes = $this->fork(function (int $i) use ($engineClass): bool { + /** @var CompareAndSwapInterface $engine */ + $engine = new $engineClass(); + return $engine->setIfAbsent('race-lock', "token-$i", 60); + }); + + $winners = count(array_filter($codes, fn($code) => $code === 0)); + $this->assertSame(1, $winners, "Expected exactly one winner, got $winners"); + } + + /** + * increment() must never hand the same number to two callers. This is the test that fails + * against a get()-then-set() seed: the losing process's set(0) lands after the winner's + * increment and resets the counter, so the value 1 gets issued twice. + */ + #[DataProvider('engineProvider')] + public function testIncrementNeverIssuesTheSameValueTwice(string $engineClass): void + { + $this->engineOrSkip($engineClass)->delete('race-counter'); + + $this->fork(function () use ($engineClass): bool { + $engine = new $engineClass(); + $engine->increment('race-counter'); + return true; + }); + + $this->assertEquals( + self::CHILDREN, + $this->engineOrSkip($engineClass)->get('race-counter'), + 'Every increment must be reflected exactly once in the final counter' + ); + } + + /** + * Concurrent appends must all survive. A read-modify-write without a compare loses whichever + * updates landed between another process's read and its write. + */ + #[DataProvider('engineProvider')] + public function testConcurrentAddLosesNothing(string $engineClass): void + { + $this->engineOrSkip($engineClass)->delete('race-list'); + + $this->fork(function (int $i) use ($engineClass): bool { + $engine = new $engineClass(); + $engine->add('race-list', "item-$i"); + return true; + }); + + $stored = $this->engineOrSkip($engineClass)->get('race-list'); + $this->assertIsArray($stored); + $this->assertCount(self::CHILDREN, $stored, 'Every appended item must still be present'); + + sort($stored); + $expected = array_map(fn($i) => "item-$i", range(0, self::CHILDREN - 1)); + sort($expected); + $this->assertEquals($expected, $stored, 'No item may be dropped or duplicated'); + } + + /** + * The first add() to a key that set() wrote has to convert it from a plain value into a list. + * Doing that as read-delete-rewrite means concurrent callers each delete a key the others are + * mid-way through rebuilding, which both duplicates the original value and drops appends. + */ + #[DataProvider('engineProvider')] + public function testConcurrentAddSurvivesTheConversionFromAPlainValue(string $engineClass): void + { + $setup = $this->engineOrSkip($engineClass); + $setup->delete('race-convert'); + $setup->set('race-convert', 'seed'); + + $this->fork(function (int $i) use ($engineClass): bool { + $engine = new $engineClass(); + $engine->add('race-convert', "item-$i"); + return true; + }); + + $stored = $this->engineOrSkip($engineClass)->get('race-convert'); + $this->assertIsArray($stored); + + sort($stored); + $expected = array_merge(['seed'], array_map(fn($i) => "item-$i", range(0, self::CHILDREN - 1))); + sort($expected); + $this->assertEquals($expected, $stored, 'The seed must appear exactly once and no append may be lost'); + } + + /** + * The whole point of a token-guarded release: only the process that actually owns the key may + * remove it, no matter how many others try at the same instant. + */ + #[DataProvider('engineProvider')] + public function testOnlyTheOwnerCanDeleteUnderContention(string $engineClass): void + { + /** @var BaseCacheEngine&CompareAndSwapInterface $setup */ + $setup = $this->engineOrSkip($engineClass); + $setup->delete('race-owned'); + $setup->setIfAbsent('race-owned', 'the-owner', 60); + + $codes = $this->fork(function (int $i) use ($engineClass): bool { + /** @var CompareAndSwapInterface $engine */ + $engine = new $engineClass(); + return $engine->deleteIfEquals('race-owned', "impostor-$i"); + }); + + $this->assertSame(0, count(array_filter($codes, fn($code) => $code === 0)), 'No impostor may succeed'); + + $verify = $this->engineOrSkip($engineClass); + $this->assertEquals('the-owner', $verify->get('race-owned'), 'The key must be untouched'); + $verify->delete('race-owned'); + } +} From a7fdfa09270053bb73bb258483bdc4391893f134 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 7 Aug 2026 10:32:19 -0400 Subject: [PATCH 02/15] Add changelog for version 6.0 --- CHANGELOG-6.0.md | 152 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) create mode 100644 CHANGELOG-6.0.md diff --git a/CHANGELOG-6.0.md b/CHANGELOG-6.0.md new file mode 100644 index 0000000..f57f9eb --- /dev/null +++ b/CHANGELOG-6.0.md @@ -0,0 +1,152 @@ +# Changelog - Version 6.0 + +## Overview + +Version 6.0 represents a major update focused on modernizing the codebase for PHP 8.3+ compatibility, improving code quality with strict typing, and enhancing developer experience with updated tooling. + +## Breaking Changes + +| Area | Before (5.x) | After (6.0) | Description | +|------|--------------|-------------|-------------| +| **PHP Version** | `>=8.1 <8.4` | `>=8.3 <8.6` | Minimum PHP version raised from 8.1 to 8.3. PHP 8.1 and 8.2 are no longer supported. Added support for PHP 8.4 and 8.5. | +| **PHPUnit** | `^9.6` | `^10.5\|^11.5` | PHPUnit 9.6 support dropped. Minimum version is now 10.5. Added support for PHPUnit 11.5. | +| **PSR-3 Logger** | `^1.0\|^1.1\|^2.0` | `^1.0\|^2.0\|^3.0` | Added PSR-3 version 3.0 support. Removed version 1.1 constraint (covered by 1.0). | +| **Psalm** | `^5.9` | `^5.9\|^6.13` | Added support for Psalm 6.13 while maintaining backward compatibility with 5.9. | + +## New Features + +### Code Quality Enhancements +- **PHP 8.3+ Override Attribute**: Added `#[Override]` attributes to all overridden methods across the codebase for better code clarity and IDE support +- **Strict Typing**: Enhanced type declarations throughout the codebase for improved type safety +- **Psalm SARIF Reporting**: Added SARIF (Static Analysis Results Interchange Format) output support for better CI/CD integration + +### Developer Experience +- **Composer Scripts**: Added convenient composer scripts: + - `composer test` - Run PHPUnit tests + - `composer psalm` - Run Psalm static analysis with single thread for stability +- **Gitpod Support**: Added `.gitpod.yml` configuration for cloud-based development environment +- **VSCode Configuration**: Added `.vscode/launch.json` with debugging configurations +- **Improved Documentation**: Enhanced all documentation files with better examples and clearer explanations + +### Testing Improvements +- **Test Class Refactoring**: Reorganized test class hierarchy for better maintainability + - Introduced `TestBase` class as the foundation for all cache tests + - Renamed test classes for clarity and consistency + - Migrated PHPUnit data providers to PHP 8.1+ syntax +- **Enhanced CI/CD**: Updated GitHub Actions workflows with better PHP version matrix testing + +### Engine Improvements +- **Consistent Key Handling**: Fixed key consistency issues in `MemcachedEngine` for more reliable caching +- **FileSystemCacheEngine**: Improved path handling and directory creation logic + +## Bug Fixes + +- Fixed unit test issues related to session handling in GitHub Actions environment +- Improved Memcached availability testing in CI/CD pipelines +- Fixed test execution issues requiring `--stderr` parameter for SessionCacheEngine tests +- Enhanced error handling and edge cases in various cache engines + +## Documentation Updates + +All documentation files have been updated to reflect version 6.0 changes: +- Updated code examples to use PHP 8.3+ syntax +- Improved atomic operations documentation +- Enhanced PSR-16 and PSR-6 usage guides +- Updated all engine-specific documentation pages +- Refreshed README with clearer quick start examples +- Added mermaid diagrams for dependency visualization + +## Migration Path from 5.x to 6.0 + +### Step 1: Update PHP Version +Ensure your environment is running PHP 8.3 or later: +```bash +php -v # Should show 8.3.x, 8.4.x, or 8.5.x +``` + +If you're on PHP 8.1 or 8.2, you must upgrade your PHP version before migrating to version 6.0. + +### Step 2: Update Dependencies +Update your `composer.json`: +```bash +composer require byjg/cache-engine:^6.0 +composer update +``` + +### Step 3: Update Development Dependencies (Optional) +If you're using PHPUnit or Psalm in your project: + +**For PHPUnit:** +```bash +composer require --dev phpunit/phpunit:^10.5 +# or +composer require --dev phpunit/phpunit:^11.5 +``` + +**For Psalm:** +```bash +composer require --dev vimeo/psalm:^6.13 +``` + +### Step 4: Test Your Application +Run your existing tests to ensure compatibility: +```bash +vendor/bin/phpunit +``` + +### Step 5: Optional Enhancements +Consider adding the `#[Override]` attribute to your own classes that extend cache engines for better IDE support and code clarity: + +```php +class MyCustomCache extends BaseCacheEngine +{ + #[Override] + public function get(string $key, mixed $default = null): mixed + { + // Your implementation + } +} +``` + +### Step 6: Update CI/CD Pipelines +Update your CI/CD configuration to use PHP 8.3+ in your testing matrix. Remove PHP 8.1 and 8.2 from your test matrix. + +### Common Migration Issues + +**Issue**: Application fails with "PHP version requirement not satisfied" +**Solution**: Upgrade your PHP version to 8.3 or later + +**Issue**: PHPUnit tests fail to run +**Solution**: Upgrade PHPUnit to version 10.5 or later: `composer require --dev phpunit/phpunit:^10.5` + +**Issue**: Psalm reports new errors +**Solution**: If using Psalm 6.x, review and address the stricter type checking. You can temporarily stay on Psalm 5.9 during migration. + +### Testing Your Migration +After completing the migration steps, verify everything works: + +```bash +# Run tests +composer test + +# Run static analysis +composer psalm + +# If using docker-compose +docker compose up -d +composer test +docker compose down +``` + +## Notes + +- All cache engines maintain backward compatibility at the API level +- No changes to PSR-6 or PSR-16 interface implementations +- Existing cache data remains compatible across versions +- The upgrade primarily affects development-time requirements (PHP version, testing tools) + +## Links + +- [Full Commit History](https://github.com/byjg/php-cache-engine/compare/5.0.4...6.0.0) +- [Documentation](https://github.com/byjg/php-cache-engine/tree/master/docs) +- [Report Issues](https://github.com/byjg/php-cache-engine/issues) From 8dfa45f6d7ea5f21e207d4737d7a9247b7d03c69 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 7 Aug 2026 10:38:39 -0400 Subject: [PATCH 03/15] Allow psr/simple-cache 3.0 The engines already declare the PSR-16 3.0 signatures - get(string $key, mixed $default = null): mixed - so the constraint excluding ^3.0 kept the package from installing alongside anything that requires it. Drop ^1.0 at the same time. PSR-16 1.0 declares get($key, $default = null) with untyped parameters, and adding a type to a parameter the interface leaves untyped is a fatal declaration error, so that branch of the constraint was never usable. --- composer.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/composer.json b/composer.json index 166244d..ac118e7 100644 --- a/composer.json +++ b/composer.json @@ -15,7 +15,7 @@ "php": ">=8.3 <8.6", "psr/cache": "^1.0|^2.0|^3.0", "psr/log": "^1.0|^2.0|^3.0", - "psr/simple-cache": "^1.0|^2.0", + "psr/simple-cache": "^2.0|^3.0", "psr/container": "^1.0|^1.1|^2.0" }, "require-dev": { @@ -29,7 +29,7 @@ }, "provide": { "psr/cache-implementation": "1.0", - "psr/simple-cache-implementation": "1.0" + "psr/simple-cache-implementation": "2.0|3.0" }, "scripts": { "test": "vendor/bin/phpunit", From bd180d8a300284f207d65bb83e198f8bd6917d71 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 7 Aug 2026 10:49:42 -0400 Subject: [PATCH 04/15] Connect lazily in RedisCacheEngine::has() and clear() Both read $this->redis without calling lazyLoadRedisServer() first, so either one used as the very first operation on a new instance died with "Call to a member function exists() on null". Every other public method already established the connection; these two were simply missed. Surfaced by calling has() as the opening operation against a fresh engine. --- src/Psr16/RedisCacheEngine.php | 4 ++++ tests/CachePSR16Test.php | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/src/Psr16/RedisCacheEngine.php b/src/Psr16/RedisCacheEngine.php index 92f7aee..d873fd5 100644 --- a/src/Psr16/RedisCacheEngine.php +++ b/src/Psr16/RedisCacheEngine.php @@ -233,6 +233,8 @@ public function delete(string $key): bool #[\Override] public function clear(): bool { + $this->lazyLoadRedisServer(); + $iterator = null; do { $keys = $this->redis->scan($iterator, 'cache:*'); @@ -252,6 +254,8 @@ public function clear(): bool #[\Override] public function has(string $key): bool { + $this->lazyLoadRedisServer(); + $result = $this->redis->exists($this->fixKey($key)); return (bool)$result; } diff --git a/tests/CachePSR16Test.php b/tests/CachePSR16Test.php index 2b99e2a..a1c7530 100644 --- a/tests/CachePSR16Test.php +++ b/tests/CachePSR16Test.php @@ -6,7 +6,9 @@ use ByJG\Cache\Exception\InvalidArgumentException; use ByJG\Cache\GarbageCollectorInterface; use ByJG\Cache\Psr16\BaseCacheEngine; +use ByJG\Cache\Psr16\MemcachedEngine; use ByJG\Cache\Psr16\NoCacheEngine; +use ByJG\Cache\Psr16\RedisCacheEngine; use PHPUnit\Framework\Attributes\DataProvider; class CachePSR16Test extends TestBase @@ -397,6 +399,27 @@ public function testAtomicAdd(BaseCacheEngine $cacheEngine) } } + /** + * Every public method has to establish the connection on its own. RedisCacheEngine::has() and + * clear() read $this->redis without calling lazyLoadRedisServer() first, so either one used as + * the very first operation on a new instance died with "call to a member function on null". + * + * Each assertion needs an instance that has never been touched, so the engines are constructed + * here rather than taken from the shared data provider. + */ + public function testEveryEntryPointConnectsOnItsOwn() + { + if (!(new RedisCacheEngine())->isAvailable()) { + $this->markTestSkipped('Redis is not available'); + } + + $this->assertFalse((new RedisCacheEngine())->has('never-touched-key')); + $this->assertTrue((new RedisCacheEngine())->clear()); + + $this->assertFalse((new MemcachedEngine())->has('never-touched-key')); + $this->assertTrue((new MemcachedEngine())->clear()); + } + /** * The TTL argument of the atomic operations has to mean the same thing it means everywhere * else: seconds from now. FileSystem used to hand it straight to the expiry file, which stores From c59a7babcf2cda739d1270f786704c8ec4df543c Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 7 Aug 2026 11:07:12 -0400 Subject: [PATCH 05/15] Document the simple-cache constraint and lazy connect fixes Both landed after the changelog was written. Listed under Bug Fixes rather than Breaking Changes: widening psr/simple-cache to allow 3.0 only unblocks installs, and the dropped ^1.0 branch could never load in the first place. --- CHANGELOG-7.0.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md index 579be28..f4dd56f 100644 --- a/CHANGELOG-7.0.md +++ b/CHANGELOG-7.0.md @@ -93,6 +93,26 @@ process already converted it. `increment()` and `decrement()` set the expiry with a follow-up `EXPIRE`. A crash between the two left a counter with no expiry at all. Both now apply it inside the script. +### `psr/simple-cache` 3.0 is now allowed + +The engines already declare the PSR-16 3.0 signatures, but the constraint stopped at `^2.0`, so the +package could not be installed alongside anything requiring `psr/simple-cache ^3.0`. The constraint +is now `^2.0|^3.0`. + +### Redis: `has()` and `clear()` never opened the connection + +Both read `$this->redis` without calling `lazyLoadRedisServer()` first. Every other public method +established the connection; these two were missed. Calling either as the first operation on a new +instance died with: + +``` +Error: Call to a member function exists() on null +``` + +It went unnoticed because in practice something else — `isAvailable()`, `get()`, `set()` — almost +always ran first and left the connection open. A regression test now exercises each entry point on +an instance that has never been touched. + ### FileSystem: expiry dropped outside the lock `putContents()` deleted the `.ttl` file before acquiring the lock, leaving the value briefly From da4032db601bb7cfea0fc632296c8f76c9e01b62 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 7 Aug 2026 11:28:37 -0400 Subject: [PATCH 06/15] Build test engines from the environment, not from hardcoded localhost The compare-and-swap and concurrency tests constructed their engines with no arguments, which points them at 127.0.0.1. CI runs the job inside a container, where the service containers are reachable by service name on the Docker network rather than on loopback, so both classes skipped themselves for the whole matrix and the build went green having never exercised a real Redis or Memcached. TestBase already read REDIS_SERVER and MEMCACHED_SERVER for its data provider. That logic now lives in EngineFactory and every test goes through it. make() returns BaseCacheEngine&AtomicOperationInterface&CompareAndSwapInterface, which is enough for the call sites to drop their local @var annotations and carry one engine variable instead of two. --- tests/CompareAndSwapTest.php | 89 +++++++++++++++++------------------- tests/ConcurrencyTest.php | 63 ++++++++++++------------- tests/EngineFactory.php | 45 ++++++++++++++++++ tests/TestBase.php | 22 +-------- 4 files changed, 119 insertions(+), 100 deletions(-) create mode 100644 tests/EngineFactory.php diff --git a/tests/CompareAndSwapTest.php b/tests/CompareAndSwapTest.php index 5815260..33f765c 100644 --- a/tests/CompareAndSwapTest.php +++ b/tests/CompareAndSwapTest.php @@ -4,8 +4,6 @@ use ByJG\Cache\CompareAndSwapInterface; use ByJG\Cache\Psr16\BaseCacheEngine; -use ByJG\Cache\Psr16\MemcachedEngine; -use ByJG\Cache\Psr16\RedisCacheEngine; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; @@ -17,8 +15,8 @@ class CompareAndSwapTest extends TestCase public static function engineProvider(): array { return [ - 'redis' => [new RedisCacheEngine()], - 'memcached' => [new MemcachedEngine()], + 'redis' => [EngineFactory::REDIS], + 'memcached' => [EngineFactory::MEMCACHED], ]; } @@ -35,14 +33,13 @@ protected function tearDown(): void * Skips instead of failing so the suite still runs without the docker-compose services up, * which is how the rest of this test suite treats an unreachable backend. */ - private function engineOrSkip(BaseCacheEngine $engine): CompareAndSwapInterface + private function engineOrSkip(string $engineName): BaseCacheEngine&CompareAndSwapInterface { + $engine = EngineFactory::make($engineName); if (!$engine->isAvailable()) { $this->markTestSkipped(get_class($engine) . ' is not available'); } - $this->assertInstanceOf(CompareAndSwapInterface::class, $engine); - $this->started[] = $engine; $engine->delete('cas-key'); @@ -50,60 +47,60 @@ private function engineOrSkip(BaseCacheEngine $engine): CompareAndSwapInterface } #[DataProvider('engineProvider')] - public function testSetIfAbsentSucceedsOnlyForTheFirstCaller(BaseCacheEngine $engine): void + public function testSetIfAbsentSucceedsOnlyForTheFirstCaller(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $this->assertTrue($cas->setIfAbsent('cas-key', 'token-a', 30)); - $this->assertFalse($cas->setIfAbsent('cas-key', 'token-b', 30)); + $this->assertTrue($engine->setIfAbsent('cas-key', 'token-a', 30)); + $this->assertFalse($engine->setIfAbsent('cas-key', 'token-b', 30)); $this->assertEquals('token-a', $engine->get('cas-key'), 'The loser must not have overwritten the winner'); } #[DataProvider('engineProvider')] - public function testSetIfAbsentSucceedsAgainOnceTheTtlHasPassed(BaseCacheEngine $engine): void + public function testSetIfAbsentSucceedsAgainOnceTheTtlHasPassed(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $this->assertTrue($cas->setIfAbsent('cas-key', 'token-a', 1)); - $this->assertFalse($cas->setIfAbsent('cas-key', 'token-b', 1)); + $this->assertTrue($engine->setIfAbsent('cas-key', 'token-a', 1)); + $this->assertFalse($engine->setIfAbsent('cas-key', 'token-b', 1)); sleep(2); - $this->assertTrue($cas->setIfAbsent('cas-key', 'token-b', 30)); + $this->assertTrue($engine->setIfAbsent('cas-key', 'token-b', 30)); $this->assertEquals('token-b', $engine->get('cas-key')); } #[DataProvider('engineProvider')] - public function testSetIfAbsentAppliesTheTtlWithTheWrite(BaseCacheEngine $engine): void + public function testSetIfAbsentAppliesTheTtlWithTheWrite(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $cas->setIfAbsent('cas-key', 'token-a', 1); + $engine->setIfAbsent('cas-key', 'token-a', 1); sleep(2); $this->assertNull($engine->get('cas-key'), 'The key must expire on its own, with no second command'); } #[DataProvider('engineProvider')] - public function testDeleteIfEqualsOnlyRemovesTheMatchingValue(BaseCacheEngine $engine): void + public function testDeleteIfEqualsOnlyRemovesTheMatchingValue(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $cas->setIfAbsent('cas-key', 'token-a', 30); + $engine->setIfAbsent('cas-key', 'token-a', 30); - $this->assertFalse($cas->deleteIfEquals('cas-key', 'token-b')); + $this->assertFalse($engine->deleteIfEquals('cas-key', 'token-b')); $this->assertEquals('token-a', $engine->get('cas-key'), 'A non-matching caller must not delete'); - $this->assertTrue($cas->deleteIfEquals('cas-key', 'token-a')); + $this->assertTrue($engine->deleteIfEquals('cas-key', 'token-a')); $this->assertNull($engine->get('cas-key')); } #[DataProvider('engineProvider')] - public function testDeleteIfEqualsOnAMissingKeyReturnsFalse(BaseCacheEngine $engine): void + public function testDeleteIfEqualsOnAMissingKeyReturnsFalse(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $this->assertFalse($cas->deleteIfEquals('cas-key', 'token-a')); + $this->assertFalse($engine->deleteIfEquals('cas-key', 'token-a')); } /** @@ -111,26 +108,26 @@ public function testDeleteIfEqualsOnAMissingKeyReturnsFalse(BaseCacheEngine $eng * delete the key that somebody else has legitimately taken over in the meantime. */ #[DataProvider('engineProvider')] - public function testAnExpiredOwnerCannotDeleteTheNewOwnersValue(BaseCacheEngine $engine): void + public function testAnExpiredOwnerCannotDeleteTheNewOwnersValue(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $cas->setIfAbsent('cas-key', 'stale-owner', 1); + $engine->setIfAbsent('cas-key', 'stale-owner', 1); sleep(2); - $this->assertTrue($cas->setIfAbsent('cas-key', 'new-owner', 30)); + $this->assertTrue($engine->setIfAbsent('cas-key', 'new-owner', 30)); - $this->assertFalse($cas->deleteIfEquals('cas-key', 'stale-owner')); - $this->assertFalse($cas->expireIfEquals('cas-key', 'stale-owner', 30)); + $this->assertFalse($engine->deleteIfEquals('cas-key', 'stale-owner')); + $this->assertFalse($engine->expireIfEquals('cas-key', 'stale-owner', 30)); $this->assertEquals('new-owner', $engine->get('cas-key')); } #[DataProvider('engineProvider')] - public function testExpireIfEqualsExtendsTheLifetimeOfAMatchingValue(BaseCacheEngine $engine): void + public function testExpireIfEqualsExtendsTheLifetimeOfAMatchingValue(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $cas->setIfAbsent('cas-key', 'token-a', 1); - $this->assertTrue($cas->expireIfEquals('cas-key', 'token-a', 30)); + $engine->setIfAbsent('cas-key', 'token-a', 1); + $this->assertTrue($engine->expireIfEquals('cas-key', 'token-a', 30)); sleep(2); @@ -138,12 +135,12 @@ public function testExpireIfEqualsExtendsTheLifetimeOfAMatchingValue(BaseCacheEn } #[DataProvider('engineProvider')] - public function testExpireIfEqualsIsRejectedForANonMatchingValue(BaseCacheEngine $engine): void + public function testExpireIfEqualsIsRejectedForANonMatchingValue(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $cas->setIfAbsent('cas-key', 'token-a', 1); - $this->assertFalse($cas->expireIfEquals('cas-key', 'token-b', 30)); + $engine->setIfAbsent('cas-key', 'token-a', 1); + $this->assertFalse($engine->expireIfEquals('cas-key', 'token-b', 30)); sleep(2); @@ -151,14 +148,14 @@ public function testExpireIfEqualsIsRejectedForANonMatchingValue(BaseCacheEngine } #[DataProvider('engineProvider')] - public function testSetIfAbsentRoundTripsNonScalarValues(BaseCacheEngine $engine): void + public function testSetIfAbsentRoundTripsNonScalarValues(string $engineName): void { - $cas = $this->engineOrSkip($engine); + $engine = $this->engineOrSkip($engineName); - $this->assertTrue($cas->setIfAbsent('cas-key', ['a' => 1, 'b' => 2], 30)); + $this->assertTrue($engine->setIfAbsent('cas-key', ['a' => 1, 'b' => 2], 30)); $this->assertEquals(['a' => 1, 'b' => 2], $engine->get('cas-key')); - $this->assertFalse($cas->deleteIfEquals('cas-key', ['a' => 1])); - $this->assertTrue($cas->deleteIfEquals('cas-key', ['a' => 1, 'b' => 2])); + $this->assertFalse($engine->deleteIfEquals('cas-key', ['a' => 1])); + $this->assertTrue($engine->deleteIfEquals('cas-key', ['a' => 1, 'b' => 2])); } } diff --git a/tests/ConcurrencyTest.php b/tests/ConcurrencyTest.php index e65ff64..61ba1a6 100644 --- a/tests/ConcurrencyTest.php +++ b/tests/ConcurrencyTest.php @@ -4,8 +4,6 @@ use ByJG\Cache\CompareAndSwapInterface; use ByJG\Cache\Psr16\BaseCacheEngine; -use ByJG\Cache\Psr16\MemcachedEngine; -use ByJG\Cache\Psr16\RedisCacheEngine; use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\Attributes\Group; use PHPUnit\Framework\TestCase; @@ -26,8 +24,8 @@ class ConcurrencyTest extends TestCase public static function engineProvider(): array { return [ - 'redis' => [RedisCacheEngine::class], - 'memcached' => [MemcachedEngine::class], + 'redis' => [EngineFactory::REDIS], + 'memcached' => [EngineFactory::MEMCACHED], ]; } @@ -82,11 +80,11 @@ private function fork(callable $child): array * back results: the children inherit the parent's open socket and close it as they exit, which * leaves the parent's own connection dead. That is a property of forking, not of the engine. */ - private function engineOrSkip(string $engineClass): BaseCacheEngine + private function engineOrSkip(string $engineName): BaseCacheEngine&CompareAndSwapInterface { - $engine = new $engineClass(); + $engine = EngineFactory::make($engineName); if (!$engine->isAvailable()) { - $this->markTestSkipped("$engineClass is not available"); + $this->markTestSkipped("$engineName is not available"); } return $engine; @@ -97,13 +95,12 @@ private function engineOrSkip(string $engineClass): BaseCacheEngine * A has()-then-set() implementation lets several win here. */ #[DataProvider('engineProvider')] - public function testExactlyOneProcessWinsSetIfAbsent(string $engineClass): void + public function testExactlyOneProcessWinsSetIfAbsent(string $engineName): void { - $this->engineOrSkip($engineClass)->delete('race-lock'); + $this->engineOrSkip($engineName)->delete('race-lock'); - $codes = $this->fork(function (int $i) use ($engineClass): bool { - /** @var CompareAndSwapInterface $engine */ - $engine = new $engineClass(); + $codes = $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); return $engine->setIfAbsent('race-lock', "token-$i", 60); }); @@ -117,19 +114,19 @@ public function testExactlyOneProcessWinsSetIfAbsent(string $engineClass): void * increment and resets the counter, so the value 1 gets issued twice. */ #[DataProvider('engineProvider')] - public function testIncrementNeverIssuesTheSameValueTwice(string $engineClass): void + public function testIncrementNeverIssuesTheSameValueTwice(string $engineName): void { - $this->engineOrSkip($engineClass)->delete('race-counter'); + $this->engineOrSkip($engineName)->delete('race-counter'); - $this->fork(function () use ($engineClass): bool { - $engine = new $engineClass(); + $this->fork(function () use ($engineName): bool { + $engine = EngineFactory::make($engineName); $engine->increment('race-counter'); return true; }); $this->assertEquals( self::CHILDREN, - $this->engineOrSkip($engineClass)->get('race-counter'), + $this->engineOrSkip($engineName)->get('race-counter'), 'Every increment must be reflected exactly once in the final counter' ); } @@ -139,17 +136,17 @@ public function testIncrementNeverIssuesTheSameValueTwice(string $engineClass): * updates landed between another process's read and its write. */ #[DataProvider('engineProvider')] - public function testConcurrentAddLosesNothing(string $engineClass): void + public function testConcurrentAddLosesNothing(string $engineName): void { - $this->engineOrSkip($engineClass)->delete('race-list'); + $this->engineOrSkip($engineName)->delete('race-list'); - $this->fork(function (int $i) use ($engineClass): bool { - $engine = new $engineClass(); + $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); $engine->add('race-list', "item-$i"); return true; }); - $stored = $this->engineOrSkip($engineClass)->get('race-list'); + $stored = $this->engineOrSkip($engineName)->get('race-list'); $this->assertIsArray($stored); $this->assertCount(self::CHILDREN, $stored, 'Every appended item must still be present'); @@ -165,19 +162,19 @@ public function testConcurrentAddLosesNothing(string $engineClass): void * mid-way through rebuilding, which both duplicates the original value and drops appends. */ #[DataProvider('engineProvider')] - public function testConcurrentAddSurvivesTheConversionFromAPlainValue(string $engineClass): void + public function testConcurrentAddSurvivesTheConversionFromAPlainValue(string $engineName): void { - $setup = $this->engineOrSkip($engineClass); + $setup = $this->engineOrSkip($engineName); $setup->delete('race-convert'); $setup->set('race-convert', 'seed'); - $this->fork(function (int $i) use ($engineClass): bool { - $engine = new $engineClass(); + $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); $engine->add('race-convert', "item-$i"); return true; }); - $stored = $this->engineOrSkip($engineClass)->get('race-convert'); + $stored = $this->engineOrSkip($engineName)->get('race-convert'); $this->assertIsArray($stored); sort($stored); @@ -191,22 +188,20 @@ public function testConcurrentAddSurvivesTheConversionFromAPlainValue(string $en * remove it, no matter how many others try at the same instant. */ #[DataProvider('engineProvider')] - public function testOnlyTheOwnerCanDeleteUnderContention(string $engineClass): void + public function testOnlyTheOwnerCanDeleteUnderContention(string $engineName): void { - /** @var BaseCacheEngine&CompareAndSwapInterface $setup */ - $setup = $this->engineOrSkip($engineClass); + $setup = $this->engineOrSkip($engineName); $setup->delete('race-owned'); $setup->setIfAbsent('race-owned', 'the-owner', 60); - $codes = $this->fork(function (int $i) use ($engineClass): bool { - /** @var CompareAndSwapInterface $engine */ - $engine = new $engineClass(); + $codes = $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); return $engine->deleteIfEquals('race-owned', "impostor-$i"); }); $this->assertSame(0, count(array_filter($codes, fn($code) => $code === 0)), 'No impostor may succeed'); - $verify = $this->engineOrSkip($engineClass); + $verify = $this->engineOrSkip($engineName); $this->assertEquals('the-owner', $verify->get('race-owned'), 'The key must be untouched'); $verify->delete('race-owned'); } diff --git a/tests/EngineFactory.php b/tests/EngineFactory.php new file mode 100644 index 0000000..1032169 --- /dev/null +++ b/tests/EngineFactory.php @@ -0,0 +1,45 @@ + self::redis(), + self::MEMCACHED => self::memcached(), + default => throw new \InvalidArgumentException("Unknown engine '$engine'"), + }; + } + + public static function redis(): RedisCacheEngine + { + return new RedisCacheEngine( + getenv('REDIS_SERVER') ?: '127.0.0.1:6379', + getenv('REDIS_PASSWORD') ?: '' + ); + } + + public static function memcached(): MemcachedEngine + { + return new MemcachedEngine([getenv('MEMCACHED_SERVER') ?: '127.0.0.1:11211']); + } +} diff --git a/tests/TestBase.php b/tests/TestBase.php index 0e8d2f9..9497438 100644 --- a/tests/TestBase.php +++ b/tests/TestBase.php @@ -5,9 +5,7 @@ use ByJG\Cache\Psr16\ArrayCacheEngine; use ByJG\Cache\Psr16\BaseCacheEngine; use ByJG\Cache\Psr16\FileSystemCacheEngine; -use ByJG\Cache\Psr16\MemcachedEngine; use ByJG\Cache\Psr16\NoCacheEngine; -use ByJG\Cache\Psr16\RedisCacheEngine; use ByJG\Cache\Psr16\SessionCacheEngine; use ByJG\Cache\Psr16\ShmopCacheEngine; use ByJG\Cache\Psr16\TmpfsCacheEngine; @@ -32,22 +30,6 @@ protected function tearDown(): void public static function CachePoolProvider() { - if (getenv('MEMCACHED_SERVER')) { - $memcachedServer = [getenv('MEMCACHED_SERVER')]; - } else { - $memcachedServer = ['127.0.0.1:11211']; - } - if (getenv('REDIS_SERVER')) { - $redisCacheServer = getenv('REDIS_SERVER'); - } else { - $redisCacheServer = '127.0.0.1:6379'; - } - if (getenv('REDIS_PASSWORD')) { - $redisPassword = getenv('REDIS_PASSWORD'); - } else { - $redisPassword = ''; - } - return [ 'Array' => [ new ArrayCacheEngine() @@ -68,10 +50,10 @@ public static function CachePoolProvider() new NoCacheEngine() ], 'Memcached' => [ - new MemcachedEngine($memcachedServer) + EngineFactory::memcached() ], 'Redis' => [ - new RedisCacheEngine($redisCacheServer, $redisPassword) + EngineFactory::redis() ], 'Memory' => [ new TmpfsCacheEngine() From b14a384c19e5b6e0bbdeb36705d225f69ac5b6e8 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Sat, 22 Aug 2026 18:22:24 -0400 Subject: [PATCH 07/15] Prepare 7.0: PHP 8.3-8.6, PHPUnit 12.5, Psalm 6.16 Widen the PHP constraint to ">=8.3 <8.7". The previous "<8.6" is exclusive and therefore excluded PHP 8.6. Pin PHPUnit to ^12.5 and Psalm to ^6.16. PHPUnit 13 is not used: it requires PHP >=8.4.1, which breaks the 8.3 floor, and it needs sebastian/diff ^9.0, which stable Psalm 6.16.1 does not accept - that combination silently resolves Psalm to an unreleased 6.x-dev branch. CI: add PHP 8.6 to the build matrix, and move the Psalm job to PHP 8.5, since Psalm 6.16.1 cannot be installed on 8.6. Housekeeping: rename phpunit.xml.dist to phpunit.xml, carry the 6.x changelogs onto this branch, and add CHANGELOG-7.0.md. --- .github/workflows/phpunit.yml | 3 ++- CHANGELOG-7.0.md | 29 ++++++++++++++++++++++++++++- composer.json | 6 +++--- phpunit.xml.dist => phpunit.xml | 0 4 files changed, 33 insertions(+), 5 deletions(-) rename phpunit.xml.dist => phpunit.xml (100%) diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 845ab02..ce0aca4 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -18,6 +18,7 @@ jobs: strategy: matrix: php-version: + - "8.6" - "8.5" - "8.4" - "8.3" @@ -54,7 +55,7 @@ jobs: # for github/codeql-action/upload-sarif to upload SARIF results security-events: write container: - image: byjg/php:8.4-cli + image: byjg/php:8.5-cli options: --user root --privileged steps: diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md index f4dd56f..190ac05 100644 --- a/CHANGELOG-7.0.md +++ b/CHANGELOG-7.0.md @@ -203,10 +203,37 @@ if ($cache->setIfAbsent('lock', $token, 30)) { - `AtomicOperationInterface` is unchanged; existing implementations continue to work untouched. - No changes to the PSR-6 or PSR-16 interface implementations. - Existing cache data remains compatible across versions. -- PHP requirement is unchanged at `>=8.3 <8.6`. +- PHP requirement is now `>=8.3 <8.7` (see Requirements below). ## Links - [Full Commit History](https://github.com/byjg/php-cache-engine/compare/6.0.1...7.0.0) - [Documentation](https://github.com/byjg/php-cache-engine/tree/master/docs) - [Report Issues](https://github.com/byjg/php-cache-engine/issues) + +## Requirements + +- PHP 8.3, 8.4, 8.5 and 8.6 are now supported: `"php": ">=8.3 <8.7"`. + The previous `<8.6` upper bound excluded PHP 8.6, since `<8.6` is exclusive. + +## Toolchain + +- PHPUnit updated to `^12.5`. +- Psalm updated to `^6.16`. + + PHPUnit 13 is deliberately **not** used. It requires PHP `>=8.4.1`, which would + break the 8.3 floor, and it pulls `sebastian/diff ^9.0`, which the newest stable + Psalm (6.16.1) does not accept — that combination silently resolves Psalm to an + unreleased `6.x-dev` branch. Pinning PHPUnit to `^12.5` keeps a single stable + PHPUnit and a single stable Psalm across the whole matrix. + +## Continuous Integration + +- The build matrix now includes PHP 8.6. +- The Psalm job now runs on PHP 8.5. Psalm 6.16.1 declares + `~8.1.31 || ~8.2.27 || ~8.3.16 || ~8.4.3 || ~8.5.0` and therefore cannot be + installed on PHP 8.6. + +## Housekeeping + +- `phpunit.xml.dist` renamed to `phpunit.xml`. diff --git a/composer.json b/composer.json index ac118e7..0b0323d 100644 --- a/composer.json +++ b/composer.json @@ -12,15 +12,15 @@ } }, "require": { - "php": ">=8.3 <8.6", + "php": ">=8.3 <8.7", "psr/cache": "^1.0|^2.0|^3.0", "psr/log": "^1.0|^2.0|^3.0", "psr/simple-cache": "^2.0|^3.0", "psr/container": "^1.0|^1.1|^2.0" }, "require-dev": { - "phpunit/phpunit": "^10.5|^11.5", - "vimeo/psalm": "^5.9|^6.13" + "phpunit/phpunit": "^12.5", + "vimeo/psalm": "^6.16" }, "suggest": { "ext-memcached": "*", diff --git a/phpunit.xml.dist b/phpunit.xml similarity index 100% rename from phpunit.xml.dist rename to phpunit.xml From 8b5a82a750d60c6dd084289b884c6ea7693c9467 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Sat, 22 Aug 2026 19:00:48 -0400 Subject: [PATCH 08/15] Move Psalm to a dedicated tools/psalm manifest Psalm enumerates the PHP versions it supports and has no 8.6 entry in any published release. As a require-dev it therefore made "composer install" fail on the 8.6 build job before a single test ran - the matrix is fail-fast, so 8.3/8.4/8.5 were cancelled rather than run. Psalm now lives in tools/psalm/composer.json with its own vendor dir, installed only by the Psalm job (PHP 8.5). The root manifest no longer constrains PHP through a dev dependency, so every build job resolves. "composer psalm" bootstraps the tool and runs it, so local usage is unchanged. tools/psalm/vendor and its lock are already covered by the existing gitignore patterns. --- .github/workflows/phpunit.yml | 5 ++++- composer.json | 8 +++++--- psalm.xml | 2 +- tools/psalm/composer.json | 5 +++++ 4 files changed, 15 insertions(+), 5 deletions(-) create mode 100644 tools/psalm/composer.json diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index ce0aca4..264b7ba 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -65,10 +65,13 @@ jobs: - name: Composer run: composer install + - name: Composer (psalm) + run: composer --working-dir=tools/psalm update --no-interaction + - name: Psalm # Note: Ignoring error code 2, which just signals that some # flaws were found, not that Psalm itself failed to run. - run: ./vendor/bin/psalm + run: ./tools/psalm/vendor/bin/psalm --show-info=true --report=psalm-results.sarif || [ $? = 2 ] diff --git a/composer.json b/composer.json index 0b0323d..5677d69 100644 --- a/composer.json +++ b/composer.json @@ -19,8 +19,7 @@ "psr/container": "^1.0|^1.1|^2.0" }, "require-dev": { - "phpunit/phpunit": "^12.5", - "vimeo/psalm": "^6.16" + "phpunit/phpunit": "^12.5" }, "suggest": { "ext-memcached": "*", @@ -33,7 +32,10 @@ }, "scripts": { "test": "vendor/bin/phpunit", - "psalm": "vendor/bin/psalm --threads=1" + "psalm": [ + "@composer --working-dir=tools/psalm update --no-interaction", + "tools/psalm/vendor/bin/psalm --threads=1" + ] }, "license": "MIT", "prefer-stable": true, diff --git a/psalm.xml b/psalm.xml index b208114..9fae632 100644 --- a/psalm.xml +++ b/psalm.xml @@ -7,7 +7,7 @@ cacheDirectory="/tmp/psalm" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" - xsi:schemaLocation="https://getpsalm.org/schema/config vendor/vimeo/psalm/config.xsd" + xsi:schemaLocation="https://getpsalm.org/schema/config tools/psalm/vendor/vimeo/psalm/config.xsd" > diff --git a/tools/psalm/composer.json b/tools/psalm/composer.json new file mode 100644 index 0000000..197dd5f --- /dev/null +++ b/tools/psalm/composer.json @@ -0,0 +1,5 @@ +{ + "require": { + "vimeo/psalm": "^6.16" + } +} From 2f1cd57282d38ecb330b7e632db6c0d7fea646e6 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Mon, 14 Sep 2026 12:08:01 -0400 Subject: [PATCH 09/15] Fix the license badge link and drop the hand-written dependency graph licensing.html returns 404; the page is /license/. The Dependencies mermaid drifted from composer.json, which is now what the site's graph is built from (collect-deps.py in add-doc). --- README.md | 12 +----------- 1 file changed, 1 insertion(+), 11 deletions(-) diff --git a/README.md b/README.md index cc60f8a..8e6a253 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,7 @@ A powerful, versatile cache implementation providing both PSR-6 and PSR-16 inter [![Build Status](https://github.com/byjg/php-cache-engine/actions/workflows/phpunit.yml/badge.svg?branch=master)](https://github.com/byjg/php-cache-engine/actions/workflows/phpunit.yml) [![Opensource ByJG](https://img.shields.io/badge/opensource-byjg-success.svg)](http://opensource.byjg.com) [![GitHub source](https://img.shields.io/badge/Github-source-informational?logo=github)](https://github.com/byjg/php-cache-engine/) -[![GitHub license](https://img.shields.io/github/license/byjg/php-cache-engine.svg)](https://opensource.byjg.com/opensource/licensing.html) +[![GitHub license](https://img.shields.io/github/license/byjg/php-cache-engine.svg)](https://opensource.byjg.com/license/) [![GitHub release](https://img.shields.io/github/release/byjg/php-cache-engine.svg)](https://github.com/byjg/php-cache-engine/releases/) ## Key Features @@ -82,15 +82,5 @@ vendor/bin/phpunit --stderr **Note:** The `--stderr` parameter is required for SessionCacheEngine tests to run properly. -## Dependencies - -```mermaid -flowchart TD - byjg/cache-engine --> psr/cache - byjg/cache-engine --> psr/log - byjg/cache-engine --> psr/simple-cache - byjg/cache-engine --> psr/container -``` - ---- [Open source ByJG](http://opensource.byjg.com) From 0e37703d6b6da3e0feffd375015b7c57717c116b Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 9 Oct 2026 15:19:38 -0400 Subject: [PATCH 10/15] Align with the 7.0 standard - Install Psalm as psalm/phar instead of the tools/psalm project - phpunit.xml: PHPUnit 12.5 schema and the failOn* flags - Workflow: actions/checkout@v7, SARIF upload only on master - README: badges, Installation and Running Tests sections, https links - Remove FUNDING.yml (comes from byjg/.github) and IDE files - CHANGELOG-7.0.md updated --- .github/FUNDING.yml | 3 --- .github/workflows/phpunit.yml | 10 ++++------ .gitignore | 1 + .gitpod.yml | 28 ---------------------------- .vscode/launch.json | 35 ----------------------------------- CHANGELOG-7.0.md | 23 +++++++++++++++-------- README.md | 7 ++++--- composer.json | 8 +++----- phpunit.xml | 6 +++++- psalm.xml | 3 +-- tools/psalm/composer.json | 5 ----- 11 files changed, 33 insertions(+), 96 deletions(-) delete mode 100644 .github/FUNDING.yml delete mode 100644 .gitpod.yml delete mode 100644 .vscode/launch.json delete mode 100644 tools/psalm/composer.json diff --git a/.github/FUNDING.yml b/.github/FUNDING.yml deleted file mode 100644 index 3a9251f..0000000 --- a/.github/FUNDING.yml +++ /dev/null @@ -1,3 +0,0 @@ -# These are supported funding model platforms - -github: byjg diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 264b7ba..bc758ad 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -44,7 +44,7 @@ jobs: --health-retries 5 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v7 - run: composer install - run: composer test @@ -60,23 +60,21 @@ jobs: steps: - name: Git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Composer run: composer install - - name: Composer (psalm) - run: composer --working-dir=tools/psalm update --no-interaction - - name: Psalm # Note: Ignoring error code 2, which just signals that some # flaws were found, not that Psalm itself failed to run. - run: ./tools/psalm/vendor/bin/psalm + run: ./vendor/bin/psalm.phar --show-info=true --report=psalm-results.sarif || [ $? = 2 ] - name: Upload Analysis results to GitHub uses: github/codeql-action/upload-sarif@v4 + if: github.ref == 'refs/heads/master' with: sarif_file: psalm-results.sarif diff --git a/.gitignore b/.gitignore index d06da71..025fdff 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ config/cacheconfig.php .phpunit.result.cache /phpunit.xml.dist.bak /.claude/settings.local.json +.vscode diff --git a/.gitpod.yml b/.gitpod.yml deleted file mode 100644 index 335de09..0000000 --- a/.gitpod.yml +++ /dev/null @@ -1,28 +0,0 @@ -tasks: - - name: Run Composer - command: | - composer install - -image: byjg/gitpod-image:latest - -jetbrains: - phpstorm: - vmoptions: '-Xmx4g' - plugins: - - com.github.copilot - - com.intellij.kubernetes - - com.intellij.mermaid - - ru.adelf.idea.dotenv - - org.toml.lang - -vscode: - extensions: - - ikappas.composer - - hbenl.test-adapter-converter - - hbenl.vscode-test-explorer - - felixfbecker.php-debug - - neilbrayfield.php-docblocker - - bmewburn.vscode-intelephense-client - - getpsalm.psalm-vscode-plugin - - SonarSource.sonarlint-vscode - - recca0120.vscode-phpunit \ No newline at end of file diff --git a/.vscode/launch.json b/.vscode/launch.json deleted file mode 100644 index a8c1b2a..0000000 --- a/.vscode/launch.json +++ /dev/null @@ -1,35 +0,0 @@ -{ - "version": "0.2.0", - "configurations": [ - { - "name": "Debug current Script in Console", - "type": "php", - "request": "launch", - "program": "${file}", - "cwd": "${fileDirname}", - "port": 9003, - "runtimeArgs": [ - "-dxdebug.start_with_request=yes" - ], - "env": { - "XDEBUG_MODE": "debug,develop", - "XDEBUG_CONFIG": "client_port=${port}" - } - }, - { - "name": "PHPUnit Debug", - "type": "php", - "request": "launch", - "program": "${workspaceFolder}/vendor/bin/phpunit", - "cwd": "${workspaceFolder}", - "port": 9003, - "runtimeArgs": [ - "-dxdebug.start_with_request=yes" - ], - "env": { - "XDEBUG_MODE": "debug,develop", - "XDEBUG_CONFIG": "client_port=${port}" - } - } - ] -} \ No newline at end of file diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md index 190ac05..07779a9 100644 --- a/CHANGELOG-7.0.md +++ b/CHANGELOG-7.0.md @@ -1,5 +1,8 @@ # Changelog - Version 7.0 +> **Status: in development.** This document tracks changes landing on the `7.0` branch. +> Nothing here is released yet, and the contents may still change. + ## Overview Version 7.0 adds `CompareAndSwapInterface`, the conditional-write primitive needed to build correct @@ -219,20 +222,24 @@ if ($cache->setIfAbsent('lock', $token, 30)) { ## Toolchain - PHPUnit updated to `^12.5`. -- Psalm updated to `^6.16`. +- Psalm is installed as `psalm/phar` (`^6.16`) instead of `vimeo/psalm`. + + `vimeo/psalm` lists the PHP versions it supports and no published release includes + 8.6, so as a dev dependency it made `composer install` fail on the 8.6 build job + before any test ran. `psalm/phar` requires only `php ^8.2` and bundles its own + dependencies, so it installs on every PHP version in the matrix and cannot conflict + with the project's. Psalm itself still refuses to *run* on 8.6, which is why the + Psalm job uses 8.5. `composer psalm` runs it. PHPUnit 13 is deliberately **not** used. It requires PHP `>=8.4.1`, which would - break the 8.3 floor, and it pulls `sebastian/diff ^9.0`, which the newest stable - Psalm (6.16.1) does not accept — that combination silently resolves Psalm to an - unreleased `6.x-dev` branch. Pinning PHPUnit to `^12.5` keeps a single stable - PHPUnit and a single stable Psalm across the whole matrix. + break the 8.3 floor. ## Continuous Integration - The build matrix now includes PHP 8.6. -- The Psalm job now runs on PHP 8.5. Psalm 6.16.1 declares - `~8.1.31 || ~8.2.27 || ~8.3.16 || ~8.4.3 || ~8.5.0` and therefore cannot be - installed on PHP 8.6. +- The Psalm job now runs on PHP 8.5. Psalm declares + `~8.1.31 || ~8.2.27 || ~8.3.16 || ~8.4.3 || ~8.5.0` and therefore does not run on + PHP 8.6. ## Housekeeping diff --git a/README.md b/README.md index 8e6a253..c028d30 100644 --- a/README.md +++ b/README.md @@ -9,7 +9,8 @@ A powerful, versatile cache implementation providing both PSR-6 and PSR-16 inter [![Sponsor](https://img.shields.io/badge/Sponsor-%23ea4aaa?logo=githubsponsors&logoColor=white&labelColor=0d1117)](https://github.com/sponsors/byjg) [![Build Status](https://github.com/byjg/php-cache-engine/actions/workflows/phpunit.yml/badge.svg?branch=master)](https://github.com/byjg/php-cache-engine/actions/workflows/phpunit.yml) -[![Opensource ByJG](https://img.shields.io/badge/opensource-byjg-success.svg)](http://opensource.byjg.com) +[![Opensource ByJG](https://img.shields.io/badge/opensource-byjg-success.svg)](https://opensource.byjg.com) +[![Install MCP Server](https://img.shields.io/badge/Install-MCP_Server-8A2BE2?logo=modelcontextprotocol&logoColor=white)](https://opensource.byjg.com/docs/ai/mcpserver-byjg-docs/) [![GitHub source](https://img.shields.io/badge/Github-source-informational?logo=github)](https://github.com/byjg/php-cache-engine/) [![GitHub license](https://img.shields.io/github/license/byjg/php-cache-engine.svg)](https://opensource.byjg.com/license/) [![GitHub release](https://img.shields.io/github/release/byjg/php-cache-engine.svg)](https://github.com/byjg/php-cache-engine/releases/) @@ -74,7 +75,7 @@ $value = $item->get(); - [Logging](docs/setup-log-handler.md) - [PSR-11 Container Usage](docs/psr11-usage.md) -## Running Unit Tests +## Running Tests ``` vendor/bin/phpunit --stderr @@ -83,4 +84,4 @@ vendor/bin/phpunit --stderr **Note:** The `--stderr` parameter is required for SessionCacheEngine tests to run properly. ---- -[Open source ByJG](http://opensource.byjg.com) +[Open source ByJG](https://opensource.byjg.com) diff --git a/composer.json b/composer.json index 5677d69..c6b4aea 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,8 @@ "psr/container": "^1.0|^1.1|^2.0" }, "require-dev": { - "phpunit/phpunit": "^12.5" + "phpunit/phpunit": "^12.5", + "psalm/phar": "^6.16" }, "suggest": { "ext-memcached": "*", @@ -32,10 +33,7 @@ }, "scripts": { "test": "vendor/bin/phpunit", - "psalm": [ - "@composer --working-dir=tools/psalm update --no-interaction", - "tools/psalm/vendor/bin/psalm --threads=1" - ] + "psalm": "vendor/bin/psalm.phar" }, "license": "MIT", "prefer-stable": true, diff --git a/phpunit.xml b/phpunit.xml index 365de7f..c3c976d 100644 --- a/phpunit.xml +++ b/phpunit.xml @@ -14,8 +14,12 @@ and open the template in the editor. displayDetailsOnTestsThatTriggerNotices="true" displayDetailsOnTestsThatTriggerWarnings="true" displayDetailsOnPhpunitDeprecations="true" + failOnWarning="true" + failOnNotice="true" + failOnDeprecation="true" + failOnPhpunitDeprecation="true" stopOnFailure="false" - xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/10.5/phpunit.xsd"> + xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/12.5/phpunit.xsd"> diff --git a/psalm.xml b/psalm.xml index 9fae632..7414b98 100644 --- a/psalm.xml +++ b/psalm.xml @@ -7,11 +7,10 @@ cacheDirectory="/tmp/psalm" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns="https://getpsalm.org/schema/config" - xsi:schemaLocation="https://getpsalm.org/schema/config tools/psalm/vendor/vimeo/psalm/config.xsd" + xsi:schemaLocation="https://getpsalm.org/schema/config https://raw.githubusercontent.com/vimeo/psalm/6.x/config.xsd" > - diff --git a/tools/psalm/composer.json b/tools/psalm/composer.json deleted file mode 100644 index 197dd5f..0000000 --- a/tools/psalm/composer.json +++ /dev/null @@ -1,5 +0,0 @@ -{ - "require": { - "vimeo/psalm": "^6.16" - } -} From 1f1e48ecb956d2077f330a4f1b498a7291949872 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Mon, 12 Oct 2026 19:22:16 -0400 Subject: [PATCH 11/15] Remove ShmopCacheEngine, fix TTL and container handling, document the public API - Remove the deprecated ShmopCacheEngine and Factory::createShmopPool() - Memcached: apply the TTL of increment/decrement to a counter that already exists - FileSystem: atomic operations keep the expiry when the TTL is omitted and start over on an expired key - clear(), collectGarbage() and isAvailable() no longer go through the PSR-11 container - Session: clear() removes only the entries of its prefix; has() starts the session - Docs: TTL semantics, isAvailable(), the CachePool buffer, getTtl(); README Installation and Running Tests sections follow the standard --- CHANGELOG-7.0.md | 67 ++++++- README.md | 12 +- composer.json | 3 +- docs/atomic-operations.md | 38 +++- docs/basic-usage-psr16-simplecache.md | 14 ++ docs/basic-usage-psr6-cachepool.md | 29 ++- docs/class-session-cache-engine.md | 3 + docs/class-shmop-cache-engine.md | 49 ----- docs/garbage-collection.md | 10 +- docs/psr11-usage.md | 2 +- docs/setup-log-handler.md | 2 +- src/Factory.php | 9 - src/Psr16/FileSystemCacheEngine.php | 53 +++-- src/Psr16/MemcachedEngine.php | 19 ++ src/Psr16/RedisCacheEngine.php | 7 +- src/Psr16/SessionCacheEngine.php | 13 +- src/Psr16/ShmopCacheEngine.php | 273 -------------------------- tests/CachePSR16Test.php | 159 +++++++++++++++ tests/TestBase.php | 4 - 19 files changed, 380 insertions(+), 386 deletions(-) delete mode 100644 docs/class-shmop-cache-engine.md delete mode 100644 src/Psr16/ShmopCacheEngine.php diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md index 07779a9..1d2961b 100644 --- a/CHANGELOG-7.0.md +++ b/CHANGELOG-7.0.md @@ -19,6 +19,9 @@ concurrent `increment()` calls on Memcached ended at **8** instead of 20, and tw |------|--------------|-------------|-------------| | **Redis atomic operations** | `INCR`/`RPUSH` + separate `EXPIRE` | Lua scripts (`EVAL`) | `increment()`, `decrement()` and `add()` are now single scripts so the value and its TTL cannot be split apart. **Requires Lua scripting to be enabled on the Redis server**; some managed offerings restrict `EVAL`. | | **Redis `add()` TTL** | `$ttl` accepted and silently ignored | `$ttl` applied | A list built with `add($key, $value, 60)` now actually expires. Code that relied on the list living forever despite passing a TTL will see it expire. | +| **`ShmopCacheEngine` removed** | `ShmopCacheEngine`, `Factory::createShmopPool()` | removed | Deprecated since 5.0 in favour of `TmpfsCacheEngine`, which keeps the data in RAM the same way. The `ext-shmop` suggestion is gone with it. | +| **Session `clear()`** | `session_destroy()` | removes only the entries of the engine's prefix | `SessionCacheEngine::clear()` destroyed the whole session — the login and everything else the application kept there. It now unsets only its own `-*` entries. Code that used it to end the session must call `session_destroy()` itself. | +| **FileSystem atomic operations without a TTL** | dropped the expiry of the key | keep it | `increment()`, `decrement()` and `add()` called without a TTL made a key that had an expiry permanent. They now leave the expiry alone, as Redis does. | | **FileSystem atomic TTL** | raw `$ttl` written as the expiry timestamp | converted with `addToNow()` | `increment($key, 1, 60)` wrote `60` into the expiry file — a moment in 1970 — so the entry was already expired when written. It now means 60 seconds from now, and the value survives as intended. | ## New Features @@ -58,8 +61,11 @@ if (!$cache instanceof \ByJG\Cache\CompareAndSwapInterface) { - New [Compare and Swap](docs/compare-and-swap.md) guide, including a worked lock example and an explicit section on what the interface does *not* give you (no fairness, no reentrancy, the TTL is still a guess). -- [Atomic Operations](docs/atomic-operations.md) now documents the TTL semantics and what happens - when `add()` is called on a key previously written by `set()`. +- [Atomic Operations](docs/atomic-operations.md) now documents the TTL semantics — including what + each engine does when the TTL is omitted on a key that already has one — the return values, and + what happens when `add()` is called on a key previously written by `set()`. +- Documented what was public but unexplained: `isAvailable()`, the `CachePool` buffer + (`$bufferSize`, `getBufferSize()`, `setBufferSize()`) and `GarbageCollectorInterface::getTtl()`. ## Bug Fixes @@ -116,6 +122,36 @@ It went unnoticed because in practice something else — `isAvailable()`, `get() always ran first and left the connection open. A regression test now exercises each entry point on an instance that has never been touched. +### Memcached: TTL ignored on a counter that already existed + +`increment()` and `decrement()` only applied the TTL on the call that created the key. Passing one +for a counter that was already there did nothing, so the counter kept its old expiry — or none. +Redis and FileSystem applied it on every call. Memcached now does too, with a `touch()` right after +the increment. + +### FileSystem: atomic operations carried on from an expired value + +An expired entry is only removed when it is read. `increment()`, `decrement()` and `add()` arriving +first read the stale file and continued from it — a counter that expired at 7 answered 8. An +expired entry is now treated as absent and the operation starts over. + +### `clear()`, `collectGarbage()` and `isAvailable()` failed with a PSR-11 container + +With `withKeysFromContainer()` set, these died with `Key '...' not found in container`: + +- `RedisCacheEngine::clear()` deleted every scanned key through `delete()`, which looked the stored + key up in the container. +- `FileSystemCacheEngine` (and `TmpfsCacheEngine`) built the `*` pattern of `clear()` and + `collectGarbage()`, and the probe path of `isAvailable()`, through the same lookup. + +They act on the whole engine and no longer go through the container. + +### Session: `has()` did not start the session + +`SessionCacheEngine::has()` read `$_SESSION` without starting the session, unlike `get()`, `set()` +and `delete()`. Called first in a request it always answered `false`, and called right after +`clear()` it still saw the entries that had just been destroyed. + ### FileSystem: expiry dropped outside the lock `putContents()` deleted the `.ttl` file before acquiring the lock, leaving the value briefly @@ -128,8 +164,10 @@ delete now happens under the lock. rejection and the stale-owner-versus-new-owner case. - `tests/ConcurrencyTest.php` — forks twenty real processes at a synchronised barrier. Sequential tests cannot prove atomicity; these fail against 6.x and pass on 7.0. -- A TTL regression test added to `tests/CachePSR16Test.php` covering every engine that implements - `AtomicOperationInterface`. +- TTL regression tests added to `tests/CachePSR16Test.php` covering every engine that implements + `AtomicOperationInterface`, for a new key and for one that already exists. +- `clear()`, `collectGarbage()` and `isAvailable()` are tested on every engine with a PSR-11 + container set. Note for anyone writing similar tests: forked children inherit the parent's Memcached socket and close it as they exit, killing the parent's connection. Reconnect in the parent before asserting. @@ -169,7 +207,26 @@ $cache->add('my-key', 'value'); // never expires, as 6.x behaved These were broken and returned already-expired entries, so working code is unlikely to depend on them. If you worked around the bug by omitting the TTL, you can now pass it. -### Step 5: Adopt compare-and-swap where you were hand-rolling a lock +### Step 5: Replace `ShmopCacheEngine` + +```php +// Before +$cache = new \ByJG\Cache\Psr16\ShmopCacheEngine(); +$pool = \ByJG\Cache\Factory::createShmopPool(); + +// After +$cache = new \ByJG\Cache\Psr16\TmpfsCacheEngine(); +$pool = \ByJG\Cache\Factory::createTmpfsCachePool(); +``` + +The `max-size` and `default-permission` options have no equivalent; entries are files in `/dev/shm`. + +### Step 6: Review any use of `SessionCacheEngine::clear()` + +It no longer destroys the session. If you relied on that to log the user out, call +`session_destroy()` yourself. + +### Step 7: Adopt compare-and-swap where you were hand-rolling a lock If you have code shaped like this, replace it — it has a race between the two calls: diff --git a/README.md b/README.md index c028d30..0347ca6 100644 --- a/README.md +++ b/README.md @@ -26,12 +26,14 @@ A powerful, versatile cache implementation providing both PSR-6 and PSR-16 inter - **PSR-11 container support** - Retrieve cache keys via dependency container - **Logging capabilities** - PSR-3 compatible logging of cache operations -## Quick Start +## Installation ```bash composer require "byjg/cache-engine" ``` +## Quick Start + ```php // PSR-16 Simple Cache $cache = new \ByJG\Cache\Psr16\FileSystemCacheEngine(); @@ -65,7 +67,6 @@ $value = $item->get(); | [RedisCacheEngine](docs/class-redis-cache-engine.md) | Redis-based caching | | [SessionCacheEngine](docs/class-session-cache-engine.md) | PHP session-based caching | | [TmpfsCacheEngine](docs/class-tmpfs-cache-engine.md) | Tmpfs-based caching | -| [ShmopCacheEngine](docs/class-shmop-cache-engine.md) | Shared memory caching (deprecated) | | [KeyValueCacheEngine](https://github.com/byjg/php-anydataset-nosql) | S3-Like or CloudflareKV storage (separate package) | ### Advanced Features @@ -77,11 +78,10 @@ $value = $item->get(); ## Running Tests +```bash +docker compose up -d +vendor/bin/phpunit ``` -vendor/bin/phpunit --stderr -``` - -**Note:** The `--stderr` parameter is required for SessionCacheEngine tests to run properly. ---- [Open source ByJG](https://opensource.byjg.com) diff --git a/composer.json b/composer.json index c6b4aea..e783134 100644 --- a/composer.json +++ b/composer.json @@ -24,8 +24,7 @@ }, "suggest": { "ext-memcached": "*", - "ext-redis": "*", - "ext-shmop": "*" + "ext-redis": "*" }, "provide": { "psr/cache-implementation": "1.0", diff --git a/docs/atomic-operations.md b/docs/atomic-operations.md index 8dc2539..7217192 100644 --- a/docs/atomic-operations.md +++ b/docs/atomic-operations.md @@ -28,32 +28,56 @@ These operations read and modify a value in one step. If what you need instead i write — "store this only if the key is free", "delete this only if it is still mine" — see [Compare and Swap](compare-and-swap.md). -All three operations accept an optional TTL, expressed in seconds from now (or as a `DateInterval`), -and apply it as part of the same operation. +## TTL + +All three operations accept an optional TTL as the last argument, expressed in seconds from now (or +as a `DateInterval`). When you pass one, it is applied on every call, whether the call creates the +key or finds it already there: + +```php +increment('my-key', 1, 60); // 'my-key' expires 60 seconds from now +``` + +Redis and FileSystem write the value and the expiry in one step. Memcached does so when the call +creates the key; on a counter that already exists the expiry is a second command right after the +increment. + +When you **omit** the TTL, the key keeps the expiry it already has. A key that has already expired +counts as absent: the operation starts over from an empty value and the new key has no expiry. + +:::caution MemcachedEngine and add() +`add()` on Memcached is the exception: called without a TTL on a list that has one, it removes the +expiry. Memcached has to be given an expiration on every write and offers no way to read the time +a key has left. Pass the TTL on every `add()` if the list has to expire. +::: ## Increment -The increment operation is used to increment a value by a given number. +The increment operation is used to increment a value by a given number. A key that does not exist +starts at zero. It returns the new value. ```php increment('my-key', 1); +$newValue = $cache->increment('my-key', 1); ``` ## Decrement -The decrement operation is used to decrement a value by a given number. +The decrement operation is used to decrement a value by a given number. It returns the new value. ```php decrement('my-key', 1); +$newValue = $cache->decrement('my-key', 1); ``` ## Add -The add operation is used to add a value to a list in the cache. +The add operation is used to add a value to a list in the cache. It returns the whole list after +the append. ```php delete($key); ``` +## Check if the engine is available + +Every engine implements `CacheAvailabilityInterface`. `isAvailable()` tells whether the engine can +be used right now: the PHP extension is loaded and the server answers (Redis, Memcached), the +directory is writable (FileSystem, Tmpfs), the session can be started (Session). + +```php +isAvailable()) { + $cacheEngine = new \ByJG\Cache\Psr16\NoCacheEngine(); +} +``` + ## Clear all cache ```php diff --git a/docs/basic-usage-psr6-cachepool.md b/docs/basic-usage-psr6-cachepool.md index db9359f..eabae08 100644 --- a/docs/basic-usage-psr6-cachepool.md +++ b/docs/basic-usage-psr6-cachepool.md @@ -36,4 +36,31 @@ $pool->deleteItem('mykey'); ```php setBufferSize(0); // disable the buffer +$size = $pool->getBufferSize(); // 0 +``` + +## Check if the engine is available + +```php +isAvailable()) { + // The underlying engine cannot be used (extension missing, server down, ...) +} +``` diff --git a/docs/class-session-cache-engine.md b/docs/class-session-cache-engine.md index 374d1b0..3a2c952 100644 --- a/docs/class-session-cache-engine.md +++ b/docs/class-session-cache-engine.md @@ -9,6 +9,9 @@ This will persist the cache between requests while the user session is active. The cache is not shared between different users. +Every entry is stored in `$_SESSION` under `-`. `clear()` removes only the entries of +that prefix; the session itself, and anything else your application keeps in it, is left alone. + ## PSR-16 Constructor diff --git a/docs/class-shmop-cache-engine.md b/docs/class-shmop-cache-engine.md deleted file mode 100644 index 7518ed6..0000000 --- a/docs/class-shmop-cache-engine.md +++ /dev/null @@ -1,49 +0,0 @@ ---- -sidebar_position: 10 ---- - -# ShmopCacheEngine - -:::warning Deprecated -This engine is deprecated. Use [TmpfsCacheEngine](class-tmpfs-cache-engine.md) instead for better performance and reliability. -::: - -This class uses PHP Shmop (shared memory) as the cache engine. - -Shared memory allows multiple processes to access the same data in memory. -You can use it to share data among running PHP scripts on the same server. - -## Configuration - -These are the default values for the configuration: - -```php -$config = [ - 'max-size' => 524288, // 512KB - 'default-permission' => '0700', -]; -``` - -## PSR-16 Constructor - -```php -$cache = new \ByJG\Cache\Psr16\ShmopCacheEngine($config, $logger); -``` - -**Parameters:** -- `$config` (array, default: []): Configuration options for shared memory -- `$logger` (LoggerInterface|null, default: null): PSR-3 logger instance - -## PSR-6 Constructor - -```php -$cachePool = \ByJG\Cache\Factory::createShmopPool($config, $bufferSize, $logger); -``` - -or - -```php -$cachePool = new \ByJG\Cache\Psr6\CachePool(new \ByJG\Cache\Psr16\ShmopCacheEngine($config, $logger)); -``` - - diff --git a/docs/garbage-collection.md b/docs/garbage-collection.md index ec92dfa..bb193b7 100644 --- a/docs/garbage-collection.md +++ b/docs/garbage-collection.md @@ -1,5 +1,5 @@ --- -sidebar_position: 12 +sidebar_position: 13 --- # Garbage Collection @@ -14,7 +14,11 @@ is based on best effort, meaning an expired key is removed only when you try to If the cache engine has a low hit rate, it is recommended to run a garbage collection process to prevent the cache from growing indefinitely. -The classes that implement the `GarbageCollectorInterface` have the method `collectGarbage()`. +The classes that implement the `GarbageCollectorInterface` have two methods: + +- `collectGarbage()` removes every expired key; +- `getTtl(string $key): ?int` returns the moment the key expires, as a Unix timestamp, or `null` + when the key has no expiry (or does not exist). **Engines that support garbage collection:** - FileSystemCacheEngine @@ -27,6 +31,8 @@ The classes that implement the `GarbageCollectorInterface` have the method `coll collectGarbage(); + +$expiresAt = $cache->getTtl('my-key'); // e.g. 1791849600, or null ``` :::caution Performance Warning diff --git a/docs/psr11-usage.md b/docs/psr11-usage.md index 0ee4681..c79a6a8 100644 --- a/docs/psr11-usage.md +++ b/docs/psr11-usage.md @@ -1,5 +1,5 @@ --- -sidebar_position: 14 +sidebar_position: 15 --- # PSR-11 Container Usage diff --git a/docs/setup-log-handler.md b/docs/setup-log-handler.md index b505b4e..c005b85 100644 --- a/docs/setup-log-handler.md +++ b/docs/setup-log-handler.md @@ -1,5 +1,5 @@ --- -sidebar_position: 13 +sidebar_position: 14 --- # Setup Log Handler diff --git a/src/Factory.php b/src/Factory.php index cfcb483..2453f1d 100644 --- a/src/Factory.php +++ b/src/Factory.php @@ -9,7 +9,6 @@ use ByJG\Cache\Psr16\NoCacheEngine; use ByJG\Cache\Psr16\RedisCacheEngine; use ByJG\Cache\Psr16\SessionCacheEngine; -use ByJG\Cache\Psr16\ShmopCacheEngine; use ByJG\Cache\Psr6\CachePool; use Psr\Log\LoggerInterface; @@ -38,14 +37,6 @@ public static function createFilePool(string $prefix = 'cache', ?string $path = ); } - public static function createShmopPool(array $config = [], int $bufferSize = 10, ?LoggerInterface $logger = null): CachePool - { - return new CachePool( - new ShmopCacheEngine($config, $logger), - $bufferSize - ); - } - public static function createArrayPool(int $bufferSize = 10, ?LoggerInterface $logger = null): CachePool { return new CachePool( diff --git a/src/Psr16/FileSystemCacheEngine.php b/src/Psr16/FileSystemCacheEngine.php index 4963ff9..929e5a6 100644 --- a/src/Psr16/FileSystemCacheEngine.php +++ b/src/Psr16/FileSystemCacheEngine.php @@ -105,15 +105,10 @@ public function delete(string $key): bool return true; } - /** - * @throws ContainerExceptionInterface - * @throws NotFoundExceptionInterface - * @throws InvalidArgumentException - */ #[Override] public function isAvailable(): bool { - return is_writable(dirname($this->fixKey('test'))); + return is_writable($this->path); } /** @@ -123,8 +118,15 @@ public function isAvailable(): bool */ protected function fixKey(string $key): string { - $key = $this->getKeyFromContainer($key); + return $this->fileName($this->getKeyFromContainer($key)); + } + /** + * The file behind a key that is already final. clear() and collectGarbage() match every entry + * with a '*' pattern, which is not a cache key and must not be looked up in the container. + */ + protected function fileName(string $key): string + { return $this->path . '/' . $this->prefix . '-' . preg_replace("/[\/\\\]/", "#", $key) @@ -135,14 +137,11 @@ protected function fixKey(string $key): string * Wipes clean the entire cache's keys. * * @return bool True on success and false on failure. - * @throws ContainerExceptionInterface - * @throws NotFoundExceptionInterface - * @throws InvalidArgumentException */ #[Override] public function clear(): bool { - $patternKey = $this->fixKey('*'); + $patternKey = $this->fileName('*'); $list = glob($patternKey); foreach ($list as $file) { if (file_exists($file)) { @@ -169,13 +168,8 @@ public function clear(): bool public function has(string $key): bool { $fileKey = $this->fixKey($key); - $fileTtl = null; if (file_exists($fileKey)) { - if (file_exists("$fileKey.ttl")) { - $fileTtl = intval($this->getContents("$fileKey.ttl")); - } - - if (!empty($fileTtl) && time() >= $fileTtl) { + if ($this->isExpired($fileKey)) { $this->logger->info("[Filesystem cache] File too old. Ignoring '$key'"); $this->delete($key); @@ -188,6 +182,17 @@ public function has(string $key): bool return false; } + protected function isExpired(string $fileKey): bool + { + if (!file_exists("$fileKey.ttl")) { + return false; + } + + $fileTtl = intval($this->getContents("$fileKey.ttl")); + + return !empty($fileTtl) && time() >= $fileTtl; + } + protected function getContents(string $fileKey, mixed $default = null): mixed { if (!file_exists($fileKey)) { @@ -225,15 +230,21 @@ protected function putContents(string $fileKey, mixed $value, ?int $ttl, ?Closur $waitIfLocked = 1; $lock = flock($fo, LOCK_EX, $waitIfLocked); try { + // An expired entry is only removed when somebody reads it, so an atomic operation can + // get here first. It has to start over, not carry on from a value that is gone. + $expired = $this->isExpired($fileKey); + // Expiry is dropped and rewritten under the lock. Doing it before acquiring the lock // left the value briefly immortal, and a reader arriving in that window saw a key that - // should already have expired. - if (file_exists("$fileKey.ttl")) { + // should already have expired. An atomic operation called without a TTL keeps the + // expiry the entry already has. + $keepExpiry = !is_null($operation) && is_null($ttl) && !$expired; + if (!$keepExpiry && file_exists("$fileKey.ttl")) { unlink("$fileKey.ttl"); } if (!is_null($operation)) { - $content = file_get_contents($fileKey); + $content = $expired ? '' : file_get_contents($fileKey); $currentValue = !empty($content) ? unserialize($content) : 0; $value = $returnValue = $operation($currentValue, $value); } @@ -252,7 +263,7 @@ protected function putContents(string $fileKey, mixed $value, ?int $ttl, ?Closur #[Override] public function collectGarbage() { - $patternKey = $this->fixKey('*'); + $patternKey = $this->fileName('*'); $list = glob("$patternKey.ttl"); foreach ($list as $file) { $fileTtl = intval($this->getContents($file)); diff --git a/src/Psr16/MemcachedEngine.php b/src/Psr16/MemcachedEngine.php index b6d328f..eabbcdd 100644 --- a/src/Psr16/MemcachedEngine.php +++ b/src/Psr16/MemcachedEngine.php @@ -241,6 +241,7 @@ public function increment(string $key, int $value = 1, DateInterval|int|null $tt $this->seed($fixKey, 0, $ttl); $result = $this->memCached->increment($fixKey, $value); + $this->refreshTtl($fixKey, $ttl); $this->logger->info("[Memcached] Increment '$key' result " . $this->memCached->getResultCode()); if ($this->memCached->getResultCode() !== Memcached::RES_SUCCESS) { $this->logger->error("[Memcached] Increment '$key' failed with status " . $this->memCached->getResultCode()); @@ -264,6 +265,7 @@ public function decrement(string $key, int $value = 1, DateInterval|int|null $tt $this->seed($fixKey, 0, $ttl); $result = $this->memCached->decrement($fixKey, $value); + $this->refreshTtl($fixKey, $ttl); $this->logger->info("[Memcached] Decrement '$key' result " . $this->memCached->getResultCode()); if ($this->memCached->getResultCode() !== Memcached::RES_SUCCESS) { $this->logger->error("[Memcached] Decrement '$key' failed with status " . $this->memCached->getResultCode()); @@ -288,6 +290,23 @@ private function seed(string $fixKey, mixed $initial, DateInterval|int|null $ttl $this->memCached->add($fixKey, $initial, is_null($ttl) ? 0 : $ttl); } + /** + * Apply the TTL to a counter that already existed. + * + * seed() only sets the expiry on the call that creates the key, and Memcached's + * increment/decrement leave it alone. Without this a TTL passed for an existing counter was + * ignored. It is a second command, but a process dying before it leaves the previous expiry in + * place rather than none at all. + */ + private function refreshTtl(string $fixKey, DateInterval|int|null $ttl): void + { + $ttl = $this->convertToSeconds($ttl); + + if (!empty($ttl)) { + $this->memCached->touch($fixKey, $ttl); + } + } + /** * @throws NotFoundExceptionInterface * @throws InvalidArgumentException diff --git a/src/Psr16/RedisCacheEngine.php b/src/Psr16/RedisCacheEngine.php index d873fd5..d82b0e4 100644 --- a/src/Psr16/RedisCacheEngine.php +++ b/src/Psr16/RedisCacheEngine.php @@ -225,10 +225,7 @@ public function delete(string $key): bool } /** - * @throws NotFoundExceptionInterface - * @throws InvalidArgumentException * @throws RedisException - * @throws ContainerExceptionInterface */ #[\Override] public function clear(): bool @@ -239,7 +236,9 @@ public function clear(): bool do { $keys = $this->redis->scan($iterator, 'cache:*'); foreach($keys as $key) { - $this->delete(substr($key, 6)); + // What scan() returns is the stored key. Going back through delete() would look + // it up in the PSR-11 container, which only knows the names the application uses. + $this->redis->del($key); } } while($iterator !== 0); return true; diff --git a/src/Psr16/SessionCacheEngine.php b/src/Psr16/SessionCacheEngine.php index af87326..0e07bdd 100644 --- a/src/Psr16/SessionCacheEngine.php +++ b/src/Psr16/SessionCacheEngine.php @@ -100,13 +100,24 @@ public function set(string $key, mixed $value, DateInterval|int|null $ttl = null #[\Override] public function clear(): bool { - session_destroy(); + $this->checkSession(); + + // Only what this engine wrote. The session also carries whatever else the application + // keeps there, and other engines with their own prefix. + foreach (array_keys($_SESSION ?? []) as $keyName) { + if (str_starts_with((string)$keyName, $this->prefix . '-')) { + unset($_SESSION[$keyName]); + } + } + return true; } #[\Override] public function has(string $key): bool { + $this->checkSession(); + $keyName = $this->keyName($key); if (isset($_SESSION[$keyName])) { diff --git a/src/Psr16/ShmopCacheEngine.php b/src/Psr16/ShmopCacheEngine.php deleted file mode 100644 index 4e84470..0000000 --- a/src/Psr16/ShmopCacheEngine.php +++ /dev/null @@ -1,273 +0,0 @@ -config = $config; - - if (!isset($this->config['max-size'])) { - $this->config['max-size'] = 524288; - } - if (!isset($this->config['default-permission'])) { - $this->config['default-permission'] = '0700'; - } - - $this->logger = $logger; - if (is_null($logger)) { - $this->logger = new NullLogger(); - } - } - - /** - * @throws ContainerExceptionInterface - * @throws NotFoundExceptionInterface - * @throws InvalidArgumentException - */ - protected function getFilenameToken(string $key): string - { - $key = $this->getKeyFromContainer($key); - return sys_get_temp_dir() . '/shmop-' . sha1($key) . '.cache'; - } - - protected function getMaxSize() - { - return $this->config['max-size']; - } - - protected function getDefaultPermission() - { - return $this->config['default-permission']; - } - - protected function getFTok(string $file): int - { - if (!file_exists($file)) { - touch($file); - } - return ftok($file, 'j'); - } - - /** - * @param string $key The object KEY - * @param mixed $default The time to live in seconds of the object. Depends on implementation. - * @return mixed The Object - * @throws ContainerExceptionInterface - * @throws InvalidArgumentException - * @throws NotFoundExceptionInterface - */ - #[\Override] - public function get(string $key, mixed $default = null): mixed - { - if ($default === false) { - $this->logger->info("[Shmop Cache] Ignored $key because TTL=FALSE"); - return $default; - } - - $file = $this->getFilenameToken($key); - $fileKey = $this->getFTok($file); - - // Opened - $shm_id = @shmop_open($fileKey, "a", 0, 0); - if (!$shm_id) { - $this->logger->info("[Shmop Cache] '$key' not exists"); - return $default; - } - - if (!$this->isValidAge($file)) { - return $default; - } - - $this->logger->info("[Shmop Cache] Get '$key'"); - - $serialized = shmop_read($shm_id, 0, shmop_size($shm_id)); - // shmop_close($shm_id); - - return unserialize($serialized); - } - - protected function isValidAge(string $file): bool - { - if (file_exists("$file.ttl")) { - $fileTtl = intval(file_get_contents("$file.ttl")); - } - - if (!empty($fileTtl) && time() >= $fileTtl) { - $this->logger->info("[Shmop Cache] File too old. Ignoring"); - $this->deleteFromFilenameToken($file); - return false; - } - - return true; - } - - /** - * Persists data in the cache, uniquely referenced by a key with an optional expiration TTL time. - * - * @param string $key The key of the item to store. - * @param mixed $value The value of the item to store, must be serializable. - * @param null|int|\DateInterval $ttl Optional. The TTL value of this item. If no value is sent and - * the driver supports TTL then the library may set a default value - * for it or let the driver take care of that. - * @return bool True on success and false on failure. - * @throws ContainerExceptionInterface - * @throws InvalidArgumentException - * @throws NotFoundExceptionInterface - * @throws StorageErrorException - */ - #[\Override] - public function set(string $key, mixed $value, DateInterval|int|null $ttl = null): bool - { - $this->logger->info("[Shmop Cache] set '$key'"); - - $this->delete($key); - - $serialized = serialize($value); - $size = strlen($serialized); - - if ($size > $this->getMaxSize()) { - throw new StorageErrorException('Object is greater than the max size allowed: ' . $this->getMaxSize()); - } - - $file = $this->getFilenameToken($key); - $shmKey = $this->getFTok($file); - $shm_id = shmop_open($shmKey, "c", 0777, $size); - if (!$shm_id) { - $message = "Couldn't create shared memory segment"; - $lastError = error_get_last(); - if (isset($lastError['message'])) { - $message = $lastError['message']; - } - throw new StorageErrorException($message); - } - - $shm_bytes_written = shmop_write($shm_id, $serialized, 0); - $this->logger->info("[Shmop Cache] set '$key' confirmed write $shm_bytes_written bytes of $size bytes"); - if ($shm_bytes_written != $size) { - $this->logger->warning("Couldn't write the entire length of data"); - } - // shmop_close($shm_id); - - $validUntil = $this->addToNow($ttl); - if (!empty($validUntil)) { - file_put_contents("$file.ttl", (string)$validUntil); - } - - return true; - } - - /** - * @param string $key - * @return bool - * @throws ContainerExceptionInterface - * @throws InvalidArgumentException - * @throws NotFoundExceptionInterface - */ - #[\Override] - public function delete(string $key): bool - { - $this->logger->info("[Shmop Cache] release '$key'"); - - if ($this->get($key) === false) { - $this->logger->info("[Shmop Cache] release '$key' does not exists."); - return false; - } - - $file = $this->getFilenameToken($key); - $this->deleteFromFilenameToken($file); - return true; - } - - private function deleteFromFilenameToken(string $file): void - { - $filekey = $this->getFTok($file); - $shm_id = @shmop_open($filekey, "w", 0, 0); - - if (file_exists($file)) { - unlink($file); - } - - if (file_exists("$file.ttl")) { - unlink("$file.ttl"); - } - - if ($shm_id) { - shmop_delete($shm_id); - // shmop_close($shm_id); - - $this->logger->info("[Shmop Cache] release confirmed."); - } - } - - #[\Override] - public function clear(): bool - { - $patternKey = sys_get_temp_dir() . '/shmop-*.cache'; - $list = glob($patternKey); - foreach ($list as $file) { - $this->deleteFromFilenameToken($file); - } - return true; - } - - /** - * @throws ContainerExceptionInterface - * @throws InvalidArgumentException - * @throws NotFoundExceptionInterface - */ - #[\Override] - public function has(string $key): bool - { - $file = $this->getFilenameToken($key); - $fileKey = $this->getFTok($file); - - // Opened - $shm_id = @shmop_open($fileKey, "a", 0, 0); - - $exists = !(!$shm_id); - - if ($exists) { - // shmop_close($shm_id); - return $this->isValidAge($file); - } - - return false; - } - - - #[\Override] - public function isAvailable(): bool - { - return function_exists('shmop_open'); - } -} diff --git a/tests/CachePSR16Test.php b/tests/CachePSR16Test.php index a1c7530..3d2920d 100644 --- a/tests/CachePSR16Test.php +++ b/tests/CachePSR16Test.php @@ -9,6 +9,7 @@ use ByJG\Cache\Psr16\MemcachedEngine; use ByJG\Cache\Psr16\NoCacheEngine; use ByJG\Cache\Psr16\RedisCacheEngine; +use ByJG\Cache\Psr16\SessionCacheEngine; use PHPUnit\Framework\Attributes\DataProvider; class CachePSR16Test extends TestBase @@ -451,4 +452,162 @@ public function testAtomicOperationsHonourTheTtl(BaseCacheEngine $cacheEngine) $this->markTestIncomplete('Does not support atomic operations or it is native'); } } + + /** + * A TTL handed to increment()/decrement() applies to the key whether or not the call created + * it. Memcached only honoured it on creation, so a counter that already existed kept whatever + * expiry it had - none at all, here - and outlived the TTL the caller had just asked for. + * + * @param BaseCacheEngine $cacheEngine + * @throws \Psr\SimpleCache\InvalidArgumentException + */ + #[DataProvider('CachePoolProvider')] + public function testAtomicOperationsApplyTheTtlToAnExistingKey(BaseCacheEngine $cacheEngine) + { + $this->cacheEngine = $cacheEngine; + + if ($cacheEngine->isAvailable() && ($cacheEngine instanceof AtomicOperationInterface)) { + $this->assertEquals(1, $cacheEngine->increment('existing-up')); + $cacheEngine->set('existing-down', 10); + + $this->assertEquals(2, $cacheEngine->increment('existing-up', 1, 1)); + $this->assertEquals(9, $cacheEngine->decrement('existing-down', 1, 1)); + sleep(2); + $this->assertNull($cacheEngine->get('existing-up'), 'A 1s TTL must have elapsed'); + $this->assertNull($cacheEngine->get('existing-down'), 'A 1s TTL must have elapsed'); + } else { + $this->markTestIncomplete('Does not support atomic operations or it is native'); + } + } + + /** + * clear() and isAvailable() are about the whole engine, not about one key, so they must not + * resolve anything through the PSR-11 container. They used to: Redis deleted every scanned key + * through delete(), and FileSystem built its glob pattern and its probe path through fixKey(), + * so with a container set they died with "Key '...' not found in container". + * + * @param BaseCacheEngine $cacheEngine + * @throws \Psr\SimpleCache\InvalidArgumentException + */ + #[DataProvider('CachePoolProvider')] + public function testClearWithKeysFromContainer(BaseCacheEngine $cacheEngine) + { + if ($cacheEngine->isAvailable()) { + $cacheEngine->withKeysFromContainer(new BasicContainer()); + $cacheEngine->set("test-key", "something"); + + $this->assertTrue($cacheEngine->isAvailable()); + $this->assertTrue($cacheEngine->clear()); + $this->assertFalse($cacheEngine->has("test-key")); + } else { + $this->markTestIncomplete('Object is not fully functional'); + } + } + + /** + * Same reasoning as clear(): collecting garbage walks every entry of the engine. + * + * @param BaseCacheEngine $cacheEngine + * @throws \Psr\SimpleCache\InvalidArgumentException + */ + #[DataProvider('CachePoolProvider')] + public function testGarbageCollectorWithKeysFromContainer(BaseCacheEngine $cacheEngine) + { + if ($cacheEngine->isAvailable() && ($cacheEngine instanceof GarbageCollectorInterface)) { + $cacheEngine->withKeysFromContainer(new BasicContainer()); + $cacheEngine->set("test-key", "something", 1); + $this->assertNotNull($cacheEngine->getTtl("test-key")); + + sleep(2); + $cacheEngine->collectGarbage(); + $this->assertNull($cacheEngine->getTtl("test-key")); + } else { + $this->markTestIncomplete('Does not support garbage collector or it is native'); + } + } + + /** + * Leaving the TTL out must not change the expiry a key already has. FileSystem used to drop + * it, so a counter created with a TTL became permanent on the first call that omitted it. + * + * Memcached's add() is the one exception and is left out on purpose: it appends through cas(), + * which has to send an expiration, and Memcached offers no way to read the time a key has left. + * + * @param BaseCacheEngine $cacheEngine + * @throws \Psr\SimpleCache\InvalidArgumentException + */ + #[DataProvider('CachePoolProvider')] + public function testAtomicOperationsKeepTheExpiryWhenTheTtlIsOmitted(BaseCacheEngine $cacheEngine) + { + $this->cacheEngine = $cacheEngine; + + if ($cacheEngine->isAvailable() && ($cacheEngine instanceof AtomicOperationInterface)) { + $cacheEngine->increment('kept-up', 1, 1); + $cacheEngine->increment('kept-down', 5, 1); + $cacheEngine->add('kept-list', 'x', 1); + + $this->assertEquals(2, $cacheEngine->increment('kept-up')); + $this->assertEquals(4, $cacheEngine->decrement('kept-down')); + $this->assertEquals(['x', 'y'], $cacheEngine->add('kept-list', 'y')); + sleep(2); + $this->assertNull($cacheEngine->get('kept-up'), 'The 1s TTL must still apply'); + $this->assertNull($cacheEngine->get('kept-down'), 'The 1s TTL must still apply'); + if (!($cacheEngine instanceof MemcachedEngine)) { + $this->assertNull($cacheEngine->get('kept-list'), 'The 1s TTL must still apply'); + } + } else { + $this->markTestIncomplete('Does not support atomic operations or it is native'); + } + } + + /** + * An expired key is an absent key. FileSystem only notices the expiry when the key is read, + * so an atomic operation arriving first carried on from the value that should be gone. + * + * @param BaseCacheEngine $cacheEngine + * @throws \Psr\SimpleCache\InvalidArgumentException + */ + #[DataProvider('CachePoolProvider')] + public function testAtomicOperationsStartOverOnAnExpiredKey(BaseCacheEngine $cacheEngine) + { + $this->cacheEngine = $cacheEngine; + + if ($cacheEngine->isAvailable() && ($cacheEngine instanceof AtomicOperationInterface)) { + $cacheEngine->increment('expired-counter', 7, 1); + $cacheEngine->increment('expired-counter-no-ttl', 7, 1); + $cacheEngine->add('expired-list', 'x', 1); + sleep(2); + + $this->assertEquals(1, $cacheEngine->increment('expired-counter', 1, 30)); + $this->assertEquals(1, $cacheEngine->increment('expired-counter-no-ttl')); + $this->assertEquals(['y'], $cacheEngine->add('expired-list', 'y', 30)); + $this->assertEquals(1, $cacheEngine->get('expired-counter-no-ttl'), 'The new counter has no expiry'); + } else { + $this->markTestIncomplete('Does not support atomic operations or it is native'); + } + } + + /** + * The session belongs to the application, not to the cache. clear() used to call + * session_destroy(), which took the login and everything else stored there along with it. + */ + public function testSessionClearOnlyRemovesItsOwnEntries() + { + $cache = new SessionCacheEngine('mine'); + $other = new SessionCacheEngine('theirs'); + + $cache->set('key', 'value', 30); + $other->set('key', 'other value'); + $_SESSION['user'] = 'logged in'; + + $this->assertTrue($cache->clear()); + + $this->assertFalse($cache->has('key')); + $this->assertSame([], array_filter(array_keys($_SESSION), fn($name) => str_starts_with($name, 'mine-'))); + $this->assertEquals('other value', $other->get('key')); + $this->assertEquals('logged in', $_SESSION['user']); + + $other->clear(); + unset($_SESSION['user']); + } } diff --git a/tests/TestBase.php b/tests/TestBase.php index 9497438..f58a8b0 100644 --- a/tests/TestBase.php +++ b/tests/TestBase.php @@ -7,7 +7,6 @@ use ByJG\Cache\Psr16\FileSystemCacheEngine; use ByJG\Cache\Psr16\NoCacheEngine; use ByJG\Cache\Psr16\SessionCacheEngine; -use ByJG\Cache\Psr16\ShmopCacheEngine; use ByJG\Cache\Psr16\TmpfsCacheEngine; use PHPUnit\Framework\TestCase; @@ -40,9 +39,6 @@ public static function CachePoolProvider() 'Tmpfs' => [ new TmpfsCacheEngine() ], - 'ShmopCache' => [ - new ShmopCacheEngine() - ], 'SessionCache' => [ new SessionCacheEngine() ], From 72ef48a3c91f0c8590fc1324ea24010546133d7e Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Fri, 16 Oct 2026 08:50:30 -0400 Subject: [PATCH 12/15] Skip, not mark incomplete, the tests an engine does not support --- tests/CachePSR16Test.php | 36 ++++++++++++++++++------------------ tests/CachePSR6Test.php | 8 ++++---- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/tests/CachePSR16Test.php b/tests/CachePSR16Test.php index 3d2920d..6ecb78d 100644 --- a/tests/CachePSR16Test.php +++ b/tests/CachePSR16Test.php @@ -46,7 +46,7 @@ public function testGetOneItem(BaseCacheEngine $cacheEngine) $item = $cacheEngine->get('chave'); $this->assertNull($item); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -87,7 +87,7 @@ public function testGetMultipleItems(BaseCacheEngine $cacheEngine) $this->assertNull($items['chave1']); $this->assertNull($items['chave2']); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -124,7 +124,7 @@ public function testTtl(BaseCacheEngine $cacheEngine) $this->assertFalse($cacheEngine->has('chave2')); } } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -157,7 +157,7 @@ public function testCacheObject(BaseCacheEngine $cacheEngine) $item = $cacheEngine->get('chave'); $this->assertNull($item); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -195,7 +195,7 @@ public function testCacheArray(BaseCacheEngine $cacheEngine) $item = $cacheEngine->get('chave'); $this->assertNull($item); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -237,7 +237,7 @@ public function testClear(BaseCacheEngine $cacheEngine) $item = $cacheEngine->getMultiple(['chave', 'chave2', 'chave3']); $this->assertEquals($empty, $item); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -255,7 +255,7 @@ public function testCacheContainerKeyNonExistent(BaseCacheEngine $cacheEngine) $cacheEngine->withKeysFromContainer(new BasicContainer()); $cacheEngine->set("abc", "30"); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -282,7 +282,7 @@ public function testCacheContainerKey(BaseCacheEngine $cacheEngine) $this->assertTrue($cacheEngine->has("container-key")); $this->assertEquals("something", $cacheEngine->get("container-key")); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -315,7 +315,7 @@ public function testGarbageCollector(BaseCacheEngine $cacheEngine) $this->assertNull($cacheEngine->getTtl('chave')); // Should be deleted $this->assertFalse($cacheEngine->has('chave')); } else { - $this->markTestIncomplete('Does not support garbage collector or it is native'); + $this->markTestSkipped('Does not support garbage collector or it is native'); } } @@ -337,7 +337,7 @@ public function testAtomicIncrement(BaseCacheEngine $cacheEngine) $this->assertEquals(15, $cacheEngine->increment('chave')); $this->assertEquals(15, $cacheEngine->get('chave')); } else { - $this->markTestIncomplete('Does not support atomic increment or it is native'); + $this->markTestSkipped('Does not support atomic increment or it is native'); } } @@ -359,7 +359,7 @@ public function testAtomicDecrement(BaseCacheEngine $cacheEngine) $this->assertEquals(5, $cacheEngine->decrement('chave')); $this->assertEquals(5, $cacheEngine->get('chave')); } else { - $this->markTestIncomplete('Does not support atomic decrement or it is native'); + $this->markTestSkipped('Does not support atomic decrement or it is native'); } } @@ -396,7 +396,7 @@ public function testAtomicAdd(BaseCacheEngine $cacheEngine) } else { - $this->markTestIncomplete('Does not support atomic add or it is native'); + $this->markTestSkipped('Does not support atomic add or it is native'); } } @@ -449,7 +449,7 @@ public function testAtomicOperationsHonourTheTtl(BaseCacheEngine $cacheEngine) $this->assertNull($cacheEngine->get('expiring-counter'), 'A 1s TTL must have elapsed'); $this->assertNull($cacheEngine->get('expiring-list'), 'A 1s TTL must have elapsed'); } else { - $this->markTestIncomplete('Does not support atomic operations or it is native'); + $this->markTestSkipped('Does not support atomic operations or it is native'); } } @@ -476,7 +476,7 @@ public function testAtomicOperationsApplyTheTtlToAnExistingKey(BaseCacheEngine $ $this->assertNull($cacheEngine->get('existing-up'), 'A 1s TTL must have elapsed'); $this->assertNull($cacheEngine->get('existing-down'), 'A 1s TTL must have elapsed'); } else { - $this->markTestIncomplete('Does not support atomic operations or it is native'); + $this->markTestSkipped('Does not support atomic operations or it is native'); } } @@ -500,7 +500,7 @@ public function testClearWithKeysFromContainer(BaseCacheEngine $cacheEngine) $this->assertTrue($cacheEngine->clear()); $this->assertFalse($cacheEngine->has("test-key")); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -522,7 +522,7 @@ public function testGarbageCollectorWithKeysFromContainer(BaseCacheEngine $cache $cacheEngine->collectGarbage(); $this->assertNull($cacheEngine->getTtl("test-key")); } else { - $this->markTestIncomplete('Does not support garbage collector or it is native'); + $this->markTestSkipped('Does not support garbage collector or it is native'); } } @@ -556,7 +556,7 @@ public function testAtomicOperationsKeepTheExpiryWhenTheTtlIsOmitted(BaseCacheEn $this->assertNull($cacheEngine->get('kept-list'), 'The 1s TTL must still apply'); } } else { - $this->markTestIncomplete('Does not support atomic operations or it is native'); + $this->markTestSkipped('Does not support atomic operations or it is native'); } } @@ -583,7 +583,7 @@ public function testAtomicOperationsStartOverOnAnExpiredKey(BaseCacheEngine $cac $this->assertEquals(['y'], $cacheEngine->add('expired-list', 'y', 30)); $this->assertEquals(1, $cacheEngine->get('expired-counter-no-ttl'), 'The new counter has no expiry'); } else { - $this->markTestIncomplete('Does not support atomic operations or it is native'); + $this->markTestSkipped('Does not support atomic operations or it is native'); } } diff --git a/tests/CachePSR6Test.php b/tests/CachePSR6Test.php index cedd0d6..8a818cf 100644 --- a/tests/CachePSR6Test.php +++ b/tests/CachePSR6Test.php @@ -42,7 +42,7 @@ public function testGetOneItem(BaseCacheEngine $cacheEngine) $item = $object->getItem('chave'); $this->assertFalse($item->isHit()); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -86,7 +86,7 @@ public function testGetMultipleItems(BaseCacheEngine $cacheEngine) $this->assertFalse($items[0]->isHit()); $this->assertFalse($items[1]->isHit()); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } @@ -133,7 +133,7 @@ public function testTtl(BaseCacheEngine $cacheEngine) $item = $object->getItem('chave'); $this->assertFalse($item->isHit()); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } } @@ -171,7 +171,7 @@ public function testCacheObject(BaseCacheEngine $cacheEngine) $item = $object->getItem('chave'); $this->assertFalse($item->isHit()); } else { - $this->markTestIncomplete('Object is not fully functional'); + $this->markTestSkipped('Object is not fully functional'); } } } From bc6ffdaff28f88f230f0a455bf20e27f94e7b1d6 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Sat, 17 Oct 2026 08:05:57 -0400 Subject: [PATCH 13/15] Align documentation and code. --- docs/class-memcached-engine.md | 2 ++ docs/class-redis-cache-engine.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/docs/class-memcached-engine.md b/docs/class-memcached-engine.md index e59e725..a517da2 100644 --- a/docs/class-memcached-engine.md +++ b/docs/class-memcached-engine.md @@ -18,6 +18,8 @@ $servers = [ ]; ``` +When `$servers` is `null` it defaults to `['127.0.0.1:11211']`; a server without a port uses `11211`. + You can also pass Memcached client options (no need to pass a Memcached instance). Options can be provided as an associative array where the keys are Memcached option constants or their string names: ```php diff --git a/docs/class-redis-cache-engine.md b/docs/class-redis-cache-engine.md index 4f1fa2e..adcbb5d 100644 --- a/docs/class-redis-cache-engine.md +++ b/docs/class-redis-cache-engine.md @@ -14,6 +14,8 @@ The constructor expects a string with the server and port. $server = 'localhost:5678' ``` +When `$server` is `null` it defaults to `127.0.0.1:6379`; a server without a port uses `6379`. + ## PSR-16 Constructor ```php From 0264c14b05f193d96aa881209d51dc7cc330e710 Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Mon, 19 Oct 2026 11:03:07 -0400 Subject: [PATCH 14/15] Remove the KeyValueCacheEngine row from the README The class was removed from byjg/anydataset-nosql 7.0. --- README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/README.md b/README.md index 0347ca6..d0a6def 100644 --- a/README.md +++ b/README.md @@ -67,7 +67,6 @@ $value = $item->get(); | [RedisCacheEngine](docs/class-redis-cache-engine.md) | Redis-based caching | | [SessionCacheEngine](docs/class-session-cache-engine.md) | PHP session-based caching | | [TmpfsCacheEngine](docs/class-tmpfs-cache-engine.md) | Tmpfs-based caching | -| [KeyValueCacheEngine](https://github.com/byjg/php-anydataset-nosql) | S3-Like or CloudflareKV storage (separate package) | ### Advanced Features - [Atomic Operations](docs/atomic-operations.md) From d25d9d9d705c9ea36446a6f951d989ae18277ffc Mon Sep 17 00:00:00 2001 From: Joao Gilberto Magalhaes Date: Mon, 19 Oct 2026 17:22:24 -0400 Subject: [PATCH 15/15] Fix the CachePool error class, remove NoCacheEngine lock/unlock and type the Memcached logger --- CHANGELOG-7.0.md | 14 ++++++++++++++ src/Psr16/MemcachedEngine.php | 6 +++--- src/Psr16/NoCacheEngine.php | 18 ------------------ src/Psr6/CachePool.php | 2 +- tests/CachePSR16Test.php | 11 +++++++++++ tests/CachePSR6Test.php | 23 +++++++++++++++++++++++ 6 files changed, 52 insertions(+), 22 deletions(-) diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md index 1d2961b..910caf8 100644 --- a/CHANGELOG-7.0.md +++ b/CHANGELOG-7.0.md @@ -23,6 +23,8 @@ concurrent `increment()` calls on Memcached ended at **8** instead of 20, and tw | **Session `clear()`** | `session_destroy()` | removes only the entries of the engine's prefix | `SessionCacheEngine::clear()` destroyed the whole session — the login and everything else the application kept there. It now unsets only its own `-*` entries. Code that used it to end the session must call `session_destroy()` itself. | | **FileSystem atomic operations without a TTL** | dropped the expiry of the key | keep it | `increment()`, `decrement()` and `add()` called without a TTL made a key that had an expiry permanent. They now leave the expiry alone, as Redis does. | | **FileSystem atomic TTL** | raw `$ttl` written as the expiry timestamp | converted with `addToNow()` | `increment($key, 1, 60)` wrote `60` into the expiry file — a moment in 1970 — so the entry was already expired when written. It now means 60 seconds from now, and the value survives as intended. | +| **`NoCacheEngine::lock()` / `unlock()` removed** | public no-op methods | removed | No interface declared them and no other engine had them, so nothing could call them through an abstraction. Code calling them directly on a `NoCacheEngine` must drop the call. | +| **`MemcachedEngine` logger argument** | untyped; a non-`LoggerInterface` value silently became a `NullLogger` | `?LoggerInterface` | Matches the other engines. Passing anything other than a logger or `null` now throws `TypeError` instead of quietly disabling logging. | ## New Features @@ -158,6 +160,18 @@ and `delete()`. Called first in a request it always answered `false`, and called immortal — a reader arriving in that window saw an entry that should already have expired. The delete now happens under the lock. +### PSR-6: `save()` named a class that does not exist + +Saving an item that is not a `ByJG\Cache\Psr6\CacheItem` threw an exception asking for +`\ByJG\Cache\Psr\CacheItem`, a namespace that does not exist. The message now names the real +class. + +### Memcached: a wrong logger was swallowed + +`MemcachedEngine::__construct()` took `$logger` untyped and replaced anything that was not a +`LoggerInterface` with a `NullLogger`, so a mistake there silently turned logging off. The argument +is now typed `?LoggerInterface`, as in every other engine (see Breaking Changes). + ## Testing - `tests/CompareAndSwapTest.php` — 18 tests across both engines covering expiry, non-owner diff --git a/src/Psr16/MemcachedEngine.php b/src/Psr16/MemcachedEngine.php index eabbcdd..ed7ba9a 100644 --- a/src/Psr16/MemcachedEngine.php +++ b/src/Psr16/MemcachedEngine.php @@ -33,7 +33,7 @@ class MemcachedEngine extends BaseCacheEngine implements AtomicOperationInterfac protected ?array $options = null; - public function __construct(?array $servers = null, $logger = null, ?array $options = null) + public function __construct(?array $servers = null, ?LoggerInterface $logger = null, ?array $options = null) { $this->servers = (array)$servers; if (is_null($servers)) { @@ -42,8 +42,8 @@ public function __construct(?array $servers = null, $logger = null, ?array $opti ]; } - $this->logger = $logger instanceof LoggerInterface ? $logger : null; - if (is_null($this->logger)) { + $this->logger = $logger; + if (is_null($logger)) { $this->logger = new NullLogger(); } diff --git a/src/Psr16/NoCacheEngine.php b/src/Psr16/NoCacheEngine.php index 8ffdb05..c3cb164 100644 --- a/src/Psr16/NoCacheEngine.php +++ b/src/Psr16/NoCacheEngine.php @@ -54,24 +54,6 @@ public function delete(string $key): bool return true; } - /** - * Lock resource before set it. - * @param string $key - */ - public function lock(string $key): void - { - return; - } - - /** - * UnLock resource after set it - * @param string $key - */ - public function unlock(string $key): void - { - return; - } - #[\Override] public function isAvailable(): bool { diff --git a/src/Psr6/CachePool.php b/src/Psr6/CachePool.php index 949baf4..d12b704 100644 --- a/src/Psr6/CachePool.php +++ b/src/Psr6/CachePool.php @@ -218,7 +218,7 @@ public function deleteItems(array $keys): bool public function save(CacheItemInterface $item): bool { if (!($item instanceof CacheItem)) { - throw new InvalidArgumentException('The cache item must be an implementation of \ByJG\Cache\Psr\CacheItem'); + throw new InvalidArgumentException('The cache item must be an implementation of ' . CacheItem::class); } if ($item->getExpiresInSecs() < 1) { diff --git a/tests/CachePSR16Test.php b/tests/CachePSR16Test.php index 6ecb78d..fb4c95c 100644 --- a/tests/CachePSR16Test.php +++ b/tests/CachePSR16Test.php @@ -610,4 +610,15 @@ public function testSessionClearOnlyRemovesItsOwnEntries() $other->clear(); unset($_SESSION['user']); } + + /** + * The logger argument was untyped and anything that was not a LoggerInterface was silently + * replaced by a NullLogger, hiding the mistake. It is now typed like every other engine. + */ + public function testMemcachedRejectsALoggerOfTheWrongType() + { + $this->expectException(\TypeError::class); + /** @psalm-suppress InvalidArgument */ + new MemcachedEngine(null, 'not-a-logger'); + } } diff --git a/tests/CachePSR6Test.php b/tests/CachePSR6Test.php index 8a818cf..518bb6f 100644 --- a/tests/CachePSR6Test.php +++ b/tests/CachePSR6Test.php @@ -2,9 +2,12 @@ namespace Tests; +use ByJG\Cache\Exception\InvalidArgumentException as CacheInvalidArgumentException; +use ByJG\Cache\Psr16\ArrayCacheEngine; use ByJG\Cache\Psr16\BaseCacheEngine; use ByJG\Cache\Psr6\CachePool; use DateInterval; +use DateTimeInterface; use PHPUnit\Framework\Attributes\DataProvider; use Psr\SimpleCache\InvalidArgumentException; @@ -174,4 +177,24 @@ public function testCacheObject(BaseCacheEngine $cacheEngine) $this->markTestSkipped('Object is not fully functional'); } } + + /** + * The message used to name a namespace that does not exist (\ByJG\Cache\Psr\CacheItem). + */ + public function testSaveRejectsAForeignCacheItemNamingTheRealClass() + { + $foreignItem = new class implements \Psr\Cache\CacheItemInterface { + public function getKey(): string { return 'key'; } + public function get(): mixed { return null; } + public function isHit(): bool { return false; } + public function set(mixed $value): static { return $this; } + public function expiresAt(?DateTimeInterface $expiration): static { return $this; } + public function expiresAfter(int|\DateInterval|null $time): static { return $this; } + }; + + $this->expectException(CacheInvalidArgumentException::class); + $this->expectExceptionMessage('The cache item must be an implementation of ByJG\Cache\Psr6\CacheItem'); + + (new CachePool(new ArrayCacheEngine()))->save($foreignItem); + } }