From 3f9edacde90729f0beb87163ce52e6d84fd85ea8 Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 15 Jun 2026 19:08:49 +0400 Subject: [PATCH 1/3] Add RefersToMorphed relation for cyclic morphed references BelongsToMorphed inherits the hard "parent before child" dependency of BelongsTo and therefore deadlocks the pool when persisting a closed cycle (A > A or A > B > A) in a single transaction. There was no morphed counterpart of RefersTo to break such cycles. Add Relation::REFERS_TO_MORPHED and the RefersToMorphed relation, which mirrors BelongsToMorphed on top of RefersTo: it stores the outer key and the target role (morph key) on the owner but resolves the outer key in a deferred, "soft" way, allowing self-linked and cyclic morphed references to be persisted within one transaction. The morph key is kept in sync with the related role and cleared when the relation is set to null; the BelongsToMorphedLoader is reused. Tests cover A > A and A > B > A (read, single-transaction create, detach) on all four drivers, plus the previously uncovered interaction of both morphed belongs-to/refers-to relations with Options::$ignoreUninitializedRelations. --- psalm-baseline.xml | 26 +- src/Config/RelationConfig.php | 4 + src/Relation.php | 1 + src/Relation/Morphed/RefersToMorphed.php | 108 +++++++ tests/ORM/Fixtures/MorphedCyclic/EntityA.php | 17 ++ tests/ORM/Fixtures/MorphedCyclic/EntityB.php | 17 ++ .../MorphedCyclic/MorphedInterface.php | 8 + .../Morphed/BelongsToMorphedRelationTest.php | 42 +++ .../Morphed/RefersToMorphedCyclicTest.php | 263 ++++++++++++++++++ .../Morphed/RefersToMorphedCyclicTest.php | 17 ++ .../Morphed/RefersToMorphedCyclicTest.php | 17 ++ .../Morphed/RefersToMorphedCyclicTest.php | 17 ++ .../Morphed/RefersToMorphedCyclicTest.php | 17 ++ 13 files changed, 551 insertions(+), 3 deletions(-) create mode 100644 src/Relation/Morphed/RefersToMorphed.php create mode 100644 tests/ORM/Fixtures/MorphedCyclic/EntityA.php create mode 100644 tests/ORM/Fixtures/MorphedCyclic/EntityB.php create mode 100644 tests/ORM/Fixtures/MorphedCyclic/MorphedInterface.php create mode 100644 tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php create mode 100644 tests/ORM/Functional/Driver/MySQL/Relation/Morphed/RefersToMorphedCyclicTest.php create mode 100644 tests/ORM/Functional/Driver/Postgres/Relation/Morphed/RefersToMorphedCyclicTest.php create mode 100644 tests/ORM/Functional/Driver/SQLServer/Relation/Morphed/RefersToMorphedCyclicTest.php create mode 100644 tests/ORM/Functional/Driver/SQLite/Relation/Morphed/RefersToMorphedCyclicTest.php diff --git a/psalm-baseline.xml b/psalm-baseline.xml index 16653684d..3866cb0ef 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -1653,10 +1653,22 @@ - + - + + + + + + + + + + morphKey]]> + + + @@ -2217,14 +2229,22 @@ define(SchemaInterface::COLUMNS)]]> - factory->make($loader->options['scope'])]]> options['minify']]]> options['minify']]]> schema[$key]]]> schema[$key]]]> + $scope, + \is_string($scope) => $this->factory->make($scope), + // false/null explicitly disable the scope for this relation + $scope === false, $scope === null => null, + // true (the default) means: use the relation source's scope + default => $this->source->getScope(), + }]]> + options['as']]]> diff --git a/src/Config/RelationConfig.php b/src/Config/RelationConfig.php index 1c5e8786d..5f1368f4c 100644 --- a/src/Config/RelationConfig.php +++ b/src/Config/RelationConfig.php @@ -57,6 +57,10 @@ public static function getDefault(): self self::LOADER => Select\Loader\Morphed\BelongsToMorphedLoader::class, self::RELATION => Relation\Morphed\BelongsToMorphed::class, ], + Relation::REFERS_TO_MORPHED => [ + self::LOADER => Select\Loader\Morphed\BelongsToMorphedLoader::class, + self::RELATION => Relation\Morphed\RefersToMorphed::class, + ], ]); } diff --git a/src/Relation.php b/src/Relation.php index 72bb27e88..d16d5448b 100644 --- a/src/Relation.php +++ b/src/Relation.php @@ -29,6 +29,7 @@ final class Relation // Morphed relations public const BELONGS_TO_MORPHED = 20; public const MORPHED_HAS_ONE = 21; + public const REFERS_TO_MORPHED = 22; public const MORPHED_HAS_MANY = 23; // Custom morph key diff --git a/src/Relation/Morphed/RefersToMorphed.php b/src/Relation/Morphed/RefersToMorphed.php new file mode 100644 index 000000000..81c257fdb --- /dev/null +++ b/src/Relation/Morphed/RefersToMorphed.php @@ -0,0 +1,108 @@ + A, A > B > A) that {@see BelongsToMorphed} can + * not persist in a single transaction. + * + * @internal + */ +class RefersToMorphed extends RefersTo +{ + private string $morphKey; + + public function __construct(ORMInterface $orm, string $role, string $name, string $target, array $schema) + { + parent::__construct($orm, $role, $name, $target, $schema); + $this->morphKey = $schema[Relation::MORPH_KEY]; + } + + public function initReference(Node $node): ReferenceInterface + { + $scope = $this->getReferenceScope($node); + $nodeData = $node->getData(); + if (!isset($nodeData[$this->morphKey], $scope)) { + return new EmptyReference('?', null); + } + $target = $nodeData[$this->morphKey]; + + return $scope === [] ? new EmptyReference($target, null) : new Reference($target, $scope); + } + + public function prepare(Pool $pool, Tuple $tuple, mixed $related, bool $load = true): void + { + // The parent RefersTo resets the node relation while handling a null value, so capture + // whether there was a related entity before delegating. + $hadRelation = $tuple->node->getRelation($this->getName()) !== null; + parent::prepare($pool, $tuple, $related, $load); + $this->syncMorphKey($pool, $tuple, $hadRelation); + } + + public function queue(Pool $pool, Tuple $tuple): void + { + $hadRelation = $tuple->node->getRelation($this->getName()) !== null; + parent::queue($pool, $tuple); + $this->syncMorphKey($pool, $tuple, $hadRelation); + } + + /** + * Assert that given entity is allowed for the relation. + * + * @throws RelationException + */ + protected function assertValid(Node $related): void + { + // no need to validate morphed relation yet + } + + /** + * Keep the morph key in sync with the related entity role. The role is known as soon as the + * related object is available, so it can be registered eagerly even while the outer key is + * still deferred by the parent {@see RefersTo} logic. + */ + private function syncMorphKey(Pool $pool, Tuple $tuple, bool $hadRelation): void + { + $relName = $this->getName(); + $state = $tuple->state; + if (!$state->hasRelation($relName)) { + return; + } + + $related = $state->getRelation($relName); + + if ($related === null) { + // Reset the morph key when the relation was changed to null + if ($hadRelation) { + $state->register($this->morphKey, null); + } + return; + } + + if ($related instanceof EmptyReference) { + return; + } + + $role = $related instanceof ReferenceInterface + ? $related->getRole() + : $pool->offsetGet($related)?->node->getRole(); + + $state->register($this->morphKey, $role); + } +} diff --git a/tests/ORM/Fixtures/MorphedCyclic/EntityA.php b/tests/ORM/Fixtures/MorphedCyclic/EntityA.php new file mode 100644 index 000000000..db9d45c5b --- /dev/null +++ b/tests/ORM/Fixtures/MorphedCyclic/EntityA.php @@ -0,0 +1,17 @@ +assertNumReads(0); } + /** + * With ignoreUninitializedRelations = true (BaseTest default) unsetting the relation property + * must leave both the outer key and the morph key untouched. + */ + public function testUnsetParentKeepsMorphWhenIgnoringUninitialized(): void + { + $c = $this->orm->getRepository(Image::class)->findByPK(1); + $this->assertInstanceOf(User::class, $c->parent); + unset($c->parent); + + $this->captureWriteQueries(); + $this->save($c); + $this->assertNumWrites(0); + + $row = $this->getDatabase()->table('image')->select()->where('id', 1)->fetchAll(); + $this->assertSame('1', (string) $row[0]['parent_id']); + $this->assertSame('user', $row[0]['parent_type']); + } + + /** + * With ignoreUninitializedRelations = false an unset relation is treated as null, so both the + * outer key and the morph key must be cleared. + */ + public function testUnsetParentClearsMorphWithoutIgnoreUninitialized(): void + { + $this->orm = $this->withSchema(new Schema($this->getNullableMorphedSchemaArray())) + ->with(options: (new Options())->withIgnoreUninitializedRelations(false)); + + $c = $this->orm->getRepository(Image::class)->findByPK(1); + $this->assertInstanceOf(User::class, $c->parent); + unset($c->parent); + + $this->captureWriteQueries(); + $this->save($c); + $this->assertNumWrites(1); + + $row = $this->getDatabase()->table('image')->select()->where('id', 1)->fetchAll(); + $this->assertNull($row[0]['parent_id'], 'parent_id should be NULL when the unset relation is treated as null'); + $this->assertNull($row[0]['parent_type'], 'parent_type should be NULL when the unset relation is treated as null'); + } + public function setUp(): void { parent::setUp(); diff --git a/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php b/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php new file mode 100644 index 000000000..d846d00c8 --- /dev/null +++ b/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php @@ -0,0 +1,263 @@ + A: an entity points to itself through a morphed relation; + * - A > B > A: two entities point at each other through morphed relations. + * + * Unlike {@see Relation::BELONGS_TO_MORPHED} (a hard "parent before child" dependency that + * deadlocks the pool on a cycle), the morphed refers-to relation resolves the outer key in a + * deferred way and therefore is able to persist a closed cycle within a single transaction. + * The related entity is resolved lazily through a promise reference. + */ +abstract class RefersToMorphedCyclicTest extends BaseTest +{ + use TableTrait; + + public function testFetchSelfReference(): void + { + // entity_a #1 references itself + $a = $this->orm->getRepository(EntityA::class)->findByPK(1); + $data = $this->extractEntity($a); + + $this->assertInstanceOf(ReferenceInterface::class, $data['parent']); + + $this->assertInstanceOf(EntityA::class, $a->parent); + $this->assertSame('a-self', $a->parent->name); + $this->assertSame($a->id, $a->parent->id); + } + + public function testFetchCycleTwoEntities(): void + { + // entity_a #2 -> entity_b #1 -> entity_a #2 + $a = $this->orm->getRepository(EntityA::class)->findByPK(2); + + $this->assertInstanceOf(EntityB::class, $a->parent); + $this->assertSame('b-1', $a->parent->name); + + $this->assertInstanceOf(EntityA::class, $a->parent->parent); + $this->assertSame($a->id, $a->parent->parent->id); + } + + public function testSetSelfReferenceToNull(): void + { + $a = $this->orm->getRepository(EntityA::class)->findByPK(1); + $this->assertInstanceOf(EntityA::class, $a->parent); + + $a->parent = null; + $this->save($a); + + $row = $this->getDatabase()->table('entity_a')->select()->where('id', 1)->fetchAll(); + $this->assertNull($row[0]['parent_id'], 'parent_id should be NULL after detaching self-reference'); + $this->assertNull($row[0]['parent_type'], 'parent_type should be NULL after detaching self-reference'); + } + + public function testCreateSelfReference(): void + { + $a = new EntityA(); + $a->name = 'new-self'; + $a->parent = $a; + + $this->captureWriteQueries(); + $this->save($a); + // INSERT the row, then a deferred UPDATE with its own id + morph type + $this->assertNumWrites(2); + + // consecutive save does nothing + $this->captureWriteQueries(); + $this->save($a); + $this->assertNumWrites(0); + + $this->assertSame($a->id, $a->parentId); + $this->assertSame('entity_a', $a->parentType); + + $row = $this->getDatabase()->table('entity_a')->select()->where('id', $a->id)->fetchAll(); + $this->assertSame((string) $a->id, (string) $row[0]['parent_id']); + $this->assertSame('entity_a', $row[0]['parent_type']); + + $this->orm = $this->orm->withHeap(new Heap()); + $reloaded = $this->orm->getRepository(EntityA::class)->findByPK($a->id); + $this->assertInstanceOf(EntityA::class, $reloaded->parent); + $this->assertSame($reloaded->id, $reloaded->parent->id); + } + + public function testCreateCycleTwoEntities(): void + { + $a = new EntityA(); + $a->name = 'cycle-a'; + + $b = new EntityB(); + $b->name = 'cycle-b'; + + $a->parent = $b; + $b->parent = $a; + + $this->captureWriteQueries(); + $this->save($a); + // INSERT a, INSERT b, then a deferred UPDATE to close the cycle + $this->assertNumWrites(3); + + // consecutive save does nothing + $this->captureWriteQueries(); + $this->save($a); + $this->assertNumWrites(0); + + $this->assertSame($b->id, $a->parentId); + $this->assertSame('entity_b', $a->parentType); + $this->assertSame($a->id, $b->parentId); + $this->assertSame('entity_a', $b->parentType); + + $this->orm = $this->orm->withHeap(new Heap()); + + $reloadedA = $this->orm->getRepository(EntityA::class)->findByPK($a->id); + $this->assertInstanceOf(EntityB::class, $reloadedA->parent); + $this->assertSame($b->id, $reloadedA->parent->id); + $this->assertInstanceOf(EntityA::class, $reloadedA->parent->parent); + $this->assertSame($a->id, $reloadedA->parent->parent->id); + } + + /** + * With ignoreUninitializedRelations = true (BaseTest default) unsetting the relation property + * must leave both the outer key and the morph key untouched. + */ + public function testUnsetParentKeepsMorphWhenIgnoringUninitialized(): void + { + $a = $this->orm->getRepository(EntityA::class)->findByPK(1); + $this->assertInstanceOf(EntityA::class, $a->parent); + unset($a->parent); + + $this->captureWriteQueries(); + $this->save($a); + $this->assertNumWrites(0); + + $row = $this->getDatabase()->table('entity_a')->select()->where('id', 1)->fetchAll(); + $this->assertSame('1', (string) $row[0]['parent_id']); + $this->assertSame('entity_a', $row[0]['parent_type']); + } + + /** + * With ignoreUninitializedRelations = false an unset relation is treated as null, so both the + * outer key and the morph key must be cleared. + */ + public function testUnsetParentClearsMorphWithoutIgnoreUninitialized(): void + { + $this->orm = $this->orm->with(options: (new Options())->withIgnoreUninitializedRelations(false)); + + $a = $this->orm->getRepository(EntityA::class)->findByPK(1); + $this->assertInstanceOf(EntityA::class, $a->parent); + unset($a->parent); + + $this->captureWriteQueries(); + $this->save($a); + $this->assertNumWrites(1); + + $row = $this->getDatabase()->table('entity_a')->select()->where('id', 1)->fetchAll(); + $this->assertNull($row[0]['parent_id'], 'parent_id should be NULL when the unset relation is treated as null'); + $this->assertNull($row[0]['parent_type'], 'parent_type should be NULL when the unset relation is treated as null'); + } + + public function setUp(): void + { + parent::setUp(); + + $this->makeTable('entity_a', [ + 'id' => 'primary', + 'name' => 'string', + 'parent_id' => 'integer,nullable', + 'parent_type' => 'string,nullable', + ]); + + $this->makeTable('entity_b', [ + 'id' => 'primary', + 'name' => 'string', + 'parent_id' => 'integer,nullable', + 'parent_type' => 'string,nullable', + ]); + + // entity_a #1 references itself; entity_a #2 -> entity_b #1 + $this->getDatabase()->table('entity_a')->insertMultiple( + ['name', 'parent_id', 'parent_type'], + [ + ['a-self', 1, 'entity_a'], + ['a-cycle', 1, 'entity_b'], + ], + ); + + // entity_b #1 -> entity_a #2 (closes the A > B > A cycle) + $this->getDatabase()->table('entity_b')->insertMultiple( + ['name', 'parent_id', 'parent_type'], + [ + ['b-1', 2, 'entity_a'], + ], + ); + + $this->orm = $this->withSchema(new Schema([ + EntityA::class => [ + Schema::ROLE => 'entity_a', + Schema::MAPPER => Mapper::class, + Schema::DATABASE => 'default', + Schema::TABLE => 'entity_a', + Schema::PRIMARY_KEY => 'id', + Schema::COLUMNS => [ + 'id' => 'id', + 'name' => 'name', + 'parentId' => 'parent_id', + 'parentType' => 'parent_type', + ], + Schema::SCHEMA => [], + Schema::RELATIONS => [ + 'parent' => $this->morphedParent(), + ], + ], + EntityB::class => [ + Schema::ROLE => 'entity_b', + Schema::MAPPER => Mapper::class, + Schema::DATABASE => 'default', + Schema::TABLE => 'entity_b', + Schema::PRIMARY_KEY => 'id', + Schema::COLUMNS => [ + 'id' => 'id', + 'name' => 'name', + 'parentId' => 'parent_id', + 'parentType' => 'parent_type', + ], + Schema::SCHEMA => [], + Schema::RELATIONS => [ + 'parent' => $this->morphedParent(), + ], + ], + ])); + } + + private function morphedParent(): array + { + return [ + Relation::TYPE => Relation::REFERS_TO_MORPHED, + Relation::TARGET => MorphedInterface::class, + Relation::LOAD => Relation::LOAD_PROMISE, + Relation::SCHEMA => [ + Relation::NULLABLE => true, + Relation::CASCADE => true, + Relation::OUTER_KEY => 'id', + Relation::INNER_KEY => 'parentId', + Relation::MORPH_KEY => 'parentType', + ], + ]; + } +} diff --git a/tests/ORM/Functional/Driver/MySQL/Relation/Morphed/RefersToMorphedCyclicTest.php b/tests/ORM/Functional/Driver/MySQL/Relation/Morphed/RefersToMorphedCyclicTest.php new file mode 100644 index 000000000..f1eae2139 --- /dev/null +++ b/tests/ORM/Functional/Driver/MySQL/Relation/Morphed/RefersToMorphedCyclicTest.php @@ -0,0 +1,17 @@ + Date: Mon, 15 Jun 2026 19:25:13 +0400 Subject: [PATCH 2/3] Add lazy-loading assertions for RefersToMorphed cycles Verify the promise reference resolves with the expected number of read queries: a self-reference is taken from the heap (0 queries), a parent that is not yet loaded costs exactly one query, and the back-reference across an A > B > A cycle is served from the heap. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Morphed/RefersToMorphedCyclicTest.php | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php b/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php index d846d00c8..e481cdc20 100644 --- a/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php +++ b/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php @@ -55,6 +55,42 @@ public function testFetchCycleTwoEntities(): void $this->assertSame($a->id, $a->parent->parent->id); } + /** + * The relation is loaded lazily: a self-reference resolves to the already-loaded entity + * straight from the heap, without issuing an extra query. + */ + public function testLazyLoadSelfReferenceResolvesFromHeap(): void + { + $a = $this->orm->getRepository(EntityA::class)->findByPK(1); + // Not resolved yet — the relation is a promise reference. + $this->assertInstanceOf(ReferenceInterface::class, $this->extractEntity($a)['parent']); + + $this->captureReadQueries(); + $this->assertSame($a, $a->parent); + $this->assertNumReads(0); + } + + /** + * Lazy loading across a morphed cycle: resolving the parent that is not in the heap costs + * exactly one query, while the back-reference is taken from the heap for free. + */ + public function testLazyLoadCycleTwoEntities(): void + { + $a = $this->orm->getRepository(EntityA::class)->findByPK(2); + $this->assertInstanceOf(ReferenceInterface::class, $this->extractEntity($a)['parent']); + + // entity_b #1 is not in the heap yet -> exactly one lazy query. + $this->captureReadQueries(); + $b = $a->parent; + $this->assertInstanceOf(EntityB::class, $b); + $this->assertNumReads(1); + + // The back-reference entity_a #2 is already in the heap -> no extra query. + $this->captureReadQueries(); + $this->assertSame($a, $b->parent); + $this->assertNumReads(0); + } + public function testSetSelfReferenceToNull(): void { $a = $this->orm->getRepository(EntityA::class)->findByPK(1); From 12afd12cecb8fb28a9aed88e50d215eb576b5c9d Mon Sep 17 00:00:00 2001 From: roxblnfk Date: Mon, 15 Jun 2026 19:29:01 +0400 Subject: [PATCH 3/3] Add eager-loading and BulkLoader tests for RefersToMorphed Cover one-level eager loading via Select::load() and batch loading via BulkLoader for the morphed refers-to relation (reusing BelongsToMorphedLoader). Both resolve the parent up front, so accessing it afterwards issues no queries. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../Morphed/RefersToMorphedCyclicTest.php | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php b/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php index e481cdc20..3a0fa591e 100644 --- a/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php +++ b/tests/ORM/Functional/Driver/Common/Relation/Morphed/RefersToMorphedCyclicTest.php @@ -10,6 +10,7 @@ use Cycle\ORM\Reference\ReferenceInterface; use Cycle\ORM\Relation; use Cycle\ORM\Schema; +use Cycle\ORM\Select; use Cycle\ORM\Tests\Functional\Driver\Common\BaseTest; use Cycle\ORM\Tests\Fixtures\MorphedCyclic\EntityA; use Cycle\ORM\Tests\Fixtures\MorphedCyclic\EntityB; @@ -91,6 +92,48 @@ public function testLazyLoadCycleTwoEntities(): void $this->assertNumReads(0); } + /** + * Eager loading via Select::load(): one level of the morphed relation is resolved up front + * (one query per distinct morph role), so accessing the parent afterwards costs no queries. + */ + public function testEagerLoadParent(): void + { + /** @var list $all */ + $all = (new Select($this->orm, EntityA::class)) + ->load('parent') + ->orderBy('entity_a.id') + ->fetchAll(); + + $this->captureReadQueries(); + // #1 references itself, #2 references entity_b #1 — both already loaded. + $this->assertInstanceOf(EntityA::class, $all[0]->parent); + $this->assertSame($all[0], $all[0]->parent); + $this->assertInstanceOf(EntityB::class, $all[1]->parent); + $this->assertSame('b-1', $all[1]->parent->name); + $this->assertNumReads(0); + } + + /** + * The morphed relation can be eagerly loaded for a batch of already-fetched entities through + * the BulkLoader. After that, accessing the parent issues no extra queries. + */ + public function testBulkLoadParent(): void + { + $this->captureReadQueries(); + /** @var list $all */ + $all = (new Select($this->orm, EntityA::class))->orderBy('entity_a.id')->fetchAll(); + $this->assertNumReads(1); + + $this->bulkLoader(...$all)->load('parent')->run(); + + $this->captureReadQueries(); + $this->assertInstanceOf(EntityA::class, $all[0]->parent); + $this->assertSame($all[0], $all[0]->parent); + $this->assertInstanceOf(EntityB::class, $all[1]->parent); + $this->assertSame('b-1', $all[1]->parent->name); + $this->assertNumReads(0); + } + public function testSetSelfReferenceToNull(): void { $a = $this->orm->getRepository(EntityA::class)->findByPK(1);