diff --git a/README.md b/README.md index ad121af..8744408 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,8 @@ Memcached daemon to run. // Anywhere in WordPress, once wp-content/object-cache.php is in place: wp_cache_set('user:42', ['name' => 'Alice'], 'users', 3600); wp_cache_get('user:42', 'users'); // ['name' => 'Alice'] -wp_cache_incr('hits:home'); // 1 +wp_cache_set('hits:home', 0); // seed the counter first +wp_cache_incr('hits:home'); // 1 (incr on a missing key returns false, per WP core) wp_cache_flush(); // really clears the whole store ``` @@ -49,7 +50,7 @@ rather than being a documented no-op. This package does **not** depend on any WordPress core Composer package — it implements the `WP_Object_Cache` surface WordPress calls, and runs its tests on plain `php-cli`. -- **The ePHPm runtime, v0.1.2 or newer** (current release: v0.8.6). The +- **The ePHPm runtime, v0.1.2 or newer** (current release: v0.10.2). The `ephpm_kv_*` SAPI functions have shipped since ePHPm v0.1.0, but `ephpm_kv_flush_all()` — which backs `wp_cache_flush()` — arrived in v0.1.2. The functions are registered @@ -133,10 +134,10 @@ delegating to the `ObjectCache` engine: | Function(s) | Backed by ePHPm KV | |--------------------------------------------------------|--------------------| -| `wp_cache_get`, `wp_cache_set`, `wp_cache_add`, `wp_cache_replace` | yes | +| `wp_cache_get`, `wp_cache_set`, `wp_cache_add`, `wp_cache_replace` | yes (`add` is atomic via `ephpm_kv_setnx`) | | `wp_cache_delete` | yes | | `wp_cache_get_multiple`, `wp_cache_set_multiple`, `wp_cache_add_multiple`, `wp_cache_delete_multiple` | yes | -| `wp_cache_incr`, `wp_cache_decr` | yes (atomic via `ephpm_kv_incr_by`) | +| `wp_cache_incr`, `wp_cache_decr` | yes (atomic via `ephpm_kv_incr_by`; miss returns `false`, per WP core) | | `wp_cache_flush` | **yes — clears the whole store** | | `wp_cache_flush_runtime` | yes (runtime array only) | | `wp_cache_flush_group` | partial (see below) | @@ -147,8 +148,11 @@ delegating to the `ObjectCache` engine: | `wp_cache_supports($feature)` | yes | `wp_cache_supports()` returns `true` for `add_multiple`, `set_multiple`, -`get_multiple`, `delete_multiple`, `flush_runtime`, and `flush_group`, so core -and well-behaved plugins take their batched/fast paths. +`get_multiple`, `delete_multiple`, and `flush_runtime`, so core and well-behaved +plugins take their batched/fast paths. It returns `false` for `flush_group`: +the persistent tier cannot be selectively flushed (the KV SAPI has no +key-enumeration primitive), so advertising it would over-promise — see the +[`wp_cache_flush_group()` limitation](#wp_cache_flush_group-limitation) below. Like core's own `WP_Object_Cache`, every value set during a request is served back from an in-request runtime array for the rest of that request; the KV store @@ -319,8 +323,8 @@ ePHPm runs PHP inside the same OS process as the KV store via the embed SAPI. Th store itself is a Rust [`DashMap`](https://docs.rs/dashmap/) plus TTL management. ePHPm registers a small set of host functions (`ephpm_kv_get`, `ephpm_kv_set`, `ephpm_kv_incr_by`, `ephpm_kv_expire`, `ephpm_kv_ttl`, `ephpm_kv_pttl`, -`ephpm_kv_del`, `ephpm_kv_exists`, `ephpm_kv_flush_all`) into PHP's global -function table. Calling one is a direct C function call into Rust — no socket, no +`ephpm_kv_del`, `ephpm_kv_exists`, `ephpm_kv_setnx`, `ephpm_kv_flush_all`) into +PHP's global function table. Calling one is a direct C function call into Rust — no socket, no protocol parser, no value serialization beyond what userland code already does. This package wraps those functions in a `WP_Object_Cache`-shaped engine diff --git a/dropin/object-cache.php b/dropin/object-cache.php index a1c15eb..065fe5e 100644 --- a/dropin/object-cache.php +++ b/dropin/object-cache.php @@ -300,13 +300,19 @@ function wp_cache_reset(): bool if (!function_exists('wp_cache_supports')) { function wp_cache_supports(string $feature): bool { + // NOTE: 'flush_group' is deliberately NOT advertised. The KV SAPI has + // no key-enumeration or prefix-scan primitive, so a *persistent* group + // cannot actually be flushed selectively (see ObjectCache::flush_group, + // which returns false for persistent groups). Advertising it would + // promise a capability we can only honour for runtime-only groups, so + // callers that gate on wp_cache_supports('flush_group') would wrongly + // believe the persistent tier was cleared. return match ($feature) { 'add_multiple', 'set_multiple', 'get_multiple', 'delete_multiple', - 'flush_runtime', - 'flush_group' => true, + 'flush_runtime' => true, default => false, }; } diff --git a/src/InMemoryKvOps.php b/src/InMemoryKvOps.php index 53e97cc..f2f84d4 100644 --- a/src/InMemoryKvOps.php +++ b/src/InMemoryKvOps.php @@ -37,6 +37,18 @@ public function set(string $key, string $value, int $ttlSeconds = 0): bool return true; } + public function setnx(string $key, string $value, int $ttlSeconds = 0): bool + { + // Insert only when no live entry exists. A key whose deadline has + // already passed is treated as absent (lazy expiry via liveValue), + // so setnx over an expired key succeeds — matching the SAPI. + if ($this->liveValue($key) !== null) { + return false; + } + $this->set($key, $value, $ttlSeconds); + return true; + } + public function del(string $key): int { if ($this->liveValue($key) === null) { diff --git a/src/KvOpsInterface.php b/src/KvOpsInterface.php index 7ea4938..548c3ad 100644 --- a/src/KvOpsInterface.php +++ b/src/KvOpsInterface.php @@ -33,6 +33,22 @@ public function get(string $key): ?string; */ public function set(string $key, string $value, int $ttlSeconds = 0): bool; + /** + * Atomically insert a key only if no live entry already exists — the + * atomic add primitive. + * + * Backed by the SAPI's `ephpm_kv_setnx()`, which performs the + * insert-if-absent under a per-shard lock, so two concurrent callers can + * never both succeed. WordPress relies on `wp_cache_add()` being atomic + * (it is used as a cron/lock mutex), which a check-then-set cannot be. + * + * @param int $ttlSeconds 0 means no expiry; positive values are seconds + * + * @return bool true if this call inserted the key; false if a live entry + * already existed (or the store rejected the write, e.g. OOM) + */ + public function setnx(string $key, string $value, int $ttlSeconds = 0): bool; + /** * Delete a key. * diff --git a/src/ObjectCache.php b/src/ObjectCache.php index 2054a97..132ec4c 100644 --- a/src/ObjectCache.php +++ b/src/ObjectCache.php @@ -146,10 +146,40 @@ public function add($key, mixed $data, string $group = 'default', int $ttl = 0): if (\function_exists('wp_suspend_cache_addition') && \wp_suspend_cache_addition()) { return false; } - if ($this->existsInRuntime($key, $group) || $this->existsInStore($key, $group)) { + + // A value already materialised in this request's runtime array always + // loses the race locally — this mirrors WP core's own `_exists()` + // short-circuit and avoids a needless store round-trip. + if ($this->existsInRuntime($key, $group)) { return false; } - return $this->set($key, $data, $group, $ttl); + + // Non-persistent groups live only in the runtime array; there is no + // shared store to race against, so a plain check-then-set is correct + // and keeps the "never touches the backend" guarantee. + if ($this->isNonPersistent($group)) { + return $this->set($key, $data, $group, $ttl); + } + + // Persistent path: `setnx` is the atomic add primitive — a single + // insert-if-absent under the KV store's per-shard lock. Two concurrent + // add()s therefore cannot both win, which WordPress depends on + // (`wp_cache_add()` is used as a cron/lock mutex). The old + // check-then-set (`exists()` then `set()`) was a race: both callers + // could observe "absent" and both write. + if (\is_object($data)) { + $data = clone $data; + } + $inserted = $this->ops->setnx( + $this->build_key($key, $group), + $this->serialize($data), + \max(0, $ttl), + ); + if (!$inserted) { + return false; + } + $this->cache[$group][(string) $key] = $data; + return true; } /** @@ -243,17 +273,27 @@ public function get($key, string $group = 'default', bool $force = false, ?bool } $skey = (string) $key; + // Non-persistent groups live ONLY in the runtime array — there is no + // persistent tier behind them. `$force` bypasses the local cache only + // to re-read the shared store, so for a runtime-only group a forced + // read must still serve the runtime value (never fall through to the + // store, which would spuriously miss). + if ($this->isNonPersistent($group)) { + if (isset($this->cache[$group]) && \array_key_exists($skey, $this->cache[$group])) { + $found = true; + $value = $this->cache[$group][$skey]; + return \is_object($value) ? clone $value : $value; + } + $found = false; + return false; + } + if (!$force && isset($this->cache[$group]) && \array_key_exists($skey, $this->cache[$group])) { $found = true; $value = $this->cache[$group][$skey]; return \is_object($value) ? clone $value : $value; } - if ($this->isNonPersistent($group)) { - $found = false; - return false; - } - $raw = $this->ops->get($this->build_key($key, $group)); if ($raw === null) { $found = false; @@ -333,6 +373,14 @@ public function incr($key, int $offset = 1, string $group = 'default'): int|fals return $this->incrRuntime($key, $offset, $group); } + // WordPress core semantics: incr/decr on a MISSING key returns false — + // it does not create the key. The raw KV SAPI's incr_by would instead + // create it at the delta (INCR on a missing key => the delta), so gate + // on existence in the store first and only then increment. + if (!$this->existsInStore($key, $group)) { + return false; + } + try { $value = $this->ops->incrBy($this->build_key($key, $group), $offset); } catch (\RuntimeException) { @@ -455,7 +503,13 @@ private function existsInStore($key, string $group): bool private function incrRuntime($key, int $offset, string $group): int|false { $skey = (string) $key; - $current = $this->cache[$group][$skey] ?? 0; + // Match WP core: incr/decr on a missing key returns false rather than + // creating it at the delta. (This is the runtime-only counterpart of + // the existsInStore() gate on the persistent path.) + if (!$this->existsInRuntime($key, $group)) { + return false; + } + $current = $this->cache[$group][$skey]; if (!\is_numeric($current)) { return false; } diff --git a/src/SapiKvOps.php b/src/SapiKvOps.php index 023d295..141c514 100644 --- a/src/SapiKvOps.php +++ b/src/SapiKvOps.php @@ -34,6 +34,11 @@ public function set(string $key, string $value, int $ttlSeconds = 0): bool return (bool) \ephpm_kv_set($key, $value, $ttlSeconds); } + public function setnx(string $key, string $value, int $ttlSeconds = 0): bool + { + return (bool) \ephpm_kv_setnx($key, $value, $ttlSeconds); + } + public function del(string $key): int { return (int) \ephpm_kv_del($key); @@ -46,7 +51,15 @@ public function exists(string $key): bool public function incrBy(string $key, int $delta): int { - return (int) \ephpm_kv_incr_by($key, $delta); + // ephpm_kv_incr_by returns `false` when the stored value is not an + // integer. A bare `(int) false === 0` would silently mask that as a + // legitimate zero result, so capture and distinguish it: the + // interface documents this method as throwing on a non-integer value. + $result = \ephpm_kv_incr_by($key, $delta); + if ($result === false) { + throw new \RuntimeException("value at key '{$key}' is not an integer"); + } + return (int) $result; } public function expire(string $key, int $ttlSeconds): bool diff --git a/tests/DropinSupportsTest.php b/tests/DropinSupportsTest.php new file mode 100644 index 0000000..06df9e4 --- /dev/null +++ b/tests/DropinSupportsTest.php @@ -0,0 +1,46 @@ +exists('k')); } + public function test_setnx_inserts_only_when_absent(): void + { + $ops = new InMemoryKvOps(); + self::assertTrue($ops->setnx('lock', 'a')); + self::assertFalse($ops->setnx('lock', 'b')); + // The first writer's value is preserved. + self::assertSame('a', $ops->get('lock')); + } + + public function test_setnx_applies_ttl_on_insert(): void + { + $ops = new InMemoryKvOps(); + self::assertTrue($ops->setnx('k', 'v', 30)); + $pttl = $ops->pttl('k'); + self::assertGreaterThan(0, $pttl); + self::assertLessThanOrEqual(30_000, $pttl); + } + + public function test_setnx_succeeds_over_an_expired_key(): void + { + $ops = new InMemoryKvOps(); + $ops->set('k', 'old', 60); + // Force the deadline into the past so the key is lazily expired. + $ops->expire('k', 1); + \usleep(1_100_000); + self::assertTrue($ops->setnx('k', 'new')); + self::assertSame('new', $ops->get('k')); + } + public function test_incr_creates_key_then_accumulates(): void { $ops = new InMemoryKvOps(); diff --git a/tests/ObjectCacheTest.php b/tests/ObjectCacheTest.php index 3cf9b2e..4e78ea5 100644 --- a/tests/ObjectCacheTest.php +++ b/tests/ObjectCacheTest.php @@ -63,6 +63,36 @@ public function test_add_only_when_absent(): void self::assertSame('first', $cache->get('k')); } + public function test_add_is_atomic_via_setnx(): void + { + // Two independent ObjectCache instances over ONE shared backend model + // two concurrent requests/nodes racing wp_cache_add() (which core uses + // as a lock/cron mutex). Exactly one add() may win, and the losing + // add() must not clobber the winner's value. A check-then-set add() + // could let both observe "absent" and both write; setnx cannot. + $shared = new InMemoryKvOps(); + $a = new ObjectCache($shared); + $b = new ObjectCache($shared); + + self::assertTrue($a->add('lock', 'owner-a', 'locks')); + self::assertFalse($b->add('lock', 'owner-b', 'locks')); + + // The store still holds the winner's value, unchanged. + self::assertSame('owner-a', $b->get('lock', 'locks', true)); + self::assertSame('owner-a', $a->get('lock', 'locks', true)); + } + + public function test_add_persistent_path_uses_setnx_not_exists_then_set(): void + { + // Proves the atomic primitive is actually exercised on the persistent + // path: a single setnx, with no separate exists()+set() race window. + $spy = new SpyKvOps(); + $cache = new ObjectCache($spy); + + self::assertTrue($cache->add('k', 'v', 'options')); + self::assertSame(['setnx'], $spy->calls); + } + public function test_replace_only_when_present(): void { $cache = new ObjectCache(new InMemoryKvOps()); @@ -86,6 +116,8 @@ public function test_delete_removes_from_runtime_and_store(): void public function test_incr_and_decr(): void { $cache = new ObjectCache(new InMemoryKvOps()); + // WP core does not create a key on incr(); seed it first. + $cache->set('hits', 0); self::assertSame(1, $cache->incr('hits')); self::assertSame(6, $cache->incr('hits', 5)); self::assertSame(4, $cache->decr('hits', 2)); @@ -99,6 +131,53 @@ public function test_incr_returns_false_on_non_numeric(): void self::assertFalse($cache->incr('label')); } + public function test_incr_on_missing_persistent_key_returns_false(): void + { + // WP core returns false for incr/decr on a missing key (it does NOT + // create-at-delta the way the raw KV SAPI's incr_by would). Nothing + // must be written to the store as a side effect. + $ops = new InMemoryKvOps(); + $cache = new ObjectCache($ops); + + self::assertFalse($cache->incr('never-seen')); + self::assertFalse($cache->decr('never-seen')); + self::assertNull($ops->get($cache->build_key('never-seen'))); + self::assertFalse($cache->get('never-seen', 'default', true)); + } + + public function test_incr_on_missing_non_persistent_key_returns_false(): void + { + $cache = new ObjectCache(new InMemoryKvOps()); + $cache->add_non_persistent_groups('counts'); + self::assertFalse($cache->incr('misses', 1, 'counts')); + } + + public function test_force_read_of_non_persistent_group_stays_local(): void + { + // force=true bypasses the local cache only for PERSISTENT groups (to + // re-read the shared store). A runtime-only group has no store behind + // it, so a forced read must still return the runtime value and never + // reach the backend. + $spy = new SpyKvOps(); + $cache = new ObjectCache($spy); + $cache->add_non_persistent_groups('counts'); + $cache->set('comments', 5, 'counts'); + + $found = null; + self::assertSame(5, $cache->get('comments', 'counts', true, $found)); + self::assertTrue($found); + + $found = null; + self::assertFalse($cache->get('absent', 'counts', true, $found)); + self::assertFalse($found); + + self::assertSame( + [], + $spy->calls, + 'a forced read of a non-persistent group must not touch the backend', + ); + } + public function test_get_multiple_and_set_multiple(): void { $cache = new ObjectCache(new InMemoryKvOps()); @@ -277,6 +356,12 @@ public function set(string $key, string $value, int $ttlSeconds = 0): bool return $this->inner->set($key, $value, $ttlSeconds); } + public function setnx(string $key, string $value, int $ttlSeconds = 0): bool + { + $this->calls[] = 'setnx'; + return $this->inner->setnx($key, $value, $ttlSeconds); + } + public function del(string $key): int { $this->calls[] = 'del'; diff --git a/tests/SapiKvOpsTest.php b/tests/SapiKvOpsTest.php new file mode 100644 index 0000000..5382f4a --- /dev/null +++ b/tests/SapiKvOpsTest.php @@ -0,0 +1,92 @@ +expectException(\RuntimeException::class); + $ops->incrBy('label', 1); + } + + public function test_incr_by_returns_int_on_success(): void + { + $GLOBALS['__kv_incr_result'] = 7; + $ops = new SapiKvOps(); + self::assertSame(7, $ops->incrBy('hits', 1)); + } + + public function test_incr_by_does_not_mask_a_real_zero(): void + { + // A genuine 0 result must pass through as int(0), not be confused + // with the false sentinel. + $GLOBALS['__kv_incr_result'] = 0; + $ops = new SapiKvOps(); + self::assertSame(0, $ops->incrBy('counter', -1)); + } + + public function test_setnx_passes_through_to_sapi(): void + { + unset($GLOBALS['__kv_store']); + $ops = new SapiKvOps(); + self::assertTrue($ops->setnx('lock', 'a')); + self::assertFalse($ops->setnx('lock', 'b')); + self::assertSame('a', $ops->get('lock')); + } + } +}