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 845ab02..bc758ad 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" @@ -43,7 +44,7 @@ jobs: --health-retries 5 steps: - - uses: actions/checkout@v5 + - uses: actions/checkout@v7 - run: composer install - run: composer test @@ -54,12 +55,12 @@ 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: - name: Git checkout - uses: actions/checkout@v4 + uses: actions/checkout@v7 - name: Composer run: composer install @@ -67,12 +68,13 @@ jobs: - 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: ./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-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) diff --git a/CHANGELOG-7.0.md b/CHANGELOG-7.0.md new file mode 100644 index 0000000..910caf8 --- /dev/null +++ b/CHANGELOG-7.0.md @@ -0,0 +1,317 @@ +# 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 +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. | +| **`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. | +| **`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 + +### 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 — 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 + +### 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. + +### `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. + +### 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 +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 + 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. +- 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. + +## 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: 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: + +```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 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 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. + +## Continuous Integration + +- The build matrix now includes 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 + +- `phpunit.xml.dist` renamed to `phpunit.xml`. diff --git a/README.md b/README.md index c297706..d0a6def 100644 --- a/README.md +++ b/README.md @@ -9,9 +9,10 @@ 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/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 @@ -20,16 +21,19 @@ 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 -## Quick Start +## Installation ```bash composer require "byjg/cache-engine" ``` +## Quick Start + ```php // PSR-16 Simple Cache $cache = new \ByJG\Cache\Psr16\FileSystemCacheEngine(); @@ -63,32 +67,20 @@ $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 - [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) -## Running Unit Tests +## Running Tests -``` -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 +```bash +docker compose up -d +vendor/bin/phpunit ``` ---- -[Open source ByJG](http://opensource.byjg.com) +[Open source ByJG](https://opensource.byjg.com) diff --git a/composer.json b/composer.json index 166244d..e783134 100644 --- a/composer.json +++ b/composer.json @@ -12,28 +12,27 @@ } }, "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": "^1.0|^2.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", + "psalm/phar": "^6.16" }, "suggest": { "ext-memcached": "*", - "ext-redis": "*", - "ext-shmop": "*" + "ext-redis": "*" }, "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", - "psalm": "vendor/bin/psalm --threads=1" + "psalm": "vendor/bin/psalm.phar" }, "license": "MIT", "prefer-stable": true, diff --git a/docs/atomic-operations.md b/docs/atomic-operations.md index 542b8cf..7217192 100644 --- a/docs/atomic-operations.md +++ b/docs/atomic-operations.md @@ -24,29 +24,60 @@ 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). + +## 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 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/basic-usage-psr16-simplecache.md b/docs/basic-usage-psr16-simplecache.md index 3850571..52dced3 100644 --- a/docs/basic-usage-psr16-simplecache.md +++ b/docs/basic-usage-psr16-simplecache.md @@ -46,6 +46,20 @@ $cacheEngine = new \ByJG\Cache\Psr16\FileSystemCacheEngine(); $cacheEngine->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-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 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/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 +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/phpunit.xml.dist b/phpunit.xml similarity index 88% rename from phpunit.xml.dist rename to phpunit.xml index 365de7f..c3c976d 100644 --- a/phpunit.xml.dist +++ 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 b208114..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 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/src/CompareAndSwapInterface.php b/src/CompareAndSwapInterface.php new file mode 100644 index 0000000..c4ddb43 --- /dev/null +++ b/src/CompareAndSwapInterface.php @@ -0,0 +1,65 @@ +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)) { @@ -211,11 +216,10 @@ protected function putContents(string $fileKey, mixed $value, ?int $ttl, ?Closur { $returnValue = true; - if (file_exists("$fileKey.ttl")) { - unlink("$fileKey.ttl"); - } - if (is_null($value)) { + if (file_exists("$fileKey.ttl")) { + unlink("$fileKey.ttl"); + } if (file_exists($fileKey)) { unlink($fileKey); } @@ -226,13 +230,22 @@ 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. 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)) { - if (!file_exists($fileKey)) { - $currentValue = 0; - } else { - $content = file_get_contents($fileKey); - $currentValue = !empty($content) ? unserialize($content) : $content; - } + $content = $expired ? '' : file_get_contents($fileKey); + $currentValue = !empty($content) ? unserialize($content) : 0; $value = $returnValue = $operation($currentValue, $value); } file_put_contents($fileKey, serialize($value)); @@ -250,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)); @@ -279,7 +292,10 @@ public function getTtl(string $key): ?int #[Override] public function increment(string $key, int $value = 1, DateInterval|int|null $ttl = null): int { - return $this->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 +303,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 +311,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..ed7ba9a 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; /** * @@ -27,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)) { @@ -36,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(); } @@ -231,16 +237,14 @@ 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->refreshTtl($fixKey, $ttl); $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 +261,52 @@ 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->refreshTtl($fixKey, $ttl); $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); + } + + /** + * 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 @@ -284,29 +319,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/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/Psr16/RedisCacheEngine.php b/src/Psr16/RedisCacheEngine.php index 20b9d26..d82b0e4 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); } } @@ -151,19 +225,20 @@ public function delete(string $key): bool } /** - * @throws NotFoundExceptionInterface - * @throws InvalidArgumentException * @throws RedisException - * @throws ContainerExceptionInterface */ #[\Override] public function clear(): bool { + $this->lazyLoadRedisServer(); + $iterator = null; 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; @@ -178,6 +253,8 @@ public function clear(): bool #[\Override] public function has(string $key): bool { + $this->lazyLoadRedisServer(); + $result = $this->redis->exists($this->fixKey($key)); return (bool)$result; } @@ -200,55 +277,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)]); + } - return $result; + /** + * 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 $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); + } - $result = $this->redis->decr($this->fixKey($key), $value); + $this->convertStringToList($key); + } - if ($ttl) { - $this->redis->expire($this->fixKey($key), $this->convertToSeconds($ttl)); + throw new StorageErrorException("Could not append to '$key': the key kept changing type"); + } + + /** + * 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; + } - $result = $this->redis->rPush($fixKey, is_object($value) || is_array($value) ? serialize($value) : $value); + #[\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)] + ); + } + + /** 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/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/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 fa64c0a..fb4c95c 100644 --- a/tests/CachePSR16Test.php +++ b/tests/CachePSR16Test.php @@ -6,7 +6,10 @@ 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 ByJG\Cache\Psr16\SessionCacheEngine; use PHPUnit\Framework\Attributes\DataProvider; class CachePSR16Test extends TestBase @@ -43,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'); } } @@ -84,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'); } } @@ -121,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'); } } @@ -154,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'); } } @@ -192,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'); } } @@ -234,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'); } } @@ -252,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'); } } @@ -279,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'); } } @@ -312,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'); } } @@ -334,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'); } } @@ -356,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'); } } @@ -393,7 +396,229 @@ 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'); } } + + /** + * 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 + * 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->markTestSkipped('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->markTestSkipped('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->markTestSkipped('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->markTestSkipped('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->markTestSkipped('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->markTestSkipped('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']); + } + + /** + * 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 cedd0d6..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; @@ -42,7 +45,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 +89,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 +136,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 +174,27 @@ 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'); } } + + /** + * 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); + } } diff --git a/tests/CompareAndSwapTest.php b/tests/CompareAndSwapTest.php new file mode 100644 index 0000000..33f765c --- /dev/null +++ b/tests/CompareAndSwapTest.php @@ -0,0 +1,161 @@ + [EngineFactory::REDIS], + 'memcached' => [EngineFactory::MEMCACHED], + ]; + } + + #[\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(string $engineName): BaseCacheEngine&CompareAndSwapInterface + { + $engine = EngineFactory::make($engineName); + if (!$engine->isAvailable()) { + $this->markTestSkipped(get_class($engine) . ' is not available'); + } + + $this->started[] = $engine; + $engine->delete('cas-key'); + + return $engine; + } + + #[DataProvider('engineProvider')] + public function testSetIfAbsentSucceedsOnlyForTheFirstCaller(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $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(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $this->assertTrue($engine->setIfAbsent('cas-key', 'token-a', 1)); + $this->assertFalse($engine->setIfAbsent('cas-key', 'token-b', 1)); + + sleep(2); + + $this->assertTrue($engine->setIfAbsent('cas-key', 'token-b', 30)); + $this->assertEquals('token-b', $engine->get('cas-key')); + } + + #[DataProvider('engineProvider')] + public function testSetIfAbsentAppliesTheTtlWithTheWrite(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $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(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $engine->setIfAbsent('cas-key', 'token-a', 30); + + $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($engine->deleteIfEquals('cas-key', 'token-a')); + $this->assertNull($engine->get('cas-key')); + } + + #[DataProvider('engineProvider')] + public function testDeleteIfEqualsOnAMissingKeyReturnsFalse(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $this->assertFalse($engine->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(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $engine->setIfAbsent('cas-key', 'stale-owner', 1); + sleep(2); + $this->assertTrue($engine->setIfAbsent('cas-key', 'new-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(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $engine->setIfAbsent('cas-key', 'token-a', 1); + $this->assertTrue($engine->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(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $engine->setIfAbsent('cas-key', 'token-a', 1); + $this->assertFalse($engine->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(string $engineName): void + { + $engine = $this->engineOrSkip($engineName); + + $this->assertTrue($engine->setIfAbsent('cas-key', ['a' => 1, 'b' => 2], 30)); + $this->assertEquals(['a' => 1, 'b' => 2], $engine->get('cas-key')); + + $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 new file mode 100644 index 0000000..61ba1a6 --- /dev/null +++ b/tests/ConcurrencyTest.php @@ -0,0 +1,208 @@ + [EngineFactory::REDIS], + 'memcached' => [EngineFactory::MEMCACHED], + ]; + } + + #[\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 $engineName): BaseCacheEngine&CompareAndSwapInterface + { + $engine = EngineFactory::make($engineName); + if (!$engine->isAvailable()) { + $this->markTestSkipped("$engineName 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 $engineName): void + { + $this->engineOrSkip($engineName)->delete('race-lock'); + + $codes = $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); + 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 $engineName): void + { + $this->engineOrSkip($engineName)->delete('race-counter'); + + $this->fork(function () use ($engineName): bool { + $engine = EngineFactory::make($engineName); + $engine->increment('race-counter'); + return true; + }); + + $this->assertEquals( + self::CHILDREN, + $this->engineOrSkip($engineName)->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 $engineName): void + { + $this->engineOrSkip($engineName)->delete('race-list'); + + $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); + $engine->add('race-list', "item-$i"); + return true; + }); + + $stored = $this->engineOrSkip($engineName)->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 $engineName): void + { + $setup = $this->engineOrSkip($engineName); + $setup->delete('race-convert'); + $setup->set('race-convert', 'seed'); + + $this->fork(function (int $i) use ($engineName): bool { + $engine = EngineFactory::make($engineName); + $engine->add('race-convert', "item-$i"); + return true; + }); + + $stored = $this->engineOrSkip($engineName)->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 $engineName): void + { + $setup = $this->engineOrSkip($engineName); + $setup->delete('race-owned'); + $setup->setIfAbsent('race-owned', 'the-owner', 60); + + $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($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..f58a8b0 100644 --- a/tests/TestBase.php +++ b/tests/TestBase.php @@ -5,11 +5,8 @@ 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; use PHPUnit\Framework\TestCase; @@ -32,22 +29,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() @@ -58,9 +39,6 @@ public static function CachePoolProvider() 'Tmpfs' => [ new TmpfsCacheEngine() ], - 'ShmopCache' => [ - new ShmopCacheEngine() - ], 'SessionCache' => [ new SessionCacheEngine() ], @@ -68,10 +46,10 @@ public static function CachePoolProvider() new NoCacheEngine() ], 'Memcached' => [ - new MemcachedEngine($memcachedServer) + EngineFactory::memcached() ], 'Redis' => [ - new RedisCacheEngine($redisCacheServer, $redisPassword) + EngineFactory::redis() ], 'Memory' => [ new TmpfsCacheEngine()