From 9340f89669979363f57aab56ff4267d4f20b9884 Mon Sep 17 00:00:00 2001 From: PINYO PATTANAWASANPORN Date: Mon, 21 Sep 2026 13:06:04 +0700 Subject: [PATCH 1/2] fix: query relations in ManyToOne deleteSetNull and guard empty collections (appwrite/appwrite#13766) In deleteSetNull(), RELATION_MANY_TO_ONE previously wrapped the related documents lookup in `if (!$twoWay)`. When deleting a child document in a two-way Many-to-One relationship, `$this->find()` was skipped, leaving `$value` unpopulated (null) and causing a fatal `TypeError: foreach() argument must be of type array|object, null given`. This fix aligns deleteSetNull() with deleteCascade() and deleteRestrict() by unconditionally querying the referencing parent documents, and adds defensive empty checks for RELATION_ONE_TO_MANY and RELATION_MANY_TO_ONE. --- src/Database/Database.php | 18 ++++++++++++------ .../Scopes/Relationships/ManyToOneTests.php | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index d581529fa5..a6a5cc2f8d 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -8194,6 +8194,10 @@ private function deleteSetNull(Document $collection, Document $relatedCollection if ($side === Database::RELATION_SIDE_CHILD) { break; } + + if (empty($value)) { + break; + } foreach ($value as $relation) { $this->authorization->skip(function () use ($relatedCollection, $twoWayKey, $relation) { $this->skipRelationships(fn () => $this->updateDocument( @@ -8212,12 +8216,14 @@ private function deleteSetNull(Document $collection, Document $relatedCollection break; } - if (!$twoWay) { - $value = $this->find($relatedCollection->getId(), [ - Query::select(['$id']), - Query::equal($twoWayKey, [$document->getId()]), - Query::limit(PHP_INT_MAX) - ]); + $value = $this->find($relatedCollection->getId(), [ + Query::select(['$id']), + Query::equal($twoWayKey, [$document->getId()]), + Query::limit(PHP_INT_MAX) + ]); + + if (empty($value)) { + break; } foreach ($value as $relation) { diff --git a/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php b/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php index e62ff735c3..0e81285959 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php @@ -782,6 +782,24 @@ public function testManyToOneTwoWayRelationship(): void $database->getDocument('product', 'product1'); $this->assertEquals(null, $product1->getAttribute('newStore')); + + // Create child with no related parents and verify deleteSetNull succeeds + $database->createDocument('store', new Document([ + '$id' => 'store8', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Store 8', + 'opensAt' => '10:00', + ])); + + $deleted = $database->deleteDocument('store', 'store8'); + $this->assertEquals(true, $deleted); + + $store8 = $database->getDocument('store', 'store8'); + $this->assertEquals(true, $store8->isEmpty()); // Change on delete to cascade $database->updateRelationship( collection: 'product', From f148181c06fc3ba53243a68d755cfc6b960de641 Mon Sep 17 00:00:00 2001 From: harsh mahajan Date: Mon, 21 Sep 2026 12:19:03 +0530 Subject: [PATCH 2/2] fix: look up referencing documents in deleteSetNull instead of trusting the relationship value deleteSetNull() read the related documents off the document being deleted. That value is only there when the document was fetched with its relationships populated. deleteDocuments() passes the caller's queries straight to find(), and a select query turns relationship population off, so the document reaches deleteSetNull() with the relationship attribute missing: - one-to-many (parent side) and many-to-one (child side) hit `foreach () argument must be of type array|object, null given`, surfacing as HTTP 500 general_unknown - one-to-one (two-way) took the `empty($value)` early return and silently left the other side pointing at a deleted row Query the referencing documents instead, the way deleteCascade() and deleteRestrict() already do, so the outcome no longer depends on how the document was loaded. The lookup runs with permissions skipped. A referencing document the caller cannot read still has to have its foreign key cleared, otherwise deleting a row leaves dangling references behind whenever document security hides some of the referencing rows. This also fixes that case for the one-way many-to-one lookup, which already queried but did so as the caller. `$value` is no longer read on entry, so it is dropped from the signature. Co-authored-by: PINYO PATTANAWASANPORN --- src/Database/Database.php | 64 ++++----- .../Scopes/Relationships/ManyToOneTests.php | 121 ++++++++++++++++++ .../Scopes/Relationships/OneToManyTests.php | 59 +++++++++ .../Scopes/Relationships/OneToOneTests.php | 59 +++++++++ 4 files changed, 274 insertions(+), 29 deletions(-) diff --git a/src/Database/Database.php b/src/Database/Database.php index a6a5cc2f8d..f5f1d066f9 100644 --- a/src/Database/Database.php +++ b/src/Database/Database.php @@ -8008,7 +8008,7 @@ private function deleteDocumentRelationships(Document $collection, Document $doc $this->deleteRestrict($relatedCollection, $document, $value, $relationType, $twoWay, $twoWayKey, $side); break; case Database::RELATION_MUTATE_SET_NULL: - $this->deleteSetNull($collection, $relatedCollection, $document, $value, $relationType, $twoWay, $twoWayKey, $side); + $this->deleteSetNull($collection, $relatedCollection, $document, $relationType, $twoWay, $twoWayKey, $side); break; case Database::RELATION_MUTATE_CASCADE: foreach ($this->relationshipDeleteStack as $processedRelationship) { @@ -8138,11 +8138,33 @@ private function deleteRestrict( } } + /** + * Find every document in $relatedCollection whose $twoWayKey points at $document. + * + * Deletes can start from a document fetched without its relationships populated - + * deleteDocuments() passes the caller's queries straight to find(), and a select + * query turns relationship population off - so the relationship value carried on + * the document cannot be trusted here. + * + * Permissions are skipped: a referencing document the caller cannot read still has + * to have its foreign key cleared, or it is left pointing at a deleted row. + * + * @return array + * @throws DatabaseException + */ + private function findReferencingDocuments(Document $relatedCollection, Document $document, string $twoWayKey): array + { + return $this->authorization->skip(fn () => $this->find($relatedCollection->getId(), [ + Query::select(['$id']), + Query::equal($twoWayKey, [$document->getId()]), + Query::limit(PHP_INT_MAX) + ])); + } + /** * @param Document $collection * @param Document $relatedCollection * @param Document $document - * @param mixed $value * @param string $relationType * @param bool $twoWay * @param string $twoWayKey @@ -8154,7 +8176,7 @@ private function deleteRestrict( * @throws RestrictedException * @throws StructureException */ - private function deleteSetNull(Document $collection, Document $relatedCollection, Document $document, mixed $value, string $relationType, bool $twoWay, string $twoWayKey, string $side): void + private function deleteSetNull(Document $collection, Document $relatedCollection, Document $document, string $relationType, bool $twoWay, string $twoWayKey, string $side): void { switch ($relationType) { case Database::RELATION_ONE_TO_ONE: @@ -8163,18 +8185,11 @@ private function deleteSetNull(Document $collection, Document $relatedCollection } // Shouldn't need read or update permission to delete - $this->authorization->skip(function () use ($document, $value, $relatedCollection, $twoWay, $twoWayKey, $side) { - if (!$twoWay && $side === Database::RELATION_SIDE_CHILD) { - $related = $this->findOne($relatedCollection->getId(), [ - Query::select(['$id']), - Query::equal($twoWayKey, [$document->getId()]) - ]); - } else { - if (empty($value)) { - return; - } - $related = $this->getDocument($relatedCollection->getId(), $value->getId(), [Query::select(['$id'])]); - } + $this->authorization->skip(function () use ($document, $relatedCollection, $twoWayKey) { + $related = $this->findOne($relatedCollection->getId(), [ + Query::select(['$id']), + Query::equal($twoWayKey, [$document->getId()]) + ]); if ($related->isEmpty()) { return; @@ -8195,10 +8210,9 @@ private function deleteSetNull(Document $collection, Document $relatedCollection break; } - if (empty($value)) { - break; - } - foreach ($value as $relation) { + $relations = $this->findReferencingDocuments($relatedCollection, $document, $twoWayKey); + + foreach ($relations as $relation) { $this->authorization->skip(function () use ($relatedCollection, $twoWayKey, $relation) { $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), @@ -8216,17 +8230,9 @@ private function deleteSetNull(Document $collection, Document $relatedCollection break; } - $value = $this->find($relatedCollection->getId(), [ - Query::select(['$id']), - Query::equal($twoWayKey, [$document->getId()]), - Query::limit(PHP_INT_MAX) - ]); - - if (empty($value)) { - break; - } + $relations = $this->findReferencingDocuments($relatedCollection, $document, $twoWayKey); - foreach ($value as $relation) { + foreach ($relations as $relation) { $this->authorization->skip(function () use ($relatedCollection, $twoWayKey, $relation) { $this->skipRelationships(fn () => $this->updateDocument( $relatedCollection->getId(), diff --git a/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php b/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php index 0e81285959..498780a564 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/ManyToOneTests.php @@ -1970,4 +1970,125 @@ public function testPartialUpdateManyToOneChildSide(): void $database->deleteCollection('departments'); $database->deleteCollection('staff'); } + + /** + * Deleting the child must clear the parent foreign keys even when the delete ran through a select query. + */ + public function testManyToOneSetNullAfterSelectDelete(): void + { + $database = static::getDatabase(); + + $collectionPermissions = [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + $documentPermissions = [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + + $database->createCollection('mto_select_parent', permissions: $collectionPermissions, documentSecurity: true); + $database->createCollection('mto_select_child', permissions: $collectionPermissions, documentSecurity: true); + $database->createAttribute('mto_select_parent', 'name', Database::VAR_STRING, 255, false); + $database->createAttribute('mto_select_child', 'name', Database::VAR_STRING, 255, false); + + $database->createRelationship( + collection: 'mto_select_parent', + relatedCollection: 'mto_select_child', + type: Database::RELATION_MANY_TO_ONE, + twoWay: true, + id: 'child', + twoWayKey: 'parent', + onDelete: Database::RELATION_MUTATE_SET_NULL, + ); + + $database->createDocument('mto_select_child', new Document([ + '$id' => 'child1', + '$permissions' => $documentPermissions, + 'name' => 'Child', + ])); + + $database->createDocument('mto_select_parent', new Document([ + '$id' => 'parent1', + '$permissions' => $documentPermissions, + 'name' => 'Parent', + 'child' => 'child1', + ])); + + // A select query turns relationship population off, so the deleted document + // reaches deleteSetNull() without its relationship value + $database->deleteDocuments('mto_select_child', [ + Query::select(['$id', 'name']), + Query::equal('$id', ['child1']), + ]); + + $survivor = $database->getDocument('mto_select_parent', 'parent1'); + $this->assertFalse($survivor->isEmpty()); + $this->assertNull($survivor->getAttribute('child')); + } + + /** + * A referencing document the caller cannot read must still have its foreign key + * cleared, otherwise it is left pointing at a deleted row. + */ + public function testManyToOneSetNullClearsUnreadableReferences(): void + { + $database = static::getDatabase(); + + // No collection-level read: only document permissions grant access + $database->createCollection('mto_hidden_product', permissions: [ + Permission::create(Role::any()), + ], documentSecurity: true); + $database->createCollection('mto_hidden_store', permissions: [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::delete(Role::any()), + ], documentSecurity: true); + + $database->createAttribute('mto_hidden_product', 'name', Database::VAR_STRING, 255, false); + $database->createAttribute('mto_hidden_store', 'name', Database::VAR_STRING, 255, false); + + $database->createRelationship( + collection: 'mto_hidden_product', + relatedCollection: 'mto_hidden_store', + type: Database::RELATION_MANY_TO_ONE, + twoWay: true, + id: 'store', + twoWayKey: 'products', + onDelete: Database::RELATION_MUTATE_SET_NULL, + ); + + $database->createDocument('mto_hidden_store', new Document([ + '$id' => 'store1', + '$permissions' => [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ], + 'name' => 'Store', + ])); + + $database->createDocument('mto_hidden_product', new Document([ + '$id' => 'product1', + '$permissions' => [ + Permission::read(Role::user('someone-else')), + Permission::update(Role::user('someone-else')), + ], + 'name' => 'Hidden Product', + 'store' => 'store1', + ])); + + $this->assertTrue($database->getDocument('mto_hidden_product', 'product1')->isEmpty()); + + $this->assertTrue($database->deleteDocument('mto_hidden_store', 'store1')); + + $hidden = $database->getAuthorization()->skip( + fn () => $database->getDocument('mto_hidden_product', 'product1') + ); + $this->assertFalse($hidden->isEmpty()); + $this->assertNull($hidden->getAttribute('store')); + } } diff --git a/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php b/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php index 7923191cd5..6a13a2d0c5 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/OneToManyTests.php @@ -2863,4 +2863,63 @@ public function testOneToManyChildSideRejectsArrayOperators(): void $database->deleteCollection('parent_o2m'); $database->deleteCollection('child_o2m'); } + + /** + * Deleting the parent must clear the child foreign keys even when the delete ran through a select query. + */ + public function testOneToManySetNullAfterSelectDelete(): void + { + $database = static::getDatabase(); + + $collectionPermissions = [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + $documentPermissions = [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + + $database->createCollection('otm_select_parent', permissions: $collectionPermissions, documentSecurity: true); + $database->createCollection('otm_select_child', permissions: $collectionPermissions, documentSecurity: true); + $database->createAttribute('otm_select_parent', 'name', Database::VAR_STRING, 255, false); + $database->createAttribute('otm_select_child', 'name', Database::VAR_STRING, 255, false); + + $database->createRelationship( + collection: 'otm_select_parent', + relatedCollection: 'otm_select_child', + type: Database::RELATION_ONE_TO_MANY, + twoWay: true, + id: 'child', + twoWayKey: 'parent', + onDelete: Database::RELATION_MUTATE_SET_NULL, + ); + + $database->createDocument('otm_select_child', new Document([ + '$id' => 'child1', + '$permissions' => $documentPermissions, + 'name' => 'Child', + ])); + + $database->createDocument('otm_select_parent', new Document([ + '$id' => 'parent1', + '$permissions' => $documentPermissions, + 'name' => 'Parent', + 'child' => ['child1'], + ])); + + // A select query turns relationship population off, so the deleted document + // reaches deleteSetNull() without its relationship value + $database->deleteDocuments('otm_select_parent', [ + Query::select(['$id', 'name']), + Query::equal('$id', ['parent1']), + ]); + + $survivor = $database->getDocument('otm_select_child', 'child1'); + $this->assertFalse($survivor->isEmpty()); + $this->assertNull($survivor->getAttribute('parent')); + } } diff --git a/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php b/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php index e67c411389..69b4c89447 100644 --- a/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php +++ b/tests/e2e/Adapter/Scopes/Relationships/OneToOneTests.php @@ -2673,4 +2673,63 @@ public function testOneToOneRelationshipRejectsArrayOperators(): void $database->deleteCollection('user_o2o'); $database->deleteCollection('profile_o2o'); } + + /** + * Deleting the child must clear the parent foreign key even when the delete ran through a select query. + */ + public function testOneToOneSetNullAfterSelectDelete(): void + { + $database = static::getDatabase(); + + $collectionPermissions = [ + Permission::create(Role::any()), + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + $documentPermissions = [ + Permission::read(Role::any()), + Permission::update(Role::any()), + Permission::delete(Role::any()), + ]; + + $database->createCollection('oto_select_parent', permissions: $collectionPermissions, documentSecurity: true); + $database->createCollection('oto_select_child', permissions: $collectionPermissions, documentSecurity: true); + $database->createAttribute('oto_select_parent', 'name', Database::VAR_STRING, 255, false); + $database->createAttribute('oto_select_child', 'name', Database::VAR_STRING, 255, false); + + $database->createRelationship( + collection: 'oto_select_parent', + relatedCollection: 'oto_select_child', + type: Database::RELATION_ONE_TO_ONE, + twoWay: true, + id: 'child', + twoWayKey: 'parent', + onDelete: Database::RELATION_MUTATE_SET_NULL, + ); + + $database->createDocument('oto_select_child', new Document([ + '$id' => 'child1', + '$permissions' => $documentPermissions, + 'name' => 'Child', + ])); + + $database->createDocument('oto_select_parent', new Document([ + '$id' => 'parent1', + '$permissions' => $documentPermissions, + 'name' => 'Parent', + 'child' => 'child1', + ])); + + // A select query turns relationship population off, so the deleted document + // reaches deleteSetNull() without its relationship value + $database->deleteDocuments('oto_select_child', [ + Query::select(['$id', 'name']), + Query::equal('$id', ['child1']), + ]); + + $survivor = $database->getDocument('oto_select_parent', 'parent1'); + $this->assertFalse($survivor->isEmpty()); + $this->assertNull($survivor->getAttribute('child')); + } }