Partial update

[fogelito]

Summary by CodeRabbit

• Bug Fixes
  • Improved document update handling to ensure only specified fields are modified during database updates.

[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: 0ab2ec42-a547-4690-85b3-6d00ace44259

📥 Commits

Reviewing files that changed from the base of the PR and between 5679019 and 0e6ad57.

📒 Files selected for processing (1)

• src/Database/Database.php

---

📝 Walkthrough

Walkthrough

The PR modifies Database::updateDocument() to restrict what data the adapter receives during updates. The method now extracts keys from the incoming document, computes allowed keys (input keys plus internal metadata), and filters the update payload to contain only those keys before passing it to the adapter.

Changes

Document update adapter filtering

Layer / File(s)	Summary
Adapter payload filtering src/Database/Database.php	updateDocument() captures input document keys and computes allowed keys from user input plus internal metadata attributes. The update payload is filtered via array_intersect_key() and passed to the adapter as $adapterDocument instead of the full $document.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• utopia-php/database#636: Both PRs modify the adapter invocation in updateDocument(); this PR filters the payload by keys while the other adds a skipPermissions flag.

Suggested reviewers

• abnegate

Poem

🐰 Keys are filtered, fields refined,
Only user input sent downstream aligned,
Adapter receives no hidden surprise,
Precision in payloads, clean and precise! ✨

🚥 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 'Partial update' directly describes the main change: restricting adapter update payload to only keys present in the incoming document, enabling partial updates.
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 partial-update

---

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

[greptile-apps]

Greptile Summary

This PR adds partial-update semantics to Database::updateDocument. Before calling the adapter, it now records the keys the caller actually provided ($inputKeys), then filters the fully-merged+encoded+cast document down to those keys plus INTERNAL_ATTRIBUTES before passing it to the adapter — so only the supplied columns are written and non-supplied columns retain their existing DB values.

• $inputKeys is captured from the raw user input before the merge with the old document, so relationship keys removed by updateDocumentRelationships and unchanged fields are correctly excluded from the adapter write.
• $internalKeys (derived from INTERNAL_ATTRIBUTES) ensures $id, $sequence, $createdAt, $updatedAt, $permissions, and $tenant are always present so every adapter's accessor methods (getSequence(), getId(), etc.) and WHERE/SET clause requirements are satisfied.
• All adapter backends (MariaDB, Postgres, SQLite, MongoDB $set, Redis sparse-merge, Memory sparse-merge) handle a partial document correctly; no adapter regression is expected.

Confidence Score: 4/5

The change is safe to merge; the core filtering logic is correct for all adapter backends and the full document is still returned to the caller.

The filtering logic is logically correct and well-targeted, but the returned document is assembled from a pre-transaction merge snapshot, so in concurrent workloads the caller may see stale values for fields that were not part of the partial update. Additionally, no tests exercise the new code path where only a subset of attributes is supplied — the existing tests always pass full documents and therefore never activate the $inputKeys filter.

src/Database/Database.php — specifically the new filter block at lines 6338–6342 and the stale-return interaction with the castingAfter/decode path for non-updated fields.

Important Files Changed

Filename

Overview

src/Database/Database.php

Adds partial-update support: captures $inputKeys before the old-doc merge, then filters the fully-processed document down to (inputKeys ∪ INTERNAL_ATTRIBUTES) before calling the adapter. Logic is sound across all adapter backends (SQL, MongoDB, Redis, Memory), but the returned document can be stale for non-updated fields in concurrent scenarios, and there are no tests for the new partial-update code path.

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

[greptile-apps]

Stale return value for non-updated fields in high-concurrency scenarios

$adapterDocument correctly limits what the adapter writes to the DB (only $inputKeys + internal fields), but $document — which is returned to the caller — is the in-memory merged snapshot assembled at the start of the transaction (old values merged over by the user's input). If another transaction writes to a non-input field between this transaction's getDocument fetch and the adapter write, the caller will receive stale values for those non-updated fields in the returned document, even though the cache is purged and subsequent reads would be accurate.

This diverges from the previous behaviour where writing the full merged document meant the caller's return value always matched what was just persisted. Consider re-fetching the document (or merging in $adapterDocument's updated fields into the final return) to guarantee the caller receives a consistent snapshot.

[greptile-apps]

No dedicated tests for the partial-update path

$inputKeys is captured here to restrict which columns the adapter writes. The existing testUpdateDocument test always passes a fully-populated document, so $inputKeys equals the full attribute set and the new filtering is never exercised. A test that passes a document with only a subset of attributes and then asserts the omitted columns retain their previous DB values would prevent regressions in this new code path.

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!