Fix cache clobber

[abnegate]

Summary by CodeRabbit

• Bug Fixes

  • Ensure document updates read fresh data during updates and avoid applying stale cached values; update detection now respects attribute types to prevent unnecessary/no-op changes.

• Tests

  • Adds end-to-end test verifying updates bypass stale cache and subsequent reads reflect the latest state.

• Chores

  • Streamlines the Docker image and build stages, simplifying extensions and build steps while preserving debug and MongoDB support.

[coderabbitai]

Warning

Rate limit exceeded

@abnegate has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 6 minutes and 37 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📝 Walkthrough

📝 Walkthrough

🚥 Pre-merge checks | ✅ 3 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 40.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Fix cache clobber' directly addresses the main issue being resolved: preventing cache from being overwritten by stale data during concurrent updates.
Merge Conflict Detection	✅ Passed	✅ No merge conflicts detected when merging into main

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

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-cache-clobber

---

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 and usage tips.}

[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)

5699-5705: ⚠️ Potential issue | 🟠 Major

Remove or gate the debug log to avoid leaking data.
Line 5704 logs raw values (including potential PII) and could flood logs; please remove or guard behind a debug flag/level.

🧹 Suggested change (remove debug log)

-                        error_log("DEBUG shouldUpdate: key={$key} type_new=" . gettype($value) . " type_old=" . gettype($oldValue) . " val_new=" . json_encode($value) . " val_old=" . json_encode($oldValue));

[coderabbitai]

Actionable comments posted: 2

🤖 Fix all issues with AI agents

In `@src/Database/Database.php`:
- Around line 5703-5706: The numeric-equality shortcut using is_numeric($value)
&& is_numeric($oldValue) in Database::... lets numeric-like strings (e.g. "01"
vs "1") bypass $shouldUpdate authorization; restrict the shortcut so it only
applies to true numeric scalars (ints/floats) or to attributes whose declared
type is numeric: check scalar types (e.g. is_int/is_float) or the attribute
metadata before continuing, and only then continue; otherwise fall back to the
normal equality/authorization path that respects $shouldUpdate. Ensure you
update the condition that references $value, $oldValue and is_numeric so it
never skips authorization for string values.

In `@tests/e2e/Adapter/Scopes/DocumentTests.php`:
- Line 5924: The test currently calls \usleep(2000) to try to force an updatedAt
change which is flaky across adapters; replace this single microsecond sleep by
waiting until the timestamp actually changes (either use sleep(1) to wait at
least one second or implement a short loop that reloads the document and breaks
when updatedAt !== createdAt, with a reasonable timeout) in the test that
contains the \usleep(2000) call so the assertion comparing createdAt and
updatedAt is deterministic.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Avoid flaky timestamp comparisons across adapters.

A 2ms sleep may not advance $updatedAt on adapters with second/millisecond precision, making the subsequent assertion non-deterministic. Consider sleeping at least a second (or looping until the timestamp changes).

🛠️ Suggested fix

-        \usleep(2000); // Ensure updatedAt timestamp differs from creation time
+        // Some adapters only store timestamps with second precision.
+        \usleep(1_100_000); // Ensure updatedAt timestamp differs from creation time

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	\usleep(2000); // Ensure updatedAt timestamp differs from creation time
	// Some adapters only store timestamps with second precision.
	\usleep(1_100_000); // Ensure updatedAt timestamp differs from creation time

🤖 Prompt for AI Agents

In `@tests/e2e/Adapter/Scopes/DocumentTests.php` at line 5924, The test currently
calls \usleep(2000) to try to force an updatedAt change which is flaky across
adapters; replace this single microsecond sleep by waiting until the timestamp
actually changes (either use sleep(1) to wait at least one second or implement a
short loop that reloads the document and breaks when updatedAt !== createdAt,
with a reasonable timeout) in the test that contains the \usleep(2000) call so
the assertion comparing createdAt and updatedAt is deterministic.

[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)

5604-5717: ⚠️ Potential issue | 🟠 Major

Type-aware comparison can misfire for array attributes and uncasted old values.
Casting only $value (and without array awareness) can force $shouldUpdate=true for equal arrays or when $oldValue is still string/uncast, leading to unnecessary updates and stricter permission checks. Consider tracking the array flag and casting both sides only for scalar attributes.

🔧 Suggested adjustment (array-aware + symmetric casting)

-            $attributeTypes = [];
-            foreach ($collection->getAttribute('attributes', []) as $attr) {
-                $attributeTypes[$attr['$id'] ?? ''] = $attr['type'] ?? '';
-            }
+            $attributeTypes = [];
+            foreach ($collection->getAttribute('attributes', []) as $attr) {
+                $attrId = $attr['$id'] ?? null;
+                if ($attrId !== null) {
+                    $attributeTypes[$attrId] = [
+                        'type' => $attr['type'] ?? null,
+                        'array' => (bool)($attr['array'] ?? false),
+                    ];
+                }
+            }

             $oldValue = $old->getAttribute($key);

             // Cast value to attribute type for consistent comparison
             // (e.g. cache JSON round-trip turns float 1.0 into int 1)
-            $attrType = $attributeTypes[$key] ?? null;
-            if ($attrType !== null && !\is_null($value) && !($value instanceof Operator)) {
-                $value = match ($attrType) {
-                    self::VAR_FLOAT => (float)$value,
-                    self::VAR_INTEGER => (int)$value,
-                    self::VAR_BOOLEAN => (bool)$value,
-                    default => $value,
-                };
-            }
+            $attrMeta = $attributeTypes[$key] ?? null;
+            if ($attrMeta && !$attrMeta['array'] && !\is_null($value) && !($value instanceof Operator)) {
+                $cast = fn ($v) => match ($attrMeta['type']) {
+                    self::VAR_FLOAT => (float)$v,
+                    self::VAR_INTEGER => (int)$v,
+                    self::VAR_BOOLEAN => (bool)$v,
+                    default => $v,
+                };
+                $value = $cast($value);
+                if ($oldValue !== null) {
+                    $oldValue = $cast($oldValue);
+                }
+            }