feat(database): use cache lease in getDocument to fix read-after-write staleness

[abnegate]

Problem

getDocument() re-populates the cache after a miss. Under concurrency, a reader that fetched a row from the database just before a concurrent updateDocument() purge can write that now-stale row back into the cache after the purge runs — so subsequent reads serve stale data until the next write. The post-commit purge can't prevent it: the stale reader's save() lands after the purge.

Observed downstream as an intermittent read-after-write failure (a just-disabled project service still appearing enabled ~1.7% of the time under concurrent load).

Fix

Capture the cache generation before the adapter read, and persist via saveWithLease():

$generation = $forUpdate ? '0' : $this->cache->getGeneration($documentKey);
$document = $this->adapter->getDocument(...);
...
$this->cache->saveWithLease($documentKey, $document->getArrayCopy(), $hashKey, $generation);

If a purge advanced the generation while we were reading, the write is rejected (CAS) and the next read re-fetches — instead of caching stale data. forUpdate reads still bypass the cache entirely.

Non-leasable cache adapters fall back to an unconditional save() via the Cache facade, so behaviour is unchanged for them.

Dependency / ordering

Requires the Leasable capability added in utopia-php/cache#75 (getGeneration() / saveWithLease()).

⚠️ composer.json is temporarily pinned to the cache feature branch (dev-feat/leasable-cache) so CI can resolve the new methods. Revert to the released ^3.1 once cache#75 is merged and tagged, then this can merge.

Verification

• Pint: clean. PHPStan (level 7): no errors.
• End-to-end against appwrite: a service-disable read-after-write that flaked 20/1200 (~1.7%) under 64 concurrent readers drops to 0/1200 with this change + cache#75. The CAS/rejection behaviour itself is unit-tested in cache#75.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Improved document caching to prevent stale entries during concurrent updates by using cache generation/lease-aware writes and only invalidating collections after successful lease saves.

• Chores

  • Updated the cache library dependency constraint to resolve the 3.1.x series for improved compatibility.

[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: e58963f1-fbe6-4961-a316-107323e29793

📥 Commits

Reviewing files that changed from the base of the PR and between 05ddac7 and 0f697a7.

⛔ Files ignored due to path filters (1)

• composer.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• composer.json
• src/Database/Database.php

🚧 Files skipped from review as they are similar to previous changes (1)

• src/Database/Database.php

---

📝 Walkthrough

Walkthrough

composer.json updates the utopia-php/cache requirement from ^3.0 to 3.1.* to access the new saveWithLease capability. Database::getDocument() is updated to capture the cache generation before the DB read and call cache->saveWithLease(...) with that generation instead of cache->save(...) when persisting an unlocked, non-relationship document. The collection invalidation marker write is now conditional on the lease-based save succeeding.

Changes

Generation-aware cache writes in getDocument()

Layer / File(s)	Summary
Cache dependency update to 3.1.** composer.json	Updates the require constraint for utopia-php/cache from ^3.0 to 3.1.* to pull in the released saveWithLease capability.
Generation capture and saveWithLease in getDocument() src/Database/Database.php	Before the DB row is fetched, captures the current cache generation for the document key (defaulting to '0' for forUpdate reads, otherwise fetched from cache with warning on failure). On the cache write path, replaces cache->save(...) with cache->saveWithLease(..., generation) and performs the collection invalidation marker write only when the lease-based save succeeds, preventing stale-row poisoning under concurrent updateDocument() calls.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related PRs

• utopia-php/database#887: Modifies the same getDocument() cache-population path to prevent race-driven stale-row cache poisoning, using purge tombstones as the mechanism instead of generation leases.
• utopia-php/database#888: Modifies getDocument()'s cache logic overlapping the same forUpdate bypass and conditional cache-population pathway that this PR also touches.

Poem

🐇 Fresh 3.1 hops into sight,
With generation leases shining bright,
Stale rows can't poison what we write,
saveWithLease guards our cache right —
The rabbit's cache stays pure and light! 🥕

🚥 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 accurately describes the main change: updating the cache mechanism in getDocument() to use a lease-based approach to prevent read-after-write staleness, which directly matches the primary objectives and code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 feat/leasable-getdocument

---

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.}

[coderabbitai]

Actionable comments posted: 2

🧹 Nitpick comments (1)

src/Database/Database.php (1)

4940-4941: 🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Only update the collection cache index after a successful lease save.

saveWithLease() returns false when a concurrent purge invalidates the generation, but Line 4941 still writes the collection reference. Gate that follow-up write on !== false so a rejected lease does not repopulate cache bookkeeping after the purge.

