diff --git a/src/Database/Database.php b/src/Database/Database.php index d581529fa5..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; @@ -8194,7 +8209,10 @@ private function deleteSetNull(Document $collection, Document $relatedCollection if ($side === Database::RELATION_SIDE_CHILD) { 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(), @@ -8212,15 +8230,9 @@ 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) - ]); - } + $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 e62ff735c3..498780a564 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', @@ -1952,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')); + } }