fix(cache): gate the query cache write on a lease to reject stale lists

[abnegate]

The race

utopia-php/cache 3.1.0 added the Leasable primitive (getGeneration() / saveWithLease(), PR #75/#77) to close the cache-aside read-after-write race: a reader that fetched a row from the database just before a concurrent purge must not re-populate the cache with the now-stale value after the purge has run. Database::getDocument() adopted it (PR #904 feat/leasable-getdocument): it captures the generation before the database read and uses saveWithLease(...) on save, so a purge that lands in between rejects the stale write.

The query/list cache added in PR #894 (feat/cached-find) was left unprotected. withCache() reads through the cache, runs the callback (the database read), then saves unconditionally. If a concurrent create/update/delete calls purgeCachedQueries() between the callback's read and the save, the older request writes the stale list back into the cache after the purge — re-poisoning exactly the race the lease was meant to kill, this time for lists instead of single documents.

The fix

In withCache() (src/Database/Database.php):

• Capture getGeneration($key) before the callback runs (wrapped in try/catch, degrading to '0' on cache failure) — placed after the optional rejected-value purge so the captured token reflects the generation as of the start of the database read.
• Replace the unconditional cache->save($key, $encoded, $hash) with cache->saveWithLease($key, $encoded, $hash, $generation) — a purge between read and save advances the generation and the now-stale list write is rejected.

This mirrors the getDocument() reference pattern exactly. Non-Leasable adapters (e.g. the Memory cache adapter) fall back to an unconditional save inside Cache, so behaviour is unchanged for them.

Writers fixed

Writer	File:line	Before	After
withCache() query/list cache	src/Database/Database.php:8808	cache->save(...) (unconditional)	cache->saveWithLease(..., $generation) (lease-gated)

getDocument() (src/Database/Database.php:4946) was already lease-gated by PR #904 and is unchanged. The storage file-content cache in downstream consumers (Appwrite app/controllers/shared/api.php) is a Filesystem-backed HTTP response cache with its own invalidation log — not a database list cache — and is out of scope.

Tests

New tests/unit/WithCacheLeaseTest.php with a self-contained Leasable in-memory cache adapter (the shipped Memory adapter is intentionally non-Leasable, so it could not exercise the lease):

• testStaleListWriteAfterConcurrentPurgeIsRejected — the callback purges the query key mid-read; asserts the stale list is not re-cached. Fails without the fix (the old list lands in the cache after the purge), passes with it.
• testListWriteLandsWhenNoConcurrentPurge — with no concurrent purge, the list is cached as normal.

Verification

• Pint (full repo): passed
• PHPStan level 7 (src + tests): no errors
• Unit suite: 397 tests, 2243 assertions, all passing (includes the 2 new regression tests)
• Confirmed the new regression test fails against the pre-fix save() and passes with saveWithLease()

Dependency / propagation

The cache constraint is already utopia-php/cache: 3.1.* and composer.lock already resolves 3.1.0, which provides getGeneration/saveWithLease — no constraint bump needed. This fix lives in utopia-php/database, so it needs a new database release and a downstream composer update (Appwrite/Cloud/Edge) to take effect for list endpoints.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Tests
  • Added coverage for cache lease behavior, including handling of stale data when a cache entry is purged during an in-flight write.
  • Added a second test to verify normal cache population and retrieval still works as expected.
  • Introduced an in-memory cache implementation for testing lease and expiration behavior.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 56eb7833-1692-49aa-a84b-bbba0ecb0ef3

📥 Commits

Reviewing files that changed from the base of the PR and between c71349d and e787492.

📒 Files selected for processing (2)

• src/Database/Database.php
• tests/unit/WithCacheLeaseTest.php

---

📝 Walkthrough

Walkthrough

Adds tests/unit/WithCacheLeaseTest.php containing a LeasableMemoryCache in-memory adapter implementing Utopia\Cache\Adapter and Utopia\Cache\Feature\Leasable, and a WithCacheLeaseTest PHPUnit suite with two tests covering cache lease and purge behavior.

Changes

Cache Lease Unit Tests

Layer / File(s)	Summary
LeasableMemoryCache adapter tests/unit/WithCacheLeaseTest.php	Defines LeasableMemoryCache with a per-key generation marker, TTL-based load, save, saveWithLease (rejects writes on generation mismatch), purge (increments generation), touch, list, and operational helpers (flush, ping, getSize, getName).
Test class and lease/purge test cases tests/unit/WithCacheLeaseTest.php	WithCacheLeaseTest::setUp() seeds an isolated utopiaTests namespace with a projects collection and document; testStaleListWriteAfterConcurrentPurgeIsRejected() simulates a mid-flight purge and asserts load() returns false; testListWriteLandsWhenNoConcurrentPurge() asserts the cache is populated when no purge occurs.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• utopia-php/database#904: Implements saveWithLease() and generation-based stale-write rejection in Database::getDocument(); the new WithCacheLeaseTest directly tests the same lease/purge contract introduced there.

Poem

🐇 A cache with a lease, a generation key,
Purge it mid-flight? The stale write won't stay!
The rabbit checks hashes with hops swift and sure,
If the gen doesn't match, that write hits the floor.
Tests green, cache clean — what a wonderful day! 🌿

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly describes the lease-based cache write change that prevents stale query lists from being re-cached.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/leasable-withcache

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[greptile-apps]

Greptile Summary

This PR adds lease-gated writes for the query/list cache. The main changes are:

• Captures the cache generation before running the cache-miss callback.
• Saves refreshed query cache entries with saveWithLease() instead of unconditional save().
• Adds regression tests for stale list write rejection and normal cache population.

Confidence Score: 5/5

The change is focused and merge-safe, with no review comments identified.

The implementation matches the intended cache lease pattern and includes targeted regression coverage for stale query list writes and normal cache population.

T-Rex Logs

What T-Rex did

• Observed the pre-purge list-cache lease state, where the stale query list was saved after adapter.purge generation 1 and the cache flag was true.
• Observed the lease-path outcome after head called adapter.saveWithLease with expected=0 and current=1, which was rejected and left the cache flag false.
• Observed the normal no-purge path where head called adapter.saveWithLease with expected=0 and current=0 and was accepted, followed by an adapter.save, ending with cached=true.

_{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "fix(cache): gate the query cache write o..." | Re-trigger Greptile}