From 5f0630b83dcd63f4e7e6a55882ff5e8407647a94 Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Thu, 21 Mar 2024 14:30:45 +0100 Subject: [PATCH] fix cascade removal when 1:1 on nonMain side is removed as well [closes #654] --- src/Entity/IPropertyContainer.php | 2 +- src/Repository/RemovalHelper.php | 38 +++++++------------ .../Repository/repository.cascadeRemove.phpt | 22 ++++++++++- ...t_testCascadeWithOneHasOneRelationship.sql | 16 ++++++++ ...RemoveTest_testSingleTableInheritance.sql} | 0 5 files changed, 52 insertions(+), 26 deletions(-) create mode 100644 tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testCascadeWithOneHasOneRelationship.sql rename tests/sqls/NextrasTests/Orm/Integration/Repository/{RepositoryCascadeRemoveTest_testSti.sql => RepositoryCascadeRemoveTest_testSingleTableInheritance.sql} (100%) diff --git a/src/Entity/IPropertyContainer.php b/src/Entity/IPropertyContainer.php index d801dee2..e4458dfc 100644 --- a/src/Entity/IPropertyContainer.php +++ b/src/Entity/IPropertyContainer.php @@ -29,7 +29,7 @@ public function &getInjectedValue(); /** * Returns true if property container has a value. - * This method is called when checking value direcly via property access. + * This method is called when checking value by {@see IEntity::hasValue()} call. * @internal */ public function hasInjectedValue(): bool; diff --git a/src/Repository/RemovalHelper.php b/src/Repository/RemovalHelper.php index 1b47e52c..b293ce5e 100644 --- a/src/Repository/RemovalHelper.php +++ b/src/Repository/RemovalHelper.php @@ -142,10 +142,10 @@ private static function setNulls( $type = $propertyMeta->relationship->type; $name = $propertyMeta->name; - $value = $entity->hasValue($name) ? $entity->getValue($name) : null; - if ($value === null || ($value instanceof HasMany && $value->count() === 0)) { - continue; - } + if (!$entity->hasValue($name)) continue; + + $value = $entity->getValue($name); + if ($value instanceof HasMany && $value->count() === 0) continue; $reverseRepository = $model->getRepository($propertyMeta->relationship->repository); $reverseProperty = $propertyMeta->relationship->property !== null @@ -163,40 +163,30 @@ private static function setNulls( } } $property->set([]); - } elseif ($type === Relationship::MANY_HAS_ONE || ($type === Relationship::ONE_HAS_ONE && $propertyMeta->relationship->isMain)) { + } elseif ($type === Relationship::MANY_HAS_ONE || $type === Relationship::ONE_HAS_ONE) { $property = $entity->getProperty($name); assert($property instanceof HasOne); if ($reverseProperty !== null) { $reverseEntity = $property->getEntity(); if ($reverseEntity === null || isset($queueRemove[spl_object_id($reverseEntity)])) { - // reverse side is also being removed, do not set null to this relationship + // The reverse side is also being removed, do not set null to this relationship. continue; } $pre[] = $reverseEntity->getProperty($reverseProperty->name); + $pre[] = $reverseEntity; } $property->set(null, true); } else { - // $type === Relationship::ONE_HAS_MANY or - // $type === Relationship::ONE_HAS_ONE && !$isMain - - if (!$entity->hasValue($name) || $reverseProperty === null) { - continue; - } + // $type === Relationship::ONE_HAS_MANY + if ($reverseProperty === null) continue; if ($reverseProperty->isNullable) { - if ($type === Relationship::ONE_HAS_MANY) { - $property = $entity->getProperty($name); - assert($property instanceof IRelationshipCollection); - foreach ($property as $subValue) { - $pre[] = $subValue; - } - $property->set([]); - } else { - $property = $entity->getProperty($name); - assert($property instanceof HasOne); - $pre[] = $property->getEntity(); - $property->set(null, true); + $property = $entity->getProperty($name); + assert($property instanceof IRelationshipCollection); + foreach ($property as $subValue) { + $pre[] = $subValue; } + $property->set([]); } else { $entityClass = get_class($entity); $reverseEntityClass = $propertyMeta->relationship->entity; diff --git a/tests/cases/integration/Repository/repository.cascadeRemove.phpt b/tests/cases/integration/Repository/repository.cascadeRemove.phpt index 6f24dc46..29e5e472 100644 --- a/tests/cases/integration/Repository/repository.cascadeRemove.phpt +++ b/tests/cases/integration/Repository/repository.cascadeRemove.phpt @@ -9,9 +9,12 @@ namespace NextrasTests\Orm\Integration\Repository; use Nextras\Orm\Exception\InvalidStateException; +use NextrasTests\Orm\Author; use NextrasTests\Orm\Book; use NextrasTests\Orm\Comment; use NextrasTests\Orm\DataTestCase; +use NextrasTests\Orm\Ean; +use NextrasTests\Orm\Publisher; use Tester\Assert; @@ -52,7 +55,7 @@ class RepositoryCascadeRemoveTest extends DataTestCase } - public function testSti(): void + public function testSingleTableInheritance(): void { $comment = $this->orm->contents->getByIdChecked(2); Assert::type(Comment::class, $comment); @@ -64,6 +67,23 @@ class RepositoryCascadeRemoveTest extends DataTestCase Assert::false($thread->isPersisted()); Assert::false($comment->isPersisted()); } + + + public function testCascadeWithOneHasOneRelationship(): void + { + $ean = new Ean(); + $ean->code = "TEST"; + $book = new Book(); + $book->title = 'Title'; + $book->ean = $ean; + $book->publisher = new Publisher(); + $book->publisher->name = "Test Publisher"; + $book->author = new Author(); + $book->author->name = "Test"; + $this->orm->persistAndFlush($book); + $this->orm->removeAndFlush($book); + Assert::false($book->isPersisted()); + } } diff --git a/tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testCascadeWithOneHasOneRelationship.sql b/tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testCascadeWithOneHasOneRelationship.sql new file mode 100644 index 00000000..c9451f7f --- /dev/null +++ b/tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testCascadeWithOneHasOneRelationship.sql @@ -0,0 +1,16 @@ +START TRANSACTION; +INSERT INTO "public"."authors" ("name", "born", "web", "favorite_author_id") VALUES ('Test', '2021-03-21 08:23:00.000000'::timestamp, 'http://www.example.com', NULL); +SELECT CURRVAL('"authors_id_seq"'); +INSERT INTO "eans" ("code", "type") VALUES ('TEST', 2); +SELECT CURRVAL('"eans_id_seq"'); +INSERT INTO "publishers" ("name") VALUES ('Test Publisher'); +SELECT CURRVAL('"publishers_publisher_id_seq"'); +INSERT INTO "books" ("title", "author_id", "translator_id", "next_part", "ean_id", "publisher_id", "genre", "published_at", "printed_at", "price", "price_currency", "orig_price_cents", "orig_price_currency") VALUES ('Title', 3, NULL, NULL, 1, 4, 'fantasy', '2021-12-31 23:59:59.000000'::timestamp, NULL, NULL, NULL, NULL, NULL); +SELECT CURRVAL('"books_id_seq"'); +COMMIT; +SELECT "books_x_tags"."tag_id", "books_x_tags"."book_id" FROM "tags" AS "tags" LEFT JOIN "books_x_tags" AS "books_x_tags" ON ("books_x_tags"."tag_id" = "tags"."id") WHERE "books_x_tags"."book_id" IN (5); +SELECT "books".* FROM "books" AS "books" WHERE "books"."next_part" IN (5); +START TRANSACTION; +DELETE FROM "books" WHERE "id" = 5; +DELETE FROM "eans" WHERE "id" = 1; +COMMIT; diff --git a/tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testSti.sql b/tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testSingleTableInheritance.sql similarity index 100% rename from tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testSti.sql rename to tests/sqls/NextrasTests/Orm/Integration/Repository/RepositoryCascadeRemoveTest_testSingleTableInheritance.sql