diff --git a/docs/cache.md b/docs/cache.md index 5002dc3..acc5965 100644 --- a/docs/cache.md +++ b/docs/cache.md @@ -291,18 +291,20 @@ return [ 'allowed_classes' => [], // extra classes the persistent drivers may store 'stores' => [ 'database' => [ - 'driver' => 'database', - 'forever_ttl' => 10 * YEAR_IN_SECONDS, - 'network' => false, - 'gc' => 'daily', // false disables the flush cleanup sweep - 'events' => true, + 'driver' => 'database', + 'forever_ttl' => 10 * YEAR_IN_SECONDS, + 'network' => false, + 'gc' => 'daily', // false disables the flush cleanup sweep + 'events' => true, + 'version_scoped' => false, // true starts the store cold on each app version change ], 'file' => [ - 'driver' => 'file', - 'path' => null, // null derives one beneath uploads - 'fallback' => 'database', - 'gc' => 'daily', // false disables the sweep - 'events' => true, + 'driver' => 'file', + 'path' => null, // null derives one beneath uploads + 'fallback' => 'database', + 'gc' => 'daily', // false disables the sweep + 'events' => true, + 'version_scoped' => false, // true starts the store cold on each app version change ], 'array' => ['driver' => 'array', 'events' => false], ], @@ -323,6 +325,37 @@ The classes the `database` and `file` drivers may store, on top of the built-in It applies to every store rather than per store, because a `file` store can silently divert to its `fallback`. A value must not be accepted by one and refused by the other. The list is only ever read from configuration, never from stored data. +### `version_scoped` + +A store with `'version_scoped' => true` starts cold whenever the application version changes. Turn it on for a store that holds values whose **shape your own code produces**, such as a remote JSON list after your plugin normalizes it. Without it, an update that changes that shape reads entries written by the previous release under the same key. In one real case, 1.0.1 read a list cached by 1.0.0 that had no `slug` field, and the WordPress update check crashed with a `TypeError`. + +```php +'stores' => [ + 'database' => [ + 'driver' => 'database', + 'version_scoped' => true, + ], +], +``` + +How it works: + +- The first time a request uses the store, the framework compares `app()->version()` with the version recorded in the option `cache_app_version_`. This is a network-wide site option when the store has `'network' => true`. +- If the two differ, the store is flushed with the ordinary `flush()`, and the new version is recorded. A downgrade counts as a change. +- A **missing** recorded version also counts as a change. So the first request after you turn the setting on, or after upgrading from a framework release without it, flushes once. +- The check runs at most once per store per request, and only in requests that use the store. The option is autoloaded, so reading it costs no query. +- The version is recorded only when `flush()` succeeds, so a failed flush is retried on the next request. +- A `file` store that diverts to its `fallback` is checked under the fallback's name, using the fallback's `version_scoped` setting. +- It has no effect on the `array` store, which lives for one request anyway. + +What it costs: + +- **Every update starts with a cold cache.** Only turn it on for entries you can rebuild from their source. Leave it off for any store that holds values you cannot recreate. +- **The flush reaches every store that shares the namespace.** Every `database` store is namespaced by the application prefix alone, not by store name, and so is every `file` store on the default path. Scoping one of them therefore empties all of them. +- **Rate-limiter counts start again.** This happens when `cache.limiter` is `null` (the limiter counts in the default store), or when the limiter's store shares the namespace with a scoped store, as the previous point describes. To keep counts across an update, give the limiter a store on a different backend. + +The flush goes through the store's normal `flush()`, not a version suffix in the key prefix. That keeps the superseded entries in the scheduled sweep described in [section 9](#9-the-drivers): a moved prefix would strand the `database` driver's stale-version queue, and every `forever()` row and `file` directory with it. + --- ## 11. Events @@ -343,7 +376,7 @@ To share a store across an entire network, set `'network' => true`. Its entries ## 13. Where this differs from Laravel -Five deliberate divergences, each forced by WordPress: +Six deliberate divergences, each forced by WordPress: **`forever()` uses a long finite lifetime.** WordPress marks a transient with no expiry as an *autoloaded* option, and skips its own 150,000-byte autoload guard when doing so — so a large `forever()` value would be read and unserialized on every request, at a cost paid before your code runs and therefore nearly undiagnosable. The entry never logically expires as far as the cache API is concerned, while the underlying transient is given a ten-year lifetime so WordPress leaves it out of autoload. Set `forever_ttl` to `0` for literal never-expiry. Note that Laravel does not treat `forever` as durable either — its own documentation warns that Memcached may evict such entries. @@ -355,6 +388,8 @@ Five deliberate divergences, each forced by WordPress: **`flush()` advances a namespace version.** Necessary because transients cannot be enumerated under an object cache. The practical effect is the same — every prior key reads as a miss — and it takes constant time. Reclaiming the rows a version leaves behind happens later, off the request that called `flush()`, in the same scheduled sweep described in [section 9](#9-the-drivers). +**`version_scoped` has no Laravel counterpart.** Laravel leaves invalidating a cache on deploy to the deploy process (`php artisan cache:clear`). A WordPress plugin update has no such step, so the framework provides one. See [section 10](#version_scoped). + `Cache::lock()` **does** exist — see [section 15](#15-locks) — though only the acquire, release and callback forms; `block()`, `funnel()` and `without_overlapping()` do not. Absent by design: tagged cache (`supports_tags()` returns `false`), the `failover` driver, `sear`, and `rememberWithWarmth`. --- diff --git a/example/config/cache.php b/example/config/cache.php index 1ddbbba..32e6b65 100644 --- a/example/config/cache.php +++ b/example/config/cache.php @@ -83,6 +83,21 @@ * this on costs almost nothing. */ 'events' => true, + + /* + * When true, the store starts over whenever the application + * version changes: the first request on the new version that uses + * the store flushes it once. A store with no recorded version + * counts as changed, so turning this on flushes once too. + * + * Turn this on for stores holding values whose shape your own code + * produces, so an update that changes the shape never reads an + * entry written by the previous release. The cost is a cold cache + * after every update. Every database store shares one namespace, + * so this flushes all of them, and the rate limiter's counts start + * again if it counts in one of them. + */ + 'version_scoped' => false, ], /* @@ -123,6 +138,21 @@ 'network' => false, 'events' => true, + + /* + * When true, the store starts over whenever the application + * version changes: the first request on the new version that uses + * the store flushes it once. A store with no recorded version + * counts as changed, so turning this on flushes once too. + * + * Turn this on for stores holding values whose shape your own code + * produces, so an update that changes the shape never reads an + * entry written by the previous release. The cost is a cold cache + * after every update. Every file store on the default path shares + * one directory, so this flushes all of them, and the rate + * limiter's counts start again if it counts in one of them. + */ + 'version_scoped' => false, ], /* diff --git a/openspec/changes/add-cache-version-scoping/.openspec.yaml b/openspec/changes/add-cache-version-scoping/.openspec.yaml new file mode 100644 index 0000000..2b638f8 --- /dev/null +++ b/openspec/changes/add-cache-version-scoping/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-11-10 diff --git a/openspec/changes/add-cache-version-scoping/design.md b/openspec/changes/add-cache-version-scoping/design.md new file mode 100644 index 0000000..6ace78a --- /dev/null +++ b/openspec/changes/add-cache-version-scoping/design.md @@ -0,0 +1,146 @@ +## Context + +See proposal.md (Why) for the motivation, and specs/cache-version-scoping/spec.md for the +requirements. + +Current state that shapes the approach: + +- `CacheManager::store()` resolves a store once per request through `resolve($name)` and memoises + the repository in `$this->stores[$name]`. `memo()` goes through `store()`, and so does the rate + limiter (`RateLimiter::cache()`). +- `Repository::flush()` clears the `MemoTable` for the store, calls `Store::flush()`, and fires + `CacheFlushed` when the flush returns `true`. +- `DatabaseStore::flush()` increments `cache_version` and queues the old version in + `cache_stale_versions` for `gc()`. `FileStore::flush()` deletes its directory. +- `DatabaseStore` takes its scope from the `network` constructor flag and reads options through + `get_site_option()` / `get_option()` (`read_option()`). +- `create_file_repository()` catches a store that cannot run and returns + `$this->store($fallback)`. That repository carries the **fallback's** name. +- Every `database`-driver store is built from `$this->prefix()` alone, and so is the default `file` + directory. Neither includes the store name, so all stores on one driver share one namespace. +- `Application::version()` returns a string (default `'1.0.0'`, set with `use_version()`). A + container that is not a `Framework\Application` may have no `version()`. + +## Goals / Non-Goals + +**Goals:** + +- Flush a scoped store at most once per version change, in the first request that uses it. +- Add no database query in the steady state. +- Keep stores that are not scoped exactly as they are. + +**Non-Goals:** + +- Per-store namespaces for stores that share a driver. This is documented as a caveat, not fixed. +- Guarding concurrent first requests with a lock. Two requests may both flush, and the namespace + version then rises by two. This is safe. +- Changing how `lock()` derives its prefix. This is reported as a known issue in the PR. + +## Decisions + +### D1. Flush through the existing namespace, not a version in the prefix + +The proposal records the reasoning ("Why a flush rather than a version in the key prefix"). The +key point is that `flush()` already keeps the `gc()` queue, the `forever()` rows and the file +directories consistent, and a prefix change would bypass all three. + +### D2. Cache the repository first, then check + +```php +if (!isset($this->stores[$name])) { + $this->stores[$name] = $this->resolve($name); + + $this->scope_to_application_version($this->stores[$name]); +} +``` + +`Repository::flush()` fires `CacheFlushed`. If the repository were checked before it is memoised, +a listener that calls `Cache::store()` for the same store would resolve it again and flush again, +or loop. Memoising first makes a re-entrant call return the same instance with no check. +*Alternative rejected:* check, then cache. It matches the brief's wording more literally, but the +re-entrancy above makes it unsafe. + +### D3. The check takes the repository and reads the store name from it + +The method signature is `scope_to_application_version(Repository $repository)`. It reads +`$repository->get_name()` and `$this->store_config($that_name)`. For a file store that fell back, +the name is the fallback's, so the fallback's `version_scoped` and `network` apply. The nested +`store($fallback)` call has already checked that store, so the outer check finds the recorded +version equal to the current one and does nothing. No special case is needed. +*Alternative rejected:* passing `$name` from `store()`. That would check the fallback repository +under the file store's name and config, which the brief rules out. + +### D4. Method shape + +- `protected`, `@since 3.3.1`, `@return void`. +- Steps: + 1. Read `$config = $this->store_config($name)`. + 2. Return early unless `($config['version_scoped'] ?? false) === true`. + 3. Read `$app = app()`. Return early unless `method_exists($app, 'version')`. + 4. Read the recorded version with default `null`, so that a missing option and a stored empty + string stay distinguishable. Compare with `(string) $app->version()` as strings. Any + difference counts, including a downgrade. + 5. If `$repository->flush()` returns `true`, write the version: `update_option($option, $version, + true)` for a site store, or `update_site_option($option, $version)` for a network store. +- The option name is built by a small protected helper, + `application_version_option(string $name)`, which returns + `$this->prefix() . 'cache_app_version_' . $name`. The test reads the name from it, and a + subclass can override it. +- Scope comes from `$config['network'] ?? false`, not from the store object, so custom drivers + behave the same way. + +### D5. Record the version only after a successful flush + +A `false` result leaves the option unchanged, so the next request retries. Exceptions are not +caught. The built-in stores cannot fail here, and a custom driver whose flush throws should +surface the error rather than serve old-shape data silently. *Alternatives rejected:* always +writing the option, which marks a stale cache as current; and catching and logging, which adds +error handling for a case the built-in stores cannot hit. + +### D6. Configuration and docs + +- `default_configuration()` adds `'version_scoped' => false` to `database` and `file` only. + It is pointless on `array`, which lives for one request. +- `example/config/cache.php` gets a `/* */` block comment on the key in both persistent stores, + in the style of `forever_ttl` and `network`. +- `docs/cache.md` §10 gets the key in the snippet and a `### version_scoped` subsection covering: + - when to turn it on; + - the cold-cache cost after each update; + - the rate-limiter reset; + - the shared-namespace caveat. +- `docs/cache.md` §13 gets one line saying Laravel has no equivalent. + +### D7. Tests + +- New file `tests/Unit/Cache/VersionScopedCacheTest.php`, extending `CacheTestCase`. +- Uses `ConfigurableCacheManager::set_stores()` and `$this->app->use_version()`. +- Each "request" is simulated with a fresh `CacheManager` (or `forget_resolved()` plus + `MemoTable::clear()`), so stores re-resolve. +- The `update_option` stub ignores `$autoload` today. To assert the option is autoloaded, the stub + records it in `$GLOBALS['framework_test_option_autoload']`, which `CacheTestCase` resets. +- The failure case uses a custom driver registered through `extend()`, whose `flush()` returns + `false`. +- The re-entrancy case registers a `CacheFlushed` listener that calls `store()` on the same store, + using the event mechanism `CacheEventsTest` uses. +- The fallback case needs no extra setup. The test process has no direct filesystem, so a `file` + store always diverts to its fallback, as + `FileStoreTest::test_the_store_diverts_to_its_fallback_when_direct_access_is_unavailable` relies on. + +## Risks / Trade-offs + +- [Every `database`-driver store shares one namespace, so scoping one store flushes all of them, + including a separately configured limiter store on that driver] → Documented in `docs/cache.md` + and the example config. To keep its counts, the limiter needs a store on another backend. +- [Any flush, this one included, changes the default store's `get_prefix()`, so locks held at that + moment become invisible to new requests] → Known issue, listed in the PR. Not changed here. +- [Concurrent first requests both see the old version and both flush, so the namespace version + rises by two] → Safe: the result is still a cold cache with every superseded version queued. + No lock is added. +- [A custom driver whose flush keeps returning `false` is retried on every request] → This is + intended: a stale-shape cache is worse than a repeated flush attempt. + +## Migration Plan + +No migration. The setting defaults to `false`. A consumer that enables it gets one flush on the +first request after deploying, because the missing option counts as a change. Rollback means +setting it back to `false`. The recorded option stays behind and is harmless. diff --git a/openspec/changes/add-cache-version-scoping/proposal.md b/openspec/changes/add-cache-version-scoping/proposal.md new file mode 100644 index 0000000..ec54a8a --- /dev/null +++ b/openspec/changes/add-cache-version-scoping/proposal.md @@ -0,0 +1,68 @@ +## Why + +A consuming plugin caches values whose shape its own code produces (for example a remote JSON +list after the plugin normalises it). When a plugin update changes that shape, the cache key stays +the same, so the new code reads an entry in the old shape and fails. In a real case, version 1.0.1 +read a list cached by 1.0.0 that had no `slug` field, and the WordPress plugin update check crashed +with a `TypeError`. Every cache entry can be rebuilt from its source, so after a version change the +correct state is a cold cache. + +## What Changes + +- New per-store configuration key `cache.stores..version_scoped` (bool, default `false`). + It is present on the built-in `database` and `file` stores. It has no useful effect on `array`. + Any store, custom drivers included, may set it. +- When a store with `version_scoped => true` is first resolved in a request, the framework + compares the application version with the version recorded for that store. If they differ, or if + no version is recorded yet, it flushes the store through the existing namespace flush and then + records the current version. +- The recorded version is an autoloaded option named `cache_app_version_`. + It is a site option when the store is `network => true`. +- The version is recorded only when the flush reports success, so a failed flush is retried on the + next request. +- Docs: `docs/cache.md` gets the configuration key and a section explaining when to turn it on + and what it costs. That section states plainly that every store on the `database` driver shares + one namespace, so the flush also covers a rate-limiter store on the same driver. The annotated + `example/config/cache.php` gets the key and a matching comment. +- No breaking change: the setting is opt-in, and with it off, behaviour is unchanged. + +### Why a flush rather than a version in the key prefix + +- `DatabaseStore` builds `cache_version`, `cache_stale_versions` and `cache_counters` from the + same prefix. A version in the prefix moves them too, so the previous version's stale-version + queue is lost and `gc()` never removes those rows. +- Rows under an old prefix are in no `gc()` queue. Entries written with `forever()` (TTL + `forever_ttl`, ten years by default) would stay behind after every release. `FileStore` would + leave one orphaned directory per release. +- `flush()` already handles all of this. `DatabaseStore::flush()` increments the namespace version + and queues the old one for `gc()`. `FileStore::flush()` deletes its directory. A persistent + object cache evicts the old namespace by itself. + +### Why opt-in + +This is a general-purpose library. A consumer may keep values in the cache that it cannot rebuild, +so a flush the consumer did not ask for would change behaviour. The rate limiter also counts in +the default store when `cache.limiter` is `null`, and a flush resets its counts to zero. + +## Capabilities + +### New Capabilities + +- `cache-version-scoping`: opt-in per-store flushing of the cache when the application version + changes, including how the recorded version is stored, scoped to the network and retried. + +### Modified Capabilities + + + +## Impact + +- `src/Cache/CacheManager.php`: `store()` runs a new protected check after resolving, and + `default_configuration()` gains `version_scoped => false` on `database` and `file`. +- `tests/Unit/Cache/VersionScopedCacheTest.php`: new test file. +- `docs/cache.md` and `example/config/cache.php`: documentation. +- Runtime cost when enabled: one read of an autoloaded option per store per request, so no extra + query. On a version change there is one flush and one option write. +- Known, unchanged: `CacheManager::lock()` builds its lock prefix from the default store's + `get_prefix()`, which includes the namespace version. Any flush, this one included, therefore + makes locks held at that moment invisible to new requests. This is reported, not fixed here. diff --git a/openspec/changes/add-cache-version-scoping/specs/cache-version-scoping/spec.md b/openspec/changes/add-cache-version-scoping/specs/cache-version-scoping/spec.md new file mode 100644 index 0000000..216341e --- /dev/null +++ b/openspec/changes/add-cache-version-scoping/specs/cache-version-scoping/spec.md @@ -0,0 +1,95 @@ +## Purpose + +Lets a consuming plugin opt a cache store into starting cold whenever the application version +changes, so that entries written by an older release can never be read by code that expects a +different shape. + +## ADDED Requirements + +### Requirement: Version scoping is opt-in per store +Each configured cache store SHALL accept a boolean `version_scoped` setting, which defaults to +`false`. A store whose setting is anything other than `true` SHALL behave exactly as it did before +this capability existed. Nothing SHALL be flushed, and no version SHALL be recorded for it. + +#### Scenario: Disabled store keeps values across a version change +- **WHEN** a store with `version_scoped => false` holds a value written while the application version was A, and the store is resolved again in a later request where the application version is B +- **THEN** the value is still returned + +#### Scenario: Default configuration leaves scoping off +- **WHEN** the built-in `database` or `file` store is resolved with no `version_scoped` setting configured +- **THEN** the setting is `false` and no version is recorded + +### Requirement: A version change empties a scoped store +When a store with `version_scoped => true` is first resolved in a request, the framework SHALL +compare the application version with the version recorded for that store. If they differ, the +framework SHALL flush the store before handing it out and SHALL then record the current +application version. Any difference SHALL count as a change, a downgrade included. + +#### Scenario: Value written under an older version is gone +- **WHEN** a store with `version_scoped => true` holds a value written while the application version was A, and the store is resolved again in a later request where the application version is B +- **THEN** the value is not returned, and B is recorded as the store's version + +#### Scenario: Same version does not flush +- **WHEN** a store with `version_scoped => true` is resolved while the recorded version equals the application version +- **THEN** the store is not flushed and its entries remain readable + +#### Scenario: Application without a version +- **WHEN** a store with `version_scoped => true` is resolved and the application exposes no version +- **THEN** the store is not flushed and no version is recorded + +### Requirement: A missing recorded version counts as a change +If no version has been recorded for a scoped store, the framework SHALL treat that as a version +change, flush the store, and record the current version. A site upgrading from a framework release +without this capability may hold entries in an old shape and MUST start cold. + +#### Scenario: First resolve after upgrading the framework +- **WHEN** a store with `version_scoped => true` holds a value and has no recorded version +- **THEN** the first resolve flushes the store, the value is no longer returned, and the current application version is recorded + +### Requirement: Check runs at most once per store per request +The version check SHALL run only when a store is resolved for the first time in a request, and +SHALL NOT run for stores the request never uses. Resolving the same store again in the same +request, including from inside a listener for the flush event, SHALL return the already-resolved +store without checking or flushing again. + +#### Scenario: Re-entrant resolve during the flush +- **WHEN** a listener for the cache-flushed event resolves the same store while the version check is flushing it +- **THEN** the store is flushed exactly once in that request + +### Requirement: Recorded version storage +The recorded version SHALL be stored in an autoloaded option named +`cache_app_version_`. For a store configured with `network => true` +it SHALL be a network-wide site option instead. + +#### Scenario: Site-scoped store +- **WHEN** a store with `version_scoped => true` and `network => false` records a version +- **THEN** the version is written to the site option `cache_app_version_` and no network option is written + +#### Scenario: Network store +- **WHEN** a store with `version_scoped => true` and `network => true` records a version +- **THEN** the version is read from and written to the network option `cache_app_version_`, and no site option is written + +### Requirement: Failed flush is retried +The version SHALL be recorded only when the flush reports success. When the flush reports failure, +the recorded version SHALL remain unchanged, so the next request tries the flush again. An +exception raised by the flush SHALL reach the caller unchanged. + +#### Scenario: Flush reports failure +- **WHEN** a scoped store's flush reports failure during a version change +- **THEN** the recorded version is not updated + +### Requirement: Fallback stores are checked under their own name +When a store diverts to a fallback store, the version check SHALL use the fallback store's name +and configuration, not those of the store that diverted. + +#### Scenario: File store falls back to the database store +- **WHEN** a `file` store with `version_scoped => true` cannot run and diverts to a `database` store with `version_scoped => false` +- **THEN** the database store is not flushed and no version is recorded under the `file` store's name + +### Requirement: Flushed entries remain reclaimable +A flush triggered by a version change SHALL leave the superseded entries reclaimable by the +store's scheduled garbage collection, exactly as an explicit flush does. + +#### Scenario: Database store queues the old namespace +- **WHEN** a scoped `database` store is flushed because the version changed +- **THEN** the superseded namespace version is in the store's garbage-collection queue, and a garbage-collection run removes its rows diff --git a/openspec/changes/add-cache-version-scoping/tasks.md b/openspec/changes/add-cache-version-scoping/tasks.md new file mode 100644 index 0000000..64b655f --- /dev/null +++ b/openspec/changes/add-cache-version-scoping/tasks.md @@ -0,0 +1,94 @@ +## 1. Test support + +- [x] 1.1 Make the `update_option` stub in `tests/Support/StubsWordPressFunctions.php` record its + `$autoload` argument in `$GLOBALS['framework_test_option_autoload'][$name]`, and reset that + global in `CacheTestCase::reset_cache_globals()`. Verify: `composer test:unit` is still green. + +## 2. Failing tests (write first, run, confirm each fails for the right reason) + +- [x] 2.1 Create `tests/Unit/Cache/VersionScopedCacheTest.php` (extends `CacheTestCase`; uses + `ConfigurableCacheManager::set_stores()` and `$this->app->use_version()`; simulates a new + request with a fresh manager plus `MemoTable::clear()`). +- [x] 2.2 Test: `version_scoped => true`: a value written under version A is not readable after + the version changes to B, and B is recorded. +- [x] 2.3 Test: `version_scoped => false`: the value is still readable after A → B, and no + `cache_app_version_*` option is written. +- [x] 2.4 Test: the option is missing but a value exists (upgrade path). The first resolve + flushes, then writes the option with autoload `true`. +- [x] 2.5 Test: the version is unchanged. No flush: `cache_version` keeps its value and the + entry stays readable. +- [x] 2.6 Test: a network store (`network => true`) reads and writes a site option, and no plain + option is written. +- [x] 2.7 Test: after a version flush of a database store, the superseded namespace version is in + `cache_stale_versions`. +- [x] 2.8 Test: a custom driver (via `extend()`) whose `flush()` returns `false` leaves the + recorded version unchanged. +- [x] 2.9 Test: a `CacheFlushed` listener that calls `store()` on the same store does not cause + a second flush (the namespace version rises by exactly one). +- [x] 2.10 Test: a `file` store with `version_scoped => true` that falls back to a `database` + store with `version_scoped => false` flushes nothing and records no version under either name. +- [x] 2.11 Test: when the application exposes no `version()`, nothing is flushed or recorded. +- [x] 2.12 Run `composer test:unit -- --filter VersionScopedCacheTest`. Verify every new test + fails on missing behaviour (not on a setup error), except 2.3/2.10/2.11, which may already pass. + +## 3. Implementation + +- [x] 3.1 Add `'version_scoped' => false` to the `database` and `file` entries of + `CacheManager::default_configuration()`. +- [x] 3.2 Add protected `application_version_option(string $name)` returning + `$this->prefix() . 'cache_app_version_' . $name` (docblock, `@since 3.3.1`). +- [x] 3.3 Add protected `scope_to_application_version(Repository $repository)` per design D4/D5: + - read the name from `get_name()`; + - read `version_scoped`/`network` from `store_config()`; + - guard on `method_exists(app(), 'version')`; + - compare as strings, with a missing option as `null`; + - flush; + - write the option only when the flush returns `true` (`update_option(..., true)` or + `update_site_option()`). + + It needs a full docblock and `@since 3.3.1`. +- [x] 3.4 In `CacheManager::store()`, memoise `resolve($name)` into `$this->stores[$name]` first, + then call `scope_to_application_version()` on it (design D2). +- [x] 3.5 Verify: `composer test:unit -- --filter 'VersionScopedCacheTest|CacheManagerTest|FileStoreTest|DatabaseGcTest'` + is green. + +## 4. Documentation + +- [x] 4.1 In `docs/cache.md` §10, add `'version_scoped' => false` to the `database` and `file` + snippet entries with a short `//` note. +- [x] 4.2 In `docs/cache.md` §10, add a `### version_scoped` subsection covering: + - what it does (a cold cache when the app version changes, the missing option counting as a + change, once per store per request, no extra query); + - when to turn it on (stores holding values whose shape your code produces); + - the costs (a cold cache after every update; rate-limiter counts reset when the limiter uses + the same store); + - the caveat that every `database`-driver store, and every `file` store on the default path, + shares one namespace, so scoping one flushes all of them, a limiter store on the same driver + included; + - that it has no effect on `array`. +- [x] 4.3 In `docs/cache.md` §13, add one line saying `version_scoped` has no Laravel equivalent. +- [x] 4.4 In `example/config/cache.php`, add `'version_scoped' => false` to the `database` and + `file` stores, with a `/* */` block comment in the style of `forever_ttl` and `network`, saying + the same thing as 4.2. +- [x] 4.5 Verify: the docs, the example config and `default_configuration()` agree on the key, + the default and the stores it appears on. + +## 5. Verification + +- [x] 5.1 `composer phpcs && composer test:unit`: both green, full suite. + +## 6. PR draft + +- [x] 6.1 Write `openspec/changes/add-cache-version-scoping/pr.md` with a title and a body + containing: + - the problem statement (the 1.0.0 → 1.0.1 `slug` `TypeError`); + - the three reasons for a flush rather than a version prefix (stale-version queue lost, + orphaned `forever()` rows and file directories, `flush()` already handles it); + - why it is opt-in; + - the shared-namespace caveat; + - **known issue:** `CacheManager::lock()` derives its prefix from `get_prefix()`, which includes + the namespace version, so any flush, this one included, makes locks held at that moment + invisible to new requests; + - **note:** concurrent first requests may both flush, the namespace version rises by two, this + is safe, and no lock was added. +- [x] 6.2 Do not commit, push or open the PR. Wait for Sajeeb to ask, then use `pr.md`. diff --git a/src/Cache/CacheManager.php b/src/Cache/CacheManager.php index de091dc..b2dadce 100644 --- a/src/Cache/CacheManager.php +++ b/src/Cache/CacheManager.php @@ -78,6 +78,8 @@ public function store($name = null) if (!isset($this->stores[$name])) { $this->stores[$name] = $this->resolve($name); + + $this->scope_to_application_version($this->stores[$name]); } return $this->stores[$name]; @@ -227,19 +229,21 @@ protected function default_configuration() return [ 'database' => [ - 'driver' => 'database', - 'forever_ttl' => 10 * $year, - 'network' => false, - 'gc' => 'daily', - 'events' => true, + 'driver' => 'database', + 'forever_ttl' => 10 * $year, + 'network' => false, + 'gc' => 'daily', + 'events' => true, + 'version_scoped' => false, ], 'file' => [ - 'driver' => 'file', - 'path' => null, - 'fallback' => 'database', - 'gc' => 'daily', - 'network' => false, - 'events' => true, + 'driver' => 'file', + 'path' => null, + 'fallback' => 'database', + 'gc' => 'daily', + 'network' => false, + 'events' => true, + 'version_scoped' => false, ], 'array' => [ 'driver' => 'array', @@ -287,6 +291,78 @@ protected function resolve(string $name) ); } + /** + * Start a version scoped store over when the application version has changed. + * + * Such a store holds values whose shape the application's own code produces, so an entry + * written by an earlier release may not be readable by this one. A flush rather than a + * version in the key prefix is used because flush() already queues the old namespace for + * gc() and removes the file store's directory, where a moved prefix would orphan both. A + * missing recorded version counts as a change, since a site upgrading from a framework + * without this setting may hold entries in an old shape. The version is recorded only after + * a successful flush, so a failed one is retried on the next request. + * + * The store name is taken from the repository rather than from the caller, because a file + * store that diverts to its fallback hands back the fallback's repository, and the fallback's + * own configuration then governs. + * + * @param Repository $repository The repository that was just resolved. + * + * @return void + * + * @since 3.3.1 + */ + protected function scope_to_application_version(Repository $repository) + { + $name = $repository->get_name(); + $config = $this->store_config($name); + + if (($config['version_scoped'] ?? false) !== true) { + return; + } + + $app = app(); + + if (!method_exists($app, 'version')) { + return; + } + + $version = (string) $app->version(); + $option = $this->application_version_option($name); + $network = (bool) ($config['network'] ?? false); + $recorded = $network ? get_site_option($option, null) : get_option($option, null); + + if (!is_null($recorded) && (string) $recorded === $version) { + return; + } + + if (!$repository->flush()) { + return; + } + + if ($network) { + update_site_option($option, $version); + + return; + } + + update_option($option, $version, true); + } + + /** + * Get the option name holding the application version a store was last flushed for. + * + * @param string $name The store name. + * + * @return string + * + * @since 3.3.1 + */ + protected function application_version_option(string $name) + { + return $this->prefix() . 'cache_app_version_' . $name; + } + /** * Build a transient backed store. * diff --git a/tests/Support/StubsWordPressFunctions.php b/tests/Support/StubsWordPressFunctions.php index 3751f90..f4eca37 100644 --- a/tests/Support/StubsWordPressFunctions.php +++ b/tests/Support/StubsWordPressFunctions.php @@ -742,6 +742,7 @@ function update_option($name, $value, $autoload = null) } $GLOBALS['framework_test_options'][$name] = $value; + $GLOBALS['framework_test_option_autoload'][$name] = $autoload; return true; } diff --git a/tests/Unit/Cache/CacheTestCase.php b/tests/Unit/Cache/CacheTestCase.php index 64685e9..025c1c3 100644 --- a/tests/Unit/Cache/CacheTestCase.php +++ b/tests/Unit/Cache/CacheTestCase.php @@ -32,6 +32,7 @@ protected function reset_cache_globals(): void $GLOBALS['framework_test_transients'] = []; $GLOBALS['framework_test_site_transients'] = []; $GLOBALS['framework_test_options'] = []; + $GLOBALS['framework_test_option_autoload'] = []; $GLOBALS['framework_test_site_options'] = []; $GLOBALS['framework_test_ext_object_cache'] = false; $GLOBALS['framework_test_object_cache'] = []; diff --git a/tests/Unit/Cache/VersionScopedCacheTest.php b/tests/Unit/Cache/VersionScopedCacheTest.php new file mode 100644 index 0000000..02031b0 --- /dev/null +++ b/tests/Unit/Cache/VersionScopedCacheTest.php @@ -0,0 +1,273 @@ +app->use_prefix('fw_'); + } + + protected function tearDown(): void + { + $this->reset_test_wpdb(); + + parent::tearDown(); + } + + protected function database_store(array $overrides = []): array + { + return array_merge([ + 'driver' => 'database', + 'forever_ttl' => 315360000, + 'network' => false, + 'events' => false, + 'version_scoped' => true, + ], $overrides); + } + + protected function request(array $stores): ConfigurableCacheManager + { + MemoTable::clear(); + + return (new ConfigurableCacheManager())->set_stores($stores); + } + + protected function options(): array + { + return $GLOBALS['framework_test_options']; + } + + protected function site_options(): array + { + return $GLOBALS['framework_test_site_options']; + } + + public function test_a_value_written_under_an_older_version_is_not_readable_after_an_update() + { + $stores = ['database' => $this->database_store()]; + + $this->app->use_version('1.0.0'); + $this->request($stores)->store('database')->put('plugins', [['name' => 'Old shape']], 600); + + $this->app->use_version('1.0.1'); + $cache = $this->request($stores)->store('database'); + + $this->assertNull($cache->get('plugins')); + $this->assertSame('1.0.1', $this->options()['fw_cache_app_version_database']); + } + + public function test_a_store_that_is_not_scoped_keeps_its_values_across_an_update() + { + $stores = ['database' => $this->database_store(['version_scoped' => false])]; + + $this->app->use_version('1.0.0'); + $this->request($stores)->store('database')->put('plugins', ['kept'], 600); + + $this->app->use_version('1.0.1'); + $cache = $this->request($stores)->store('database'); + + $this->assertSame(['kept'], $cache->get('plugins')); + $this->assertArrayNotHasKey('fw_cache_app_version_database', $this->options()); + } + + public function test_the_built_in_persistent_stores_are_not_scoped_by_default() + { + $manager = new CacheManager(); + + $this->assertFalse($manager->store_config('database')['version_scoped']); + $this->assertFalse($manager->store_config('file')['version_scoped']); + $this->assertArrayNotHasKey('version_scoped', $manager->store_config('array')); + } + + public function test_a_missing_recorded_version_flushes_once_and_records_the_version() + { + $this->app->use_version('1.0.1'); + + // Written by a framework release that predates version scoping, so no version is recorded. + $this->request(['database' => $this->database_store(['version_scoped' => false])]) + ->store('database') + ->put('plugins', ['old shape'], 600); + + $cache = $this->request(['database' => $this->database_store()])->store('database'); + + $this->assertNull($cache->get('plugins')); + $this->assertSame(2, $this->options()['fw_cache_version']); + $this->assertSame('1.0.1', $this->options()['fw_cache_app_version_database']); + $this->assertTrue($GLOBALS['framework_test_option_autoload']['fw_cache_app_version_database']); + } + + public function test_an_unchanged_version_does_not_flush() + { + $stores = ['database' => $this->database_store()]; + + $this->app->use_version('1.0.0'); + $this->request($stores)->store('database')->put('plugins', ['current'], 600); + + $version = $this->options()['fw_cache_version']; + + $cache = $this->request($stores)->store('database'); + + $this->assertSame(['current'], $cache->get('plugins')); + $this->assertSame($version, $this->options()['fw_cache_version']); + } + + public function test_a_downgrade_also_flushes() + { + $stores = ['database' => $this->database_store()]; + + $this->app->use_version('1.0.1'); + $this->request($stores)->store('database')->put('plugins', ['newer shape'], 600); + + $this->app->use_version('1.0.0'); + $cache = $this->request($stores)->store('database'); + + $this->assertNull($cache->get('plugins')); + $this->assertSame('1.0.0', $this->options()['fw_cache_app_version_database']); + } + + public function test_a_network_store_records_its_version_as_a_site_option() + { + $stores = ['database' => $this->database_store(['network' => true])]; + + $this->app->use_version('1.0.0'); + $this->request($stores)->store('database')->put('license', 'old shape', 600); + + $this->assertSame('1.0.0', $this->site_options()['fw_cache_app_version_database']); + + $this->app->use_version('1.0.1'); + $cache = $this->request($stores)->store('database'); + + $this->assertNull($cache->get('license')); + $this->assertSame('1.0.1', $this->site_options()['fw_cache_app_version_database']); + $this->assertArrayNotHasKey('fw_cache_app_version_database', $this->options()); + } + + public function test_the_flushed_namespace_is_queued_for_and_reclaimed_by_gc() + { + $this->app->use_version('1.0.0'); + + $manager = $this->request(['database' => $this->database_store()]); + $manager->store('database'); + + $this->assertSame([1], $this->options()['fw_cache_stale_versions']); + + global $wpdb; + + $wpdb = new FakeWpdb(); + $wpdb->option_rows = [ + '_transient_fw_c1_aaaa', + '_transient_timeout_fw_c1_aaaa', + '_transient_fw_c2_bbbb', + ]; + + $this->assertSame(2, $manager->collect_garbage('database')); + $this->assertSame(['_transient_fw_c2_bbbb'], array_values($wpdb->option_rows)); + } + + public function test_a_failed_flush_leaves_the_recorded_version_unchanged() + { + $this->app->use_version('1.0.1'); + update_option('fw_cache_app_version_unflushable', '1.0.0'); + + $manager = $this->request(['unflushable' => ['driver' => 'unflushable', 'events' => false, 'version_scoped' => true]]); + $manager->extend('unflushable', function () { + return new class extends ArrayStore { + public function flush() + { + return false; + } + }; + }); + + $manager->store('unflushable'); + + $this->assertSame('1.0.0', $this->options()['fw_cache_app_version_unflushable']); + } + + public function test_resolving_the_store_from_a_flush_listener_does_not_flush_again() + { + $this->app->use_version('1.0.0'); + + $manager = $this->request(['database' => $this->database_store(['events' => true])]); + + $events = new class extends RecordingEventManager { + public $on_dispatch; + + public function dispatch($event): array + { + parent::dispatch($event); + + ($this->on_dispatch)(); + + return []; + } + }; + $events->listen_for(CacheFlushed::class); + $events->on_dispatch = function () use ($manager) { + $manager->store('database'); + }; + + $this->app->instance('event', $events); + + $manager->store('database'); + + $this->assertSame(2, $this->options()['fw_cache_version']); + $this->assertCount(1, $events->dispatched); + } + + public function test_a_file_store_that_falls_back_is_checked_under_the_fallback_name() + { + // No direct filesystem in the test process, so the file store diverts to its fallback. + $stores = [ + 'file' => ['driver' => 'file', 'fallback' => 'database', 'events' => false, 'version_scoped' => true], + 'database' => $this->database_store(['version_scoped' => false]), + ]; + + $this->app->use_version('1.0.0'); + $this->request($stores)->store('file')->put('plugins', ['kept'], 600); + + $this->app->use_version('1.0.1'); + $cache = $this->request($stores)->store('file'); + + $this->assertSame(['kept'], $cache->get('plugins')); + $this->assertArrayNotHasKey('fw_cache_app_version_file', $this->options()); + $this->assertArrayNotHasKey('fw_cache_app_version_database', $this->options()); + } + + public function test_an_application_without_a_version_is_never_flushed() + { + $this->set_container_instance(new class extends Container { + public function config_path($path = '') + { + return '/nonexistent/config/' . $path; + } + }); + + $stores = ['database' => $this->database_store()]; + + $this->request($stores)->store('database')->put('plugins', ['kept'], 600); + + $cache = $this->request($stores)->store('database'); + + $this->assertSame(['kept'], $cache->get('plugins')); + $this->assertArrayNotHasKey('cache_app_version_database', $this->options()); + } +}