Notify related documents when deleting two-way relationships - #13690
Conversation
✨ Benchmark resultsComparing
Per-scenario breakdown & investigation detailsMetrics below reflect the current branch (after). Δ P95 compares against the base.
Top API waits (after)
|
|
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.
🟢 Tier S · Ready to mergeThis 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.
📂 Walkthrough · 6
✅ Fixed since the last review · 2
Reviewed the commits since |
…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.
|
|
||
| $collectionsCache = []; | ||
|
|
||
| foreach ($related as $peer) { |
There was a problem hiding this 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.
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.There was a problem hiding this comment.
Doesn't apply: the library keys related documents by collection and id, so a peer linked through several relationships fires one update.
There was a problem hiding this comment.
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.
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
updatefor every surviving peer the delete changed. utopia-php/database 7.4.0 (utopia-php/database#981) firesEVENT_DOCUMENT_UPDATEfor each of them once the delete commits; the action listens for it during the delete and publishes one realtime update per peer. This covers everyonDeletemode:setNull,restrictand child-sidecascadeall leave the peer alive with a changed relationship.cascaderemoves, 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 anupdatewithout 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 receivesrows.<id>.updateafter a child row is deleted.Out of scope:
Notes:
output(), which would replace the audited response payload.getDatabasesDBbuilds a new database instance per call anyway.