Make withCache private

[premtsd-code]

Summary

• make Database::withCache() private
• update cache-focused unit tests to exercise the private method through local reflection helpers

Tests

• php -l src/Database/Database.php
• php -l tests/unit/QueryCacheTest.php
• php -l tests/unit/WithCacheLeaseTest.php
• git diff --check

Could not run composer lint or composer format because vendor/bin/pint is unavailable. Local dependency install was blocked by missing PHP extensions and a write permission error while downloading Composer packages.

Summary by CodeRabbit

• Bug Fixes

  • Improved query cache handling coverage to ensure cached results and cache bypass behavior continue to work as expected, including edge cases like empty results and null values.
  • Strengthened security-related cache hit checks so filtered document data remains consistent when served from cache.

• Tests

  • Updated automated test coverage to use a more reliable internal invocation path for cache-related behavior.

[coderabbitai]

📝 Walkthrough

Walkthrough

Database::withCache() visibility changed from public to private. Test files QueryCacheTest.php and WithCacheLeaseTest.php were updated to add reflection-based private helper methods that invoke withCache via ReflectionMethod, replacing all direct calls throughout the test suites.

Changes

withCache Visibility Change

Layer / File(s)	Summary
Restrict withCache to private src/Database/Database.php	withCache method visibility changed from public to private, restricting external access.
QueryCacheTest reflection wiring tests/unit/QueryCacheTest.php	Adds a private withCache(...) helper using ReflectionMethod and updates findWithCache plus all test cases to call it instead of $database->withCache(...) directly.
WithCacheLeaseTest reflection wiring tests/unit/WithCacheLeaseTest.php	Adds a private withCache(...) reflection helper and updates two lease-related tests to call it instead of $this->database->withCache(...) directly.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

• utopia-php/database#894: Introduced public Database::withCache(), which this PR now makes private, directly affecting the same method.
• utopia-php/database#905: Modified withCache() cache-write logic and added WithCacheLeaseTest, which this PR updates to use reflection-based invocation.

🚥 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 and concisely describes the main change: making Database::withCache private.
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.

✨ 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 make-with-cache-private

---

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 narrows Database::withCache() from public to private and updates the two cache-focused unit test classes to invoke the method through ReflectionMethod instead of calling it directly.

• withCache is made private in Database.php, but a full-file search finds no $this->withCache(\u2026) call sites anywhere in the class \u2014 the method is unreachable in production and is exercised only through reflection in tests.
• Both QueryCacheTest and WithCacheLeaseTest gain identical private helper methods that create a ReflectionMethod for 'withCache' and delegate to invoke(); the reflection string literal is now duplicated across two files with no compile-time safety net.

Confidence Score: 3/5

The production Database class is not affected at runtime because withCache was already unused internally, but the change leaves a private method that no production code can reach and whose test coverage relies entirely on reflection into dead code.

The method is declared private yet has zero internal callers — it cannot be invoked by any production path. The unit tests now validate a code path that is structurally unreachable in production, so the cache-lease logic they exercise is untested through real call chains.

src/Database/Database.php — specifically whether withCache should be wired into find/getDocument or removed entirely given it has no internal callers.

Important Files Changed

Filename	Overview
src/Database/Database.php	Changes withCache from public to private, but the method has no internal callers in Database.php — it becomes unreachable production code accessible only via reflection in tests.
tests/unit/QueryCacheTest.php	Adds a private withCache reflection helper and updates all call sites to route through it; test logic is unchanged but is now coupled to the method name string via reflection.
tests/unit/WithCacheLeaseTest.php	Adds a parallel reflection helper for withCache and updates both call sites; duplicates the same reflection pattern already introduced in QueryCacheTest.

Comments Outside Diff (1)

