From c17f47becfa514b4be24903a125a78dbd201d71f Mon Sep 17 00:00:00 2001 From: Jan Skrasek Date: Sat, 4 Jan 2025 19:34:28 +0100 Subject: [PATCH] fix OneHasOne loading from non-main side [closes #714] --- src/Relationships/HasOne.php | 48 ++++++++++++------- src/Relationships/OneHasOne.php | 12 ++--- .../relationships.oneHasOne.phpt | 22 +++++++++ ...ConditionsInDifferentJoinsAndSameTable.sql | 6 +-- .../CollectionTest_testJoinDifferentPath.sql | 6 +-- ...ipManyHasOneTest_testPersistenceHasOne.sql | 2 +- ...hipOneHasManyCompositePkTest_testLimit.sql | 4 +- ...HasManyRemoveTest_testRemoveCollection.sql | 2 +- ...moveTest_testRemoveCollectionAndParent.sql | 2 +- .../RelationshipOneHasManyTest_testLimit.sql | 4 +- ...neHasOneTest_testAccessFromNonMainSide.sql | 10 ++++ 11 files changed, 80 insertions(+), 38 deletions(-) create mode 100644 tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasOneTest_testAccessFromNonMainSide.sql diff --git a/src/Relationships/HasOne.php b/src/Relationships/HasOne.php index 6bf9eb0d..9abf7345 100644 --- a/src/Relationships/HasOne.php +++ b/src/Relationships/HasOne.php @@ -12,6 +12,7 @@ use Nextras\Orm\Exception\InvalidStateException; use Nextras\Orm\Exception\NullValueException; use Nextras\Orm\Repository\IRepository; +use function _PHPStan_c875e8309\React\Promise\all; use function assert; @@ -30,11 +31,23 @@ abstract class HasOne implements IRelationshipContainer /** @var ICollection|null */ protected ?ICollection $collection = null; - /** Is value validated against storage? */ + /** + * Denotes if the value is validated, i.e. if the value is pk of valid entity or if the value is not null when it is + * disallowed. + * + * By default, relationship is validated because no initial value has been set yet. + * The first setRawValue will change that to false (with exception on null, which won't be validated later). + */ protected bool $isValueValidated = true; - /** Is raw value loaded from storage and not converted yet? */ - protected bool $isValueFromStorage = false; + /** + * Denotes if the value is present. Value is not present when this relationship side + * is not the main one and the reverse side was not yet asked to get the initial value. + * After setting this value in runtime, the value is always present. + * + * If value is not present and is worked with, it is fetched via {@see fetchValue()}. + */ + protected bool $isValuePresent = true; /** @var E|string|int|null */ protected mixed $value = null; @@ -88,10 +101,9 @@ public function convertToRawValue($value) public function setRawValue($value): void { - $isChanged = $this->getPrimaryValue() !== $value; $this->value = $value; - $this->isValueValidated = !$isChanged && $value === null; - $this->isValueFromStorage = true; + $this->isValueValidated = $value === null; + $this->isValuePresent = true; } @@ -122,7 +134,7 @@ public function hasInjectedValue(): bool public function isLoaded(): bool { - return !$this->isValueFromStorage || $this->isValueValidated; + return $this->value instanceof IEntity; } @@ -141,12 +153,10 @@ public function set($value, bool $allowNull = false): bool return false; } - if (($this->parent !== null && $this->parent->isAttached()) || $value === null) { - $entity = $this->createEntity($value, $allowNull); - $isValueValidated = true; + if ($this->parent?->isAttached() === true || $value === null) { + $entity = $this->createEntity($value, allowNull: $allowNull); } else { $entity = $value; - $isValueValidated = false; } if ($entity instanceof IEntity || $entity === null) { @@ -167,8 +177,8 @@ public function set($value, bool $allowNull = false): bool } $this->value = $entity; - $this->isValueValidated = $isValueValidated; - $this->isValueFromStorage = false; + $this->isValueValidated = $entity === null || $entity instanceof IEntity; + $this->isValuePresent = true; return $isChanged; } @@ -214,7 +224,7 @@ protected function getPrimaryValue(): mixed */ protected function getValue(bool $allowPreloadContainer = true): ?IEntity { - if (!$this->isValueValidated && ($this->value !== null || $this->metadata->isNullable)) { + if ((!$this->isValueValidated && ($this->value !== null || $this->metadata->isNullable)) || !$this->isValuePresent) { $this->initValue($allowPreloadContainer); } @@ -229,9 +239,9 @@ protected function initValue(bool $allowPreloadContainer = true): void throw new InvalidStateException('Relationship is not attached to a parent entity.'); } - if ($this->isValueFromStorage && $allowPreloadContainer) { - // load the value using relationship mapper to utilize preload container and not to validate if - // relationship's entity is really present in the database; + if (!$this->isValuePresent || $allowPreloadContainer) { + // load the value using relationship mapper to utilize preload container to avoid validation if the + // relationship's entity is actually present in the database; $this->set($this->fetchValue()); } else { @@ -339,6 +349,10 @@ protected function isChanged(?IEntity $newValue): bool // newValue is null return true; + } else if (!$this->isValuePresent) { + // before initial load, we cannot detect changes + return false; + } elseif ($newValue instanceof IEntity && $newValue->isPersisted()) { // value is persisted entity or null // newValue is persisted entity diff --git a/src/Relationships/OneHasOne.php b/src/Relationships/OneHasOne.php index 9775520a..b65eb7c3 100644 --- a/src/Relationships/OneHasOne.php +++ b/src/Relationships/OneHasOne.php @@ -18,7 +18,7 @@ class OneHasOne extends HasOne public function __construct(PropertyMetadata $metadata) { parent::__construct($metadata); - $this->isValueFromStorage = !$this->metadataRelationship->isMain; + $this->isValuePresent = $this->metadataRelationship->isMain; } @@ -34,25 +34,21 @@ public function setRawValue($value): void { parent::setRawValue($value); if (!$this->metadataRelationship->isMain) { - $this->isValueValidated = false; + $this->isValuePresent = $value !== null; } } public function getRawValue() { - if ($this->isValueFromStorage && !$this->metadataRelationship->isMain) { - $this->initValue(); - } + if (!$this->isValuePresent) $this->initValue(); return parent::getRawValue(); } public function hasInjectedValue(): bool { - if ($this->isValueFromStorage && !$this->metadataRelationship->isMain) { - $this->initValue(); - } + if (!$this->isValuePresent) $this->initValue(); return parent::hasInjectedValue(); } diff --git a/tests/cases/integration/Relationships/relationships.oneHasOne.phpt b/tests/cases/integration/Relationships/relationships.oneHasOne.phpt index 5a4e1c6b..7bcf8402 100644 --- a/tests/cases/integration/Relationships/relationships.oneHasOne.phpt +++ b/tests/cases/integration/Relationships/relationships.oneHasOne.phpt @@ -46,6 +46,27 @@ class RelationshipOneHasOneTest extends DataTestCase } + public function testAccessFromNonMainSide(): void + { + $book = new Book(); + $book->author = $this->orm->authors->getByIdChecked(1); + $book->title = 'GoT'; + $book->publisher = 1; + + $ean = new Ean(); + $ean->code = 'GoTEAN'; + $ean->book = $book; + + $this->orm->books->persistAndFlush($book); + $eanId = $ean->id; + + $this->orm->clear(); + + $ean = $this->orm->eans->getByIdChecked($eanId); + Assert::same('GoT', $ean->book->title); + } + + public function testReadOnNullable(): void { $album = new PhotoAlbum(); @@ -265,6 +286,7 @@ class RelationshipOneHasOneTest extends DataTestCase $ean = new Ean(); $ean->code = '1234'; $ean->book = $this->orm->books->getByIdChecked(1); + Assert::true($ean->isAttached()); $this->orm->eans->persistAndFlush($ean); $this->orm->clear(); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testConditionsInDifferentJoinsAndSameTable.sql b/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testConditionsInDifferentJoinsAndSameTable.sql index 8857d10c..7cd91d8c 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testConditionsInDifferentJoinsAndSameTable.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testConditionsInDifferentJoinsAndSameTable.sql @@ -1,6 +1,6 @@ -SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" = 1; -SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" = 2; -SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; +SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" IN (1); +SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" IN (2); +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" IN (1); START TRANSACTION; INSERT INTO "books" ("title", "author_id", "translator_id", "next_part", "ean_id", "publisher_id", "genre", "published_at", "printed_at", "thread_id", "price", "price_currency", "orig_price_cents", "orig_price_currency") VALUES ('Books 5', 1, 2, NULL, NULL, 1, 'fantasy', '2021-12-31 23:59:59.000000'::timestamp, NULL, NULL, NULL, NULL, NULL, NULL); SELECT CURRVAL('public.books_id_seq'); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testJoinDifferentPath.sql b/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testJoinDifferentPath.sql index 3cfb51f3..aed6e372 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testJoinDifferentPath.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Collection/CollectionTest_testJoinDifferentPath.sql @@ -4,9 +4,9 @@ INSERT INTO "eans" ("code", "type") VALUES ('123', 2); SELECT CURRVAL('public.eans_id_seq'); UPDATE "books" SET "ean_id" = 1 WHERE "id" = 3; COMMIT; -SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" = 1; -SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; -SELECT "books".* FROM "books" AS "books" WHERE "books"."id" = 4; +SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" IN (1); +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" IN (1); +SELECT "books".* FROM "books" AS "books" WHERE "books"."id" IN (4); START TRANSACTION; INSERT INTO "eans" ("code", "type") VALUES ('456', 2); SELECT CURRVAL('public.eans_id_seq'); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipManyHasOneTest_testPersistenceHasOne.sql b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipManyHasOneTest_testPersistenceHasOne.sql index 866756ee..1f59ca18 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipManyHasOneTest_testPersistenceHasOne.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipManyHasOneTest_testPersistenceHasOne.sql @@ -1,4 +1,4 @@ -SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" IN (1); START TRANSACTION; INSERT INTO "public"."authors" ("name", "born", "web", "favorite_author_id") VALUES ('Jon Snow', '2021-03-21 08:23:00.000000'::timestamp, 'http://www.example.com', NULL); SELECT CURRVAL('public.authors_id_seq'); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyCompositePkTest_testLimit.sql b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyCompositePkTest_testLimit.sql index 089feff9..1bd51985 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyCompositePkTest_testLimit.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyCompositePkTest_testLimit.sql @@ -1,5 +1,5 @@ -SELECT "tags".* FROM "tags" AS "tags" WHERE "tags"."id" = 2; -SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" = 1; +SELECT "tags".* FROM "tags" AS "tags" WHERE "tags"."id" IN (2); +SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" IN (1); START TRANSACTION; INSERT INTO "tag_followers" ("created_at", "author_id", "tag_id") VALUES ('2021-12-02 19:21:00.000000'::timestamptz, 1, 2); COMMIT; diff --git a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollection.sql b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollection.sql index 75658c06..a09cc252 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollection.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollection.sql @@ -1,4 +1,4 @@ -SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" IN (1); START TRANSACTION; INSERT INTO "public"."authors" ("name", "born", "web", "favorite_author_id") VALUES ('A', '2021-03-21 08:23:00.000000'::timestamp, 'http://www.example.com', NULL); SELECT CURRVAL('public.authors_id_seq'); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollectionAndParent.sql b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollectionAndParent.sql index 2a9a972d..fe58b805 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollectionAndParent.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyRemoveTest_testRemoveCollectionAndParent.sql @@ -1,4 +1,4 @@ -SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" IN (1); START TRANSACTION; INSERT INTO "public"."authors" ("name", "born", "web", "favorite_author_id") VALUES ('A', '2021-03-21 08:23:00.000000'::timestamp, 'http://www.example.com', NULL); SELECT CURRVAL('public.authors_id_seq'); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyTest_testLimit.sql b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyTest_testLimit.sql index 1908b3c2..16ed432b 100644 --- a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyTest_testLimit.sql +++ b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasManyTest_testLimit.sql @@ -1,5 +1,5 @@ -SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" = 1; -SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; +SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" IN (1); +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" IN (1); START TRANSACTION; INSERT INTO "books" ("title", "author_id", "translator_id", "next_part", "ean_id", "publisher_id", "genre", "published_at", "printed_at", "thread_id", "price", "price_currency", "orig_price_cents", "orig_price_currency") VALUES ('Book 5', 1, NULL, NULL, NULL, 1, 'fantasy', '2021-12-31 23:59:59.000000'::timestamp, NULL, NULL, NULL, NULL, NULL, NULL); SELECT CURRVAL('public.books_id_seq'); diff --git a/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasOneTest_testAccessFromNonMainSide.sql b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasOneTest_testAccessFromNonMainSide.sql new file mode 100644 index 00000000..64b9da03 --- /dev/null +++ b/tests/sqls/NextrasTests/Orm/Integration/Relationships/RelationshipOneHasOneTest_testAccessFromNonMainSide.sql @@ -0,0 +1,10 @@ +SELECT "authors".* FROM "public"."authors" AS "authors" WHERE "authors"."id" = 1; +SELECT "publishers".* FROM "publishers" AS "publishers" WHERE "publishers"."publisher_id" = 1; +START TRANSACTION; +INSERT INTO "eans" ("code", "type") VALUES ('GoTEAN', 2); +SELECT CURRVAL('public.eans_id_seq'); +INSERT INTO "books" ("title", "author_id", "translator_id", "next_part", "ean_id", "publisher_id", "genre", "published_at", "printed_at", "thread_id", "price", "price_currency", "orig_price_cents", "orig_price_currency") VALUES ('GoT', 1, NULL, NULL, 1, 1, 'fantasy', '2021-12-31 23:59:59.000000'::timestamp, NULL, NULL, NULL, NULL, NULL, NULL); +SELECT CURRVAL('public.books_id_seq'); +COMMIT; +SELECT "eans".* FROM "eans" AS "eans" WHERE "eans"."id" = 1; +SELECT "books".* FROM "books" AS "books" WHERE "books"."ean_id" IN (1);