Proposed fix

-                $this->cache->saveWithLease($documentKey, $document->getArrayCopy(), $hashKey, $generation);
-                $this->cache->save($collectionKey, 'empty', $documentKey);
+                $saved = $this->cache->saveWithLease($documentKey, $document->getArrayCopy(), $hashKey, $generation);
+                if ($saved !== false) {
+                    $this->cache->save($collectionKey, 'empty', $documentKey);
+                }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/Database/Database.php` around lines 4940 - 4941, The cache update for the
collection index on line 4941 executes unconditionally, but it should only
execute when the preceding saveWithLease() call on line 4940 succeeds. Capture
the return value of saveWithLease() into a variable and wrap the subsequent
cache->save() call in a condition that only executes when saveWithLease() does
not return false, ensuring that rejected leases due to concurrent purges do not
repopulate the collection cache bookkeeping.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@composer.json`:
- Line 49: The "utopia-php/cache" dependency on line 49 is pinned to a moving
development branch (dev-feat/leasable-cache as 3.1.0) which creates
non-deterministic builds since the branch can move over time. Replace the branch
alias with a specific commit SHA to lock the exact version being used. Once the
stable 3.1 release is published on Packagist, update the dependency to use the
version constraint "^3.1" instead of the commit hash.

In `@src/Database/Database.php`:
- Around line 4882-4886: Wrap the $generation assignment that calls
getGeneration() on line 4886 in a try/catch block to match the error handling
pattern used in load() and saveWithLease() methods. Initialize $generation to
null in the catch block so that when getGeneration() throws during a cache
outage, the code gracefully falls back to database operations. Then, before
passing $generation to saveWithLease(), add a check to ensure $generation !==
null so that the cache save operation is only executed when generation was
successfully captured.

---

Nitpick comments:
In `@src/Database/Database.php`:
- Around line 4940-4941: The cache update for the collection index on line 4941
executes unconditionally, but it should only execute when the preceding
saveWithLease() call on line 4940 succeeds. Capture the return value of
saveWithLease() into a variable and wrap the subsequent cache->save() call in a
condition that only executes when saveWithLease() does not return false,
ensuring that rejected leases due to concurrent purges do not repopulate the
collection cache bookkeeping.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4201ccf9-ef4a-4312-9faa-88734fc4abc2

📥 Commits

Reviewing files that changed from the base of the PR and between 83f78ec and 911ea94.

⛔ Files ignored due to path filters (1)

• composer.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• composer.json
• src/Database/Database.php

[greptile-apps]

Greptile Summary

This PR fixes a read-after-write cache-staleness race in getDocument() by capturing the cache generation before the adapter read and persisting via saveWithLease(), so a concurrent purge that advances the generation causes the CAS to reject the now-stale write rather than poisoning the cache.

• Cache generation capture (src/Database/Database.php): generation is fetched before the DB read (inside a try/catch that falls back to '0'/no-lease on error), and saveWithLease() is used in place of the previous unconditional save(); the collection-key invalidation index entry is now only written when saveWithLease() succeeds, keeping the two in sync.
• Dependency bump (composer.json / composer.lock): cache constraint updated from ^3.0 to 3.1.* to resolve the released utopia-php/cache 3.1.0, which provides getGeneration() / saveWithLease(); no dev-branch pin is present in the current diff.
• All three issues raised in the previous review thread (unguarded getGeneration(), ungated collection-key save, composer dev-branch pin) are addressed in this version of the PR.

Confidence Score: 5/5

Safe to merge — the CAS-based cache write is a targeted, well-guarded addition with an explicit safe fallback, and all three issues from the prior review round have been addressed.

The logic change is small and correctly scoped: generation capture is guarded by try/catch with a documented no-lease fallback, the collection-key invalidation write is gated on the lease save succeeding, and composer.json points to the tagged 3.1.0 release. No new defects were found.

No files require special attention.

Important Files Changed

Filename	Overview
src/Database/Database.php	Adds generation capture + lease-aware CAS write to getDocument(); collection-key registration now gated on saveWithLease() success; getGeneration() failure falls back safely to '0' (no-lease) via try/catch
composer.json	Cache dependency tightened from ^3.0 to 3.1.* to pick up the Leasable capability; no dev-branch pin present in the current diff
composer.lock	Lock updated to utopia-php/cache 3.1.0 (tagged release) plus routine patch bumps for brick/math, ramsey/uuid, symfony/, and utopia-php/ packages

_{Reviews (3): Last reviewed commit: "chore(deps): use released cache 3.1.0 an..." | Re-trigger Greptile}