Skip to content

Notify related documents when deleting two-way relationships - #13690

Merged
HarshMN2345 merged 29 commits into
mainfrom
codex/fix-relationship-delete-realtime
Sep 29, 2026
Merged

HarshMN2345 merged 29 commits into
mainfrom
codex/fix-relationship-delete-realtime

Conversation

@HarshMN2345

@HarshMN2345 HarshMN2345 commented Sep 15, 2026 •

Copy link
Copy Markdown
Member

Deleting a document can change a two-way relationship on a document that survives it without writing to that document. For example, deleting a part removes it from its assembly's parts, but a subscriber to the assembly receives nothing.

After a successful direct delete, publish a realtime update for every surviving peer the delete changed. utopia-php/database 7.4.0 (utopia-php/database#981) fires EVENT_DOCUMENT_UPDATE for each of them once the delete commits; the action listens for it during the delete and publishes one realtime update per peer. This covers every onDelete mode:

  • setNull, restrict and child-side cascade all leave the peer alive with a changed relationship.
  • Documents a cascade removes, directly or further down its chain, are not reported.

Nothing is re-read: the database hands over each peer as the delete left it. Each update is routed with the peer's own collection and permissions, and a peer linked through several attributes gets one update. The original delete event and the response/audit payload are untouched. This applies to Databases and TablesDB. Requires utopia-php/database ^7.4.0.

Fixes #6265.

Tests (RealtimeCustomClientTest):

  • testChannelDatabaseRelationshipDelete: the surviving side receives an update without relationship keys. Cases: oneToMany setNull, restrict and cascade (child deleted); oneToOne setNull (parent deleted); manyToOne restrict (parent deleted); manyToMany cascade (child deleted).
  • testChannelDatabaseRelationshipDeletePermissions: a user who can read the parent receives the update; a guest does not.
  • testChannelTablesDBRelationshipDelete: the parent row channel receives rows.<id>.update after a child row is deleted.

Out of scope:

  • One-way relationships.
  • Documents changed transitively by a cascade.
  • Bulk deletes and transaction commits.

Notes:

  • Peer updates go to realtime only, not to functions or webhooks, and can arrive before the delete event.
  • The peer payload is shaped by the response model's filter without calling output(), which would replace the audited response payload.
  • The listener is scoped to the delete call and removed afterwards; getDatabasesDB builds a new database instance per call anyway.
  • The delete has already happened when the updates are published, so a failure there is logged as a warning and the request still returns 204 with its normal delete event.

@github-actions

github-actions Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

✨ Benchmark results

Comparing main (before) → codex/fix-relationship-delete-realtime (after).

Metric Before After Change
🚀 Requests/sec 175.23 179.73 ⚪ +2.6%
⏱️ Latency P50 98.72 ms 96.56 ms ⚪ -2.2%
⏱️ Latency P95 231.92 ms 222.65 ms ⚪ -4%
Per-scenario breakdown & investigation details

Metrics below reflect the current branch (after). Δ P95 compares against the base.

Scenario P50 (ms) P95 (ms) Requests RPS Δ P95 (ms)
API total 96.56 222.65 11,400 179.73 -9.27
Account 182.79 340.84 600 10.15 -4.97
TablesDB 93.82 177.08 6,200 100.1 -5.97
Storage 87.82 188.12 3,000 49.74 -8.77
Functions 139.44 265.97 1,600 27.22 -8.58

Top API waits (after)

API request Max wait (ms)
functions.variables.update 468.18
account.name.update 462.37
account.prefs.update 409.84
functions.delete 384.73
storage.buckets.create 383.41

@HarshMN2345
HarshMN2345 marked this pull request as ready for review September 15, 2026 07:49
@greptile-apps

greptile-apps Bot commented Sep 15, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[High risk] Upgrades database library and adds relationship-delete notifications.

The PR appears safe to merge, although duplicate notifications for a peer linked through multiple relationship attributes remain possible.

Fix All in Claude CodeFindings

  1. P2 Duplicate peer updates ▶
Fix with agent prompt
### Issue 1
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Delete.php:undefined-318
When two relationship attributes link the deleted document to the same surviving peer, each database update is added to `$related`. This loop then sends an update for every entry, so subscribers can receive multiple notifications for one peer. Deduplicating peers before publishing would avoid the extra messages.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR adds realtime updates for surviving documents affected by a direct two-way relationship delete, updates the database dependency, and adds Databases and TablesDB end-to-end coverage.

  • Peer updates use the surviving document’s collection and permissions.
  • The original delete event and response remain separate from the peer notifications.

Reviews (9) · Last reviewed commit: "Merge branch 'main' into codex/fix-relat..."

HarshMN2345 and others added 3 commits September 15, 2026 14:22
Restrict and child-side cascade deletes also change surviving two-way
peers, so drop the setNull filter; cascaded peers read back empty and are
skipped. Re-read peers without loading relationships, and shape the
payload through the model filter so output() no longer replaces the
audited response payload.
Cover setNull, restrict and cascade survivors across cardinalities in one
test, permission routing with an owner and a guest socket, and the TablesDB
row channel. Drop tests that never reach the new code.
Comment thread tests/e2e/Services/Realtime/RealtimeCustomClientTest.php Outdated
@hansi-codes

hansi-codes Bot commented Sep 29, 2026 •

Copy link
Copy Markdown

🟢 Tier S · Ready to merge

This pull request publishes realtime updates for related documents that survive a direct delete across Databases, DocumentsDB, TablesDB, and VectorsDB. It upgrades the database dependency to supply relationship update events and adds realtime E2E coverage for relationship types, permissions, payloads, and TablesDB rows. The latest changes isolate notification failures from the completed delete and strengthen the payload assertion.

Latest changes: The newest commits catch and log peer-notification failures after deletion and assert the surviving document ID and omitted relationship key in the realtime test.

Verdict New comments Fixed Still open
✅ Approved 0 2 0
📂 Walkthrough · 6
File Change
composer.json, composer.lock Require utopia-php/database 7.4.0 for relationship update events.
src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Delete.php Capture relationship peer updates during deletion, publish realtime events, and log notification failures without failing the delete.
src/Appwrite/Platform/Modules/Databases/Http/DocumentsDB/Collections/Documents/Delete.php Inject the realtime publisher into the inherited document delete action.
src/Appwrite/Platform/Modules/Databases/Http/TablesDB/Tables/Rows/Delete.php Inject the realtime publisher into the inherited row delete action.
src/Appwrite/Platform/Modules/Databases/Http/VectorsDB/Collections/Documents/Delete.php Inject the realtime publisher into the inherited document delete action.
tests/e2e/Services/Realtime/RealtimeCustomClientTest.php Test relationship-delete notifications, permissions, TablesDB channels, and survivor payload fields.
✅ Fixed since the last review · 2
  • Keep notification lookup failures from failing a completed delete · src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Delete.php:248
  • Assert the surviving document's update payload · tests/e2e/Services/Realtime/RealtimeCustomClientTest.php:4310

Reviewed the commits since 448f7d2 · Details · Comment @hansi-codes review to re-run, or mention @hansi-codes with a question.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Tier B · 1 blocking finding to address. Summary

Comment thread tests/e2e/Services/Realtime/RealtimeCustomClientTest.php Outdated
…te events

The database now fires an update for each two-way related document a
delete changes, after the delete commits. Collect those while deleting
instead of walking the relationships and re-reading every related
document afterwards.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Tier B · See the inline comments. Summary


$collectionsCache = [];

foreach ($related as $peer) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Duplicate peer updates When two relationship attributes link the deleted document to the same surviving peer, each database update is added to $related. This loop then sends an update for every entry, so subscribers can receive multiple notifications for one peer. Deduplicating peers before publishing would avoid the extra messages.

Prompt To Fix With AI
This is a comment left during a code review.
Path: src/Appwrite/Platform/Modules/Databases/Http/Databases/Collections/Documents/Delete.php
Line: 312

Comment:
**Duplicate peer updates** When two relationship attributes link the deleted document to the same surviving peer, each database update is added to `$related`. This loop then sends an update for every entry, so subscribers can receive multiple notifications for one peer. Deduplicating peers before publishing would avoid the extra messages.

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Fix in Claude Code Fix in Codex

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't apply: the library keys related documents by collection and id, so a peer linked through several relationships fires one update.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. The database event collector keys related documents by collection and document ID, so multiple relationship attributes pointing to the same peer are coalesced before this loop. My duplicate-notification concern does not apply here, so I withdraw it.

@hansi-codes hansi-codes Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Tier A · Looks good to merge. Summary

@HarshMN2345
HarshMN2345 merged commit 915af23 into main Sep 29, 2026
218 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

🐛 Bug Report: Realtime not triggering document updates on parent for removals of association

1 participant