fix(adapter): write permission rows with the document's tenant on update

[abnegate]

The bug

createDocument binds the document's own tenant for the rows it writes to the _perms side table. updateDocument bound the adapter's:

// createDocument
$stmtPermissions->bindValue(':_tenant', $document->getTenant());

// updateDocument
$stmtRemovePermissions->bindValue(':_tenant', $this->tenant);
$stmtAddPermissions->bindValue(':_tenant', $this->tenant);

The two only diverge for a row whose tenant is not the selected one. On a shared pool that is the null-tenant _metadata row of a collection created once for every tenant: getTenantQuery() carves out OR _tenant IS NULL when the collection is Database::METADATA, so such a row is readable and updatable with any tenant selected, and Database::updateDocument deliberately forces $document['$tenant'] = $old->getTenant() to keep it where it is.

Updating one under a tenant left the document row on NULL and its permission rows on that tenant:

after createCollection (tenant null):
  read/any tenant=NULL
after updateDocument under tenant 989:
  read/any   tenant='989'
  update/any tenant='989'
document row tenant=NULL

Permission filtering reads those rows (getSQLPermissionsCondition subqueries _perms), so the shared definition became invisible to every other tenant on the pool — the same repro reports permission-filtered find as an unrelated tenant: NOT FOUND.

SQLite went further: its own updateDocument override rewrote the document row's _tenant column itself ($attributes['_tenant'] = $this->tenant), moving the row off the shared scope entirely.

The fix

All three write sites use $document->getTenant(), matching createDocument. For every ordinary document this is a no-op — $old is fetched under the tenant query, so its tenant already equals the adapter's.

Postgres is fixed for consistency with its own createDocument. Its permission filtering reads the denormalised _permissions JSONB column rather than the side table, so the regression test cannot observe the difference there — stated plainly because that adapter's change is not test-covered.

Testing

testUpdatingASharedDefinitionKeepsItsPermissionRowsTenantless in the shared PermissionTests scope, guarded to shared-tables runs on SQL adapters (Mongo keeps permissions on the document, so it cannot have this bug).

Seen red before green:

adapter	without fix	with fix
MariaDB	FAILURES	OK
MySQL	FAILURES	OK
SQLite	FAILURES	OK
Postgres	OK (filters on _permissions, cannot observe)	OK

Full tests/e2e/Adapter/SharedTables suites pass on all four: 672 tests each.

composer lint already fails on main for untouched files (Mongo.php, Redis.php); the four files here report the same violations before and after this change.

Follow-up, not changed here

SQLite::createDocument also uses $this->tenant for the document row where MariaDB::createDocument uses $document->getTenant() (src/Database/Adapter/SQLite.php:1155). Same class of divergence, but nothing here exercises it, so it is left for a change that can prove it.

🤖 Generated with Claude Code

Summary by CodeRabbit

• Bug Fixes

  • Fixed permission handling when updating shared records across tenants.
  • Shared definitions now retain tenant-independent permissions and remain visible to other tenants after updates.
  • Prevented duplicate or incorrectly tenant-scoped permission entries.

• Tests

  • Added coverage verifying shared definition visibility and permission behavior across tenants.

[coderabbitai]

Warning

Review limit reached

Next included review available in 53 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 36a5599d-ef78-4b3a-8514-1488bb951bda

📥 Commits

Reviewing files that changed from the base of the PR and between 6718834 and db47d8b.

📒 Files selected for processing (3)

• src/Database/Adapter/MariaDB.php
• src/Database/Adapter/Postgres.php
• src/Database/Adapter/SQLite.php

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f88a29e0-5938-44bc-8ae9-36bc2492bdbd

📥 Commits

Reviewing files that changed from the base of the PR and between 704dd6e and 6718834.

📒 Files selected for processing (4)

• src/Database/Adapter/MariaDB.php
• src/Database/Adapter/Postgres.php
• src/Database/Adapter/SQLite.php
• tests/e2e/Adapter/Scopes/PermissionTests.php

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The SQL adapters now use each document’s tenant for shared-table updates and permission deletion or insertion. An end-to-end test verifies that tenantless shared metadata remains visible across tenants after an update.

Changes

Shared tenant permissions

Layer / File(s)	Summary
Document tenant binding src/Database/Adapter/MariaDB.php, src/Database/Adapter/Postgres.php, src/Database/Adapter/SQLite.php	Shared-table updates use the document tenant for document assignment and permission deletion or insertion.
Shared permission validation tests/e2e/Adapter/Scopes/PermissionTests.php	The end-to-end test updates tenantless shared metadata under one tenant and verifies visibility from another tenant. Unsupported configurations are skipped, and the original tenant is restored after the test.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 67188

The update keeps shared documents and their permission rows in the document's existing tenant scope across the affected SQL adapters. No actionable merge-blocking risk remains after normal checks and review.

Suggested reviewers: fogelito

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 60.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 4 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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 summarizes the main change: permission rows now use the document's tenant during updates.
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 💡 1

📝 Generate docstrings 💡

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/update-document-permissions-tenant

---

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

Fixes SQL shared-table updates so document rows and permission side-table rows retain the persisted document tenant.

• Uses the document tenant when removing and inserting permission rows in MariaDB, PostgreSQL, and SQLite.
• Keeps SQLite document rows under their existing tenant during updates.
• Adds shared-table SQL coverage confirming tenantless metadata remains visible after an update from another tenant.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/Database/Adapter/MariaDB.php	Aligns permission-row tenant bindings with the updated document’s persisted tenant.
src/Database/Adapter/Postgres.php	Applies the same document-tenant binding consistently to PostgreSQL permission-row updates.
src/Database/Adapter/SQLite.php	Preserves the document tenant in both the main row and permission rows during updates.
tests/e2e/Adapter/Scopes/PermissionTests.php	Covers cross-tenant visibility of tenantless shared metadata after permission updates.

_{Reviews (2): Last reviewed commit: "Apply suggestions from code review" | Re-trigger Greptile}