1. src/Database/Database.php, line 8614-8618 (link)

   Dead private method — never called within the class

   withCache is now private but a search of Database.php shows no internal call site ($this->withCache(…)) anywhere in the file. A private PHP method with no internal callers is unreachable production code. The production document-caching paths (getDocument, etc.) bypass this method entirely and drive $this->cache directly. The unit tests reach it only through ReflectionMethod::invoke, so they verify a code path that is never exercised at runtime, giving false confidence in the cache-lease logic. Either the method should be wired into the production cache paths (replacing the scattered direct $this->cache calls), or — if it was always purely a test seam — it should be removed and the tests should be rewritten to exercise the public find/getDocument API instead.

   Prompt To Fix With AI

   This is a comment left during a code review.
   Path: src/Database/Database.php
   Line: 8614-8618
   
   Comment:
   **Dead private method — never called within the class**
   
   `withCache` is now `private` but a search of `Database.php` shows no internal call site (`$this->withCache(…)`) anywhere in the file. A private PHP method with no internal callers is unreachable production code. The production document-caching paths (`getDocument`, etc.) bypass this method entirely and drive `$this->cache` directly. The unit tests reach it only through `ReflectionMethod::invoke`, so they verify a code path that is never exercised at runtime, giving false confidence in the cache-lease logic. Either the method should be wired into the production cache paths (replacing the scattered direct `$this->cache` calls), or — if it was always purely a test seam — it should be removed and the tests should be rewritten to exercise the public `find`/`getDocument` API instead.
   
   How can I resolve this? If you propose a fix, please make it concise.

   

Prompt To Fix All With AI

Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.

---

### Issue 1 of 2
src/Database/Database.php:8614-8618
**Dead private method — never called within the class**

`withCache` is now `private` but a search of `Database.php` shows no internal call site (`$this->withCache(…)`) anywhere in the file. A private PHP method with no internal callers is unreachable production code. The production document-caching paths (`getDocument`, etc.) bypass this method entirely and drive `$this->cache` directly. The unit tests reach it only through `ReflectionMethod::invoke`, so they verify a code path that is never exercised at runtime, giving false confidence in the cache-lease logic. Either the method should be wired into the production cache paths (replacing the scattered direct `$this->cache` calls), or — if it was always purely a test seam — it should be removed and the tests should be rewritten to exercise the public `find`/`getDocument` API instead.

### Issue 2 of 2
tests/unit/QueryCacheTest.php:37-48
**Reflection-based test helper is fragile**

`new \ReflectionMethod(Database::class, 'withCache')` will throw a `ReflectionException` at runtime if the method is renamed or removed, with no compile-time signal. The same helper is duplicated in `WithCacheLeaseTest`, so a rename would silently break two test files. Consider naming the helper more explicitly (e.g. `invokeWithCache`) or centralising it in a shared trait so the single `'withCache'` string literal is easy to find and update.

_{Reviews (1): Last reviewed commit: "Make withCache private" | Re-trigger Greptile}

[greptile-apps]

Reflection-based test helper is fragile

new \ReflectionMethod(Database::class, 'withCache') will throw a ReflectionException at runtime if the method is renamed or removed, with no compile-time signal. The same helper is duplicated in WithCacheLeaseTest, so a rename would silently break two test files. Consider naming the helper more explicitly (e.g. invokeWithCache) or centralising it in a shared trait so the single 'withCache' string literal is easy to find and update.

Prompt To Fix With AI

This is a comment left during a code review.
Path: tests/unit/QueryCacheTest.php
Line: 37-48

Comment:
**Reflection-based test helper is fragile**

`new \ReflectionMethod(Database::class, 'withCache')` will throw a `ReflectionException` at runtime if the method is renamed or removed, with no compile-time signal. The same helper is duplicated in `WithCacheLeaseTest`, so a rename would silently break two test files. Consider naming the helper more explicitly (e.g. `invokeWithCache`) or centralising it in a shared trait so the single `'withCache'` string literal is easy to find and update.

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/Database/Database.php (1)

8614-8618: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Remove or wire up withCache src/Database/Database.php:8614
Only reflection-based unit tests reach this helper; no production path calls it, so the private method is dead code unless a public caller is added.

🤖 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 8614 - 8618, The private helper
withCache in Database is currently unreachable from production code, so either
remove it if it is unused or wire it into a real caller path; if you keep it,
add a public/internal method that invokes withCache and ensure the cache
key/hash behavior is exercised by an existing Database workflow. Use the
withCache method name and related Database call sites to locate the change.

Source: Linters/SAST tools

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

Outside diff comments:
In `@src/Database/Database.php`:
- Around line 8614-8618: The private helper withCache in Database is currently
unreachable from production code, so either remove it if it is unused or wire it
into a real caller path; if you keep it, add a public/internal method that
invokes withCache and ensure the cache key/hash behavior is exercised by an
existing Database workflow. Use the withCache method name and related Database
call sites to locate the change.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: b18d3616-fe03-409b-8f77-a472c6fd98db

📥 Commits

Reviewing files that changed from the base of the PR and between a090699 and 577049e.

📒 Files selected for processing (3)

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