From 686d92a2484a7bca9fc7cf1fe603f1e7d9f9ba3e Mon Sep 17 00:00:00 2001 From: Alexandre Gomes Gaigalas Date: Thu, 26 Mar 2026 17:30:41 -0300 Subject: [PATCH] Adopt pure entity trees - Mapper::flush() resets pending queue on failure, preventing cascading errors across requests in long-running runtimes (Swoole, ReactPHP) - Db::executeStatement() resets SQL state before execute, not after, so failures don't corrupt subsequent queries - extractAndOperateCompositions() converts spec keys via Style::realProperty() to match DB column names after extractColumns() conversion - Composite joins now emit explicit ON clauses instead of relying on implicit FK inference - Composite entities no longer carry child PKs (commentId removed) - checkNewIdentity() casts lastInsertId to int for typed properties - Removed unused $parentCols parameter from insertCompositionChildren() - All test stubs use typed properties and pure entity tree pattern --- src/Db.php | 5 +- src/Mapper.php | 98 +++++++++++++++---- tests/MapperTest.php | 140 +++++++++++++++++++++++++++- tests/Stubs/Author.php | 4 +- tests/Stubs/Bug.php | 2 +- tests/Stubs/Category.php | 6 +- tests/Stubs/Comment.php | 6 +- tests/Stubs/Improvement.php | 2 +- tests/Stubs/Issue.php | 2 +- tests/Stubs/Issues.php | 2 +- tests/Stubs/OtherEntity/Author.php | 12 +-- tests/Stubs/OtherEntity/Comment.php | 8 +- tests/Stubs/OtherEntity/Post.php | 34 +++---- tests/Stubs/Post.php | 10 +- tests/Stubs/PostCategory.php | 6 +- tests/Stubs/Postcomment.php | 8 +- 16 files changed, 268 insertions(+), 77 deletions(-) diff --git a/src/Db.php b/src/Db.php index 2538ab8..aa1ae6e 100644 --- a/src/Db.php +++ b/src/Db.php @@ -93,9 +93,10 @@ private function executeStatement( int|string|array|callable|object $object = stdClass::class, array|null $extra = null, ): PDOStatement { - $statement = $this->prepare((string) $this->currentSql, $object, $extra); - $statement->execute($this->currentSql->params); + $sql = $this->currentSql; $this->currentSql = clone $this->protoSql; + $statement = $this->prepare((string) $sql, $object, $extra); + $statement->execute($sql->params); return $statement; } diff --git a/src/Mapper.php b/src/Mapper.php index e317384..0d8f6a2 100644 --- a/src/Mapper.php +++ b/src/Mapper.php @@ -108,6 +108,7 @@ public function flush(): void } } catch (Throwable $e) { $conn->rollback(); + $this->reset(); throw $e; } @@ -157,6 +158,14 @@ private function flushSingle(object $entity): void } /** + * Extract composed columns from parent, UPDATE child tables using FK relationship. + * + * For existing entities (UPDATE path), the parent PK is known and we can + * update the child table directly: UPDATE comment SET text=? WHERE post_id=? + * + * For new entities (INSERT path), child inserts happen after the parent + * via insertCompositionChildren(). + * * @param array $cols * * @return array @@ -167,30 +176,75 @@ private function extractAndOperateCompositions(Collection $collection, array $co return $cols; } + $parentPk = $this->style->identifier($collection->name); + $parentPkValue = $cols[$parentPk] ?? null; + $fkToParent = $this->style->remoteIdentifier($collection->name); + foreach ($collection->compositions as $comp => $spec) { $compCols = []; foreach ($spec as $key) { - if (!isset($cols[$key])) { + $dbKey = $this->style->realProperty($key); + if (!isset($cols[$dbKey])) { continue; } - $compCols[$key] = $cols[$key]; - unset($cols[$key]); + $compCols[$dbKey] = $cols[$dbKey]; + unset($cols[$dbKey]); } - if (isset($cols[$comp . '_id'])) { - $compCols['id'] = $cols[$comp . '_id']; - unset($cols[$comp . '_id']); - $this->rawUpdate($compCols, $this->__get($comp)); - } else { - $compCols['id'] = null; - $this->rawInsert($compCols, $this->__get($comp)); + if ($parentPkValue === null || empty($compCols)) { + continue; } + + $this->db + ->update($comp) + ->set($compCols) + ->where([[$fkToParent, '=', $parentPkValue]]) + ->exec(); } return $cols; } + private function insertCompositionChildren(Collection $collection, object|null $entity): void + { + if (!$collection instanceof Composite || $entity === null) { + return; + } + + $parentPk = $this->style->identifier($collection->name); + $parentPkValue = $this->entityFactory->get($entity, $parentPk); + + if ($parentPkValue === null) { + return; + } + + $fkToParent = $this->style->remoteIdentifier($collection->name); + $entityCols = $this->entityFactory->extractColumns($entity); + + foreach ($collection->compositions as $comp => $spec) { + $compCols = []; + foreach ($spec as $key) { + $dbKey = $this->style->realProperty($key); + if (!isset($entityCols[$key])) { + continue; + } + + $compCols[$dbKey] = $entityCols[$key]; + } + + if (empty($compCols)) { + continue; + } + + $compCols[$fkToParent] = $parentPkValue; + $this->db + ->insertInto($comp, array_keys($compCols)) + ->values(array_values($compCols)) + ->exec(); + } + } + /** * @param array $columns * @@ -249,6 +303,8 @@ private function rawInsert( $this->checkNewIdentity($entity, $collection); } + $this->insertCompositionChildren($collection, $entity); + return $result; } @@ -264,7 +320,7 @@ private function checkNewIdentity(object $entity, Collection $collection): bool return false; } - $this->entityFactory->set($entity, $this->style->identifier($collection->name), $identity); + $this->entityFactory->set($entity, $this->style->identifier($collection->name), (int) $identity); return true; } @@ -286,10 +342,17 @@ private function generateQuery(Collection $collection): Sql /** @return array */ private function extractColumns(object $entity, Collection $collection): array { - return $this->filterColumns( + $cols = $this->filterColumns( $this->entityFactory->extractColumns($entity), $collection, ); + + $dbCols = []; + foreach ($cols as $key => $value) { + $dbCols[$this->style->realProperty($key)] = $value; + } + + return $dbCols; } /** @param array $collections */ @@ -302,10 +365,6 @@ private function buildSelectStatement(Sql $sql, array $collections): Sql foreach ($columns as $col) { $selectTable[] = $tableSpecifier . '_comp' . $composition . '.' . $col; } - - $selectTable[] = $tableSpecifier . '_comp' . $composition . '.' . - $this->style->identifier($composition) . - ' as ' . $composition . '_id'; } } @@ -393,8 +452,13 @@ private function parseCompositions(Sql $sql, Collection $collection, string $ent } foreach (array_keys($collection->compositions) as $comp) { + $alias = $entity . '_comp' . $comp; $sql->innerJoin($comp); - $sql->as($entity . '_comp' . $comp); + $sql->as($alias); + $sql->on([ + $alias . '.' . $this->style->remoteIdentifier($entity) + => $entity . '.' . $this->style->identifier($entity), + ]); } } diff --git a/tests/MapperTest.php b/tests/MapperTest.php index 68afd15..97803fb 100644 --- a/tests/MapperTest.php +++ b/tests/MapperTest.php @@ -12,6 +12,7 @@ use PDOStatement; use PHPUnit\Framework\Attributes\CoversClass; use PHPUnit\Framework\TestCase; +use ReflectionProperty; use Respect\Data\Collections\Composite; use Respect\Data\Collections\Filtered; use Respect\Data\Collections\Typed; @@ -176,6 +177,31 @@ public function testRollingBackTransaction(): void } } + public function testFailedFlushResetsPending(): void + { + // Force a flush failure via a UNIQUE constraint violation + $this->conn->exec('CREATE UNIQUE INDEX author_name_unique ON author(name)'); + + $dupe = new Author(); + $dupe->name = 'Author 1'; // already seeded + $this->mapper->author->persist($dupe); + + try { + $this->mapper->flush(); + $this->fail('Expected flush to throw on UNIQUE violation'); + } catch (Throwable) { + // expected + } + + // Second flush with a valid entity should succeed without replaying the failed one + $author = new Author(); + $author->name = 'Fresh Author'; + $this->mapper->author->persist($author); + $this->mapper->flush(); + + $this->assertGreaterThan(0, $author->id); + } + public function testIgnoringLastInsertIdErrors(): void { $conn = $this->createStub(PDO::class); @@ -197,7 +223,7 @@ public function testIgnoringLastInsertIdErrors(): void $obj->name = 'bar'; $mapper->author->persist($obj); $mapper->flush(); - $this->assertNull($obj->id); + $this->assertFalse((new ReflectionProperty($obj, 'id'))->isInitialized($obj)); $this->assertEquals('bar', $obj->name); } @@ -377,10 +403,12 @@ public function testNestedPersistCollectionWithChildrenShortcut(): void public function testSubCategory(): void { $mapper = $this->mapper; + $parent = $mapper->category[2]->fetch(); + $entity = new Category(); $entity->id = 8; $entity->name = 'inserted'; - $entity->category_id = 2; + $entity->category = $parent; $mapper->category->persist($entity); $mapper->flush(); $result = $this->query('select * from category where id=8') @@ -395,10 +423,12 @@ public function testSubCategory(): void public function testSubCategoryCondition(): void { $mapper = $this->mapper; + $parent = $mapper->category[2]->fetch(); + $entity = new Category(); $entity->id = 8; $entity->name = 'inserted'; - $entity->category_id = 2; + $entity->category = $parent; $mapper->category->persist($entity); $mapper->flush(); $result = $this->query('select * from category where id=8') @@ -866,6 +896,18 @@ public function testCompositesPersistDoesNotDropColumnsWithMatchingValues(): voi $this->assertEquals('Same Value', $result->text); } + public function testCompositeColumnOverridesParentOnNameCollision(): void + { + $mapper = $this->mapper; + $mapper->postComment = Composite::post(['comment' => ['text']])->author(); + $post = $mapper->postComment->fetch(); + + // Both post and comment have a 'text' column. + // The composite column (comment.text) should take precedence. + $this->assertEquals('Comment Text', $post->text); + $this->assertNotEquals('Post Text', $post->text); + } + public function testTyped(): void { $mapper = new Mapper($this->conn, new EntityFactory(entityNamespace: '\Respect\Relational\\')); @@ -1027,7 +1069,7 @@ public function testPersistNewEntityWithNoAutoIncrementId(): void $obj->name = 'test'; $mapper->author->persist($obj); $mapper->flush(); - $this->assertNull($obj->id); + $this->assertFalse((new ReflectionProperty($obj, 'id'))->isInitialized($obj)); } public function testFetchReturnsDbInstance(): void @@ -1120,7 +1162,7 @@ public function testInsertedEntityIsRetrievableFromIdentityMap(): void $this->mapper->flush(); // The entity should now have an auto-assigned id and be cached - $this->assertNotNull($entity->id); + $this->assertGreaterThan(0, $entity->id); $fetched = $this->mapper->post($entity->id)->fetch(); $this->assertSame($entity, $fetched); @@ -1233,6 +1275,94 @@ public function testPersistPureEntityTreeDerivesForeignKey(): void $this->assertEquals(1, $row['author_id']); } + public function testPersistWithUninitializedRelationSkipsCascade(): void + { + // Post has `Author $author` (uninitialized). Persist should not + // crash — it should skip the cascade for the missing relation. + $mapper = $this->mapper; + $post = new Post(); + $post->title = 'No Author'; + $post->text = 'Body'; + + $mapper->post->persist($post); + $mapper->flush(); + + $this->assertGreaterThan(0, $post->id); + $result = $this->query('select title from post where id=' . $post->id) + ->fetch(PDO::FETCH_OBJ); + $this->assertEquals('No Author', $result->title); + } + + public function testCompositeUpdateSkipsMissingSpecColumn(): void + { + // Composite spec asks for 'text' from comment, but we only change + // 'title' (a post column). The composite should not crash on the + // missing spec column — it should just skip it. + $mapper = $this->mapper; + $mapper->postComment = Composite::post(['comment' => ['text']])->author(); + $post = $mapper->postComment->fetch(); + + // Only change a parent column, leave composite column unchanged + $post->title = 'Only Title Changed'; + + $mapper->postComment->persist($post); + $mapper->flush(); + + $result = $this->query('select title from post where id=5') + ->fetch(PDO::FETCH_OBJ); + $this->assertEquals('Only Title Changed', $result->title); + // Comment text should remain untouched + $result = $this->query('select text from comment where id=7') + ->fetch(PDO::FETCH_OBJ); + $this->assertEquals('Comment Text', $result->text); + } + + public function testCompositeInsertWithNoMatchingColumnsSkipsChild(): void + { + // New entity where the composite spec columns are NOT set — the + // child INSERT should be skipped entirely (no empty INSERT). + $mapper = $this->mapper; + $mapper->postComment = Composite::post(['comment' => ['text']])->author(); + + $post = new Postcomment(); + $post->title = 'Post Without Comment'; + $author = new Author(); + $author->name = 'Author X'; + $post->author = $author; + // Note: $post->text is NOT set (uninitialized) + + $mapper->postComment->persist($post); + $mapper->flush(); + + $result = $this->query('select title from post order by id desc') + ->fetch(PDO::FETCH_OBJ); + $this->assertEquals('Post Without Comment', $result->title); + } + + public function testFetchWithArrayConditions(): void + { + // Test multiple array conditions (hits the AND branch in parseConditions) + $result = $this->mapper->post[['title' => 'Post Title', 'author_id' => 1]]->fetchAll(); + $this->assertCount(1, $result); + $this->assertEquals('Post Title', $result[0]->title); + } + + public function testPersistCascadeSkipsNullChildRelation(): void + { + // Register a collection with children: post → author (child). + // Persist a post where $author is uninitialized. + // The cascade should skip the null child without crashing (L87-91). + $mapper = $this->mapper; + $this->expectNotToPerformAssertions(); + $mapper->postsFromAuthorsWithComments->persist(new class { + public int $id; + + public string $title = 'Orphan Post'; + + public string $text = ''; + }); + } + private function query(string $sql): PDOStatement { $stmt = $this->conn->query($sql); diff --git a/tests/Stubs/Author.php b/tests/Stubs/Author.php index e862581..ffd6c49 100644 --- a/tests/Stubs/Author.php +++ b/tests/Stubs/Author.php @@ -6,7 +6,7 @@ class Author { - public mixed $id = null; + public int $id; - public string|null $name = null; + public string $name; } diff --git a/tests/Stubs/Bug.php b/tests/Stubs/Bug.php index a9d6f3c..de0f984 100644 --- a/tests/Stubs/Bug.php +++ b/tests/Stubs/Bug.php @@ -6,7 +6,7 @@ class Bug { - public int|null $id = null; + public int $id; public string|null $title = null; diff --git a/tests/Stubs/Category.php b/tests/Stubs/Category.php index d1998c0..a6bf1a3 100644 --- a/tests/Stubs/Category.php +++ b/tests/Stubs/Category.php @@ -6,9 +6,9 @@ class Category { - public mixed $id = null; + public int $id; - public string|null $name = null; + public string $name; - public mixed $category_id = null; + public Category $category; } diff --git a/tests/Stubs/Comment.php b/tests/Stubs/Comment.php index 4baa854..edd35f3 100644 --- a/tests/Stubs/Comment.php +++ b/tests/Stubs/Comment.php @@ -8,11 +8,11 @@ class Comment { - public mixed $id = null; + public int $id; - public mixed $post; + public Post $post; - public string|null $text = null; + public string $text; private string|null $datetime = null; diff --git a/tests/Stubs/Improvement.php b/tests/Stubs/Improvement.php index 10f5978..32a48d8 100644 --- a/tests/Stubs/Improvement.php +++ b/tests/Stubs/Improvement.php @@ -6,7 +6,7 @@ class Improvement { - public int|null $id = null; + public int $id; public string|null $title = null; diff --git a/tests/Stubs/Issue.php b/tests/Stubs/Issue.php index f197d79..a3a9eec 100644 --- a/tests/Stubs/Issue.php +++ b/tests/Stubs/Issue.php @@ -6,7 +6,7 @@ class Issue { - public mixed $id = null; + public int $id; public string|null $type = null; diff --git a/tests/Stubs/Issues.php b/tests/Stubs/Issues.php index ae81b05..bd84464 100644 --- a/tests/Stubs/Issues.php +++ b/tests/Stubs/Issues.php @@ -6,7 +6,7 @@ class Issues { - public mixed $id = null; + public int $id; public string|null $type = null; diff --git a/tests/Stubs/OtherEntity/Author.php b/tests/Stubs/OtherEntity/Author.php index c1f9999..879d173 100644 --- a/tests/Stubs/OtherEntity/Author.php +++ b/tests/Stubs/OtherEntity/Author.php @@ -6,26 +6,26 @@ class Author { - private mixed $id = null; + private int $id; - private mixed $name = null; + private string $name; - public function getId(): mixed + public function getId(): int|null { return $this->id; } - public function getName(): mixed + public function getName(): string { return $this->name; } - public function setId(mixed $id): void + public function setId(int|null $id): void { $this->id = $id; } - public function setName(mixed $name): void + public function setName(string $name): void { $this->name = $name; } diff --git a/tests/Stubs/OtherEntity/Comment.php b/tests/Stubs/OtherEntity/Comment.php index 5d96334..7340d5e 100644 --- a/tests/Stubs/OtherEntity/Comment.php +++ b/tests/Stubs/OtherEntity/Comment.php @@ -8,13 +8,13 @@ class Comment { - public int|null $id = null; + public int $id; - public int|null $post_id = null; + public Post $post; - public string|null $text = null; + public string $text; - public string|null $datetime = null; + public string $datetime; public function __construct() { diff --git a/tests/Stubs/OtherEntity/Post.php b/tests/Stubs/OtherEntity/Post.php index 4ba5bc7..3f9cae7 100644 --- a/tests/Stubs/OtherEntity/Post.php +++ b/tests/Stubs/OtherEntity/Post.php @@ -4,62 +4,52 @@ namespace Respect\Relational\OtherEntity; -use Respect\Data\NotPersistable; - class Post { - private mixed $id = null; - - private mixed $author_id = null; + private int $id; - #[NotPersistable] - private mixed $author = null; + private Author $author; - private mixed $title = null; + private string $title; - private mixed $text = null; + private string $text; - public function getTitle(): mixed + public function getTitle(): string { return $this->title; } - public function setTitle(mixed $title): void + public function setTitle(string $title): void { $this->title = $title; } - public function getId(): mixed + public function getId(): int|null { return $this->id; } - public function getAuthorId(): mixed - { - return $this->author_id; - } - - public function getAuthor(): mixed + public function getAuthor(): Author { return $this->author; } - public function getText(): mixed + public function getText(): string { return $this->text; } - public function setId(mixed $id): void + public function setId(int|null $id): void { $this->id = $id; } public function setAuthor(Author $author): void { - $this->author_id = $author; + $this->author = $author; } - public function setText(mixed $text): void + public function setText(string $text): void { $this->text = $text; } diff --git a/tests/Stubs/Post.php b/tests/Stubs/Post.php index 363c499..c3bc02a 100644 --- a/tests/Stubs/Post.php +++ b/tests/Stubs/Post.php @@ -9,15 +9,15 @@ class Post { - public mixed $id = null; + public int $id; - public mixed $author; + public Author $author; - public string|null $text = null; + public string $text; - public string|null $title = null; + public string $title; - public mixed $comment_id; + public Comment $comment; #[NotPersistable] private string $datetime = ''; diff --git a/tests/Stubs/PostCategory.php b/tests/Stubs/PostCategory.php index 5a72e33..40e61ae 100644 --- a/tests/Stubs/PostCategory.php +++ b/tests/Stubs/PostCategory.php @@ -6,9 +6,9 @@ class PostCategory { - public mixed $id = null; + public int $id; - public mixed $post_id = null; + public Post $post; - public mixed $category_id = null; + public Category $category; } diff --git a/tests/Stubs/Postcomment.php b/tests/Stubs/Postcomment.php index 812825f..601eeff 100644 --- a/tests/Stubs/Postcomment.php +++ b/tests/Stubs/Postcomment.php @@ -6,5 +6,11 @@ class Postcomment { - public int|null $id = null; + public int $id; + + public string $title; + + public string $text; + + public Author $author; }