Skip to content

Commit 61bd2da

Browse files
committed
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
1 parent f0f0ba4 commit 61bd2da

16 files changed

Lines changed: 268 additions & 77 deletions

src/Db.php

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,10 @@ private function executeStatement(
9393
int|string|array|callable|object $object = stdClass::class,
9494
array|null $extra = null,
9595
): PDOStatement {
96-
$statement = $this->prepare((string) $this->currentSql, $object, $extra);
97-
$statement->execute($this->currentSql->params);
96+
$sql = $this->currentSql;
9897
$this->currentSql = clone $this->protoSql;
98+
$statement = $this->prepare((string) $sql, $object, $extra);
99+
$statement->execute($sql->params);
99100

100101
return $statement;
101102
}

src/Mapper.php

Lines changed: 81 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ public function flush(): void
108108
}
109109
} catch (Throwable $e) {
110110
$conn->rollback();
111+
$this->reset();
111112

112113
throw $e;
113114
}
@@ -157,6 +158,14 @@ private function flushSingle(object $entity): void
157158
}
158159

159160
/**
161+
* Extract composed columns from parent, UPDATE child tables using FK relationship.
162+
*
163+
* For existing entities (UPDATE path), the parent PK is known and we can
164+
* update the child table directly: UPDATE comment SET text=? WHERE post_id=?
165+
*
166+
* For new entities (INSERT path), child inserts happen after the parent
167+
* via insertCompositionChildren().
168+
*
160169
* @param array<string, mixed> $cols
161170
*
162171
* @return array<string, mixed>
@@ -167,30 +176,75 @@ private function extractAndOperateCompositions(Collection $collection, array $co
167176
return $cols;
168177
}
169178

179+
$parentPk = $this->style->identifier($collection->name);
180+
$parentPkValue = $cols[$parentPk] ?? null;
181+
$fkToParent = $this->style->remoteIdentifier($collection->name);
182+
170183
foreach ($collection->compositions as $comp => $spec) {
171184
$compCols = [];
172185
foreach ($spec as $key) {
173-
if (!isset($cols[$key])) {
186+
$dbKey = $this->style->realProperty($key);
187+
if (!isset($cols[$dbKey])) {
174188
continue;
175189
}
176190

177-
$compCols[$key] = $cols[$key];
178-
unset($cols[$key]);
191+
$compCols[$dbKey] = $cols[$dbKey];
192+
unset($cols[$dbKey]);
179193
}
180194

181-
if (isset($cols[$comp . '_id'])) {
182-
$compCols['id'] = $cols[$comp . '_id'];
183-
unset($cols[$comp . '_id']);
184-
$this->rawUpdate($compCols, $this->__get($comp));
185-
} else {
186-
$compCols['id'] = null;
187-
$this->rawInsert($compCols, $this->__get($comp));
195+
if ($parentPkValue === null || empty($compCols)) {
196+
continue;
188197
}
198+
199+
$this->db
200+
->update($comp)
201+
->set($compCols)
202+
->where([[$fkToParent, '=', $parentPkValue]])
203+
->exec();
189204
}
190205

191206
return $cols;
192207
}
193208

209+
private function insertCompositionChildren(Collection $collection, object|null $entity): void
210+
{
211+
if (!$collection instanceof Composite || $entity === null) {
212+
return;
213+
}
214+
215+
$parentPk = $this->style->identifier($collection->name);
216+
$parentPkValue = $this->entityFactory->get($entity, $parentPk);
217+
218+
if ($parentPkValue === null) {
219+
return;
220+
}
221+
222+
$fkToParent = $this->style->remoteIdentifier($collection->name);
223+
$entityCols = $this->entityFactory->extractColumns($entity);
224+
225+
foreach ($collection->compositions as $comp => $spec) {
226+
$compCols = [];
227+
foreach ($spec as $key) {
228+
$dbKey = $this->style->realProperty($key);
229+
if (!isset($entityCols[$key])) {
230+
continue;
231+
}
232+
233+
$compCols[$dbKey] = $entityCols[$key];
234+
}
235+
236+
if (empty($compCols)) {
237+
continue;
238+
}
239+
240+
$compCols[$fkToParent] = $parentPkValue;
241+
$this->db
242+
->insertInto($comp, array_keys($compCols))
243+
->values(array_values($compCols))
244+
->exec();
245+
}
246+
}
247+
194248
/**
195249
* @param array<string, mixed> $columns
196250
*
@@ -249,6 +303,8 @@ private function rawInsert(
249303
$this->checkNewIdentity($entity, $collection);
250304
}
251305

306+
$this->insertCompositionChildren($collection, $entity);
307+
252308
return $result;
253309
}
254310

@@ -264,7 +320,7 @@ private function checkNewIdentity(object $entity, Collection $collection): bool
264320
return false;
265321
}
266322

267-
$this->entityFactory->set($entity, $this->style->identifier($collection->name), $identity);
323+
$this->entityFactory->set($entity, $this->style->identifier($collection->name), (int) $identity);
268324

269325
return true;
270326
}
@@ -286,10 +342,17 @@ private function generateQuery(Collection $collection): Sql
286342
/** @return array<string, mixed> */
287343
private function extractColumns(object $entity, Collection $collection): array
288344
{
289-
return $this->filterColumns(
345+
$cols = $this->filterColumns(
290346
$this->entityFactory->extractColumns($entity),
291347
$collection,
292348
);
349+
350+
$dbCols = [];
351+
foreach ($cols as $key => $value) {
352+
$dbCols[$this->style->realProperty($key)] = $value;
353+
}
354+
355+
return $dbCols;
293356
}
294357

295358
/** @param array<string, Collection> $collections */
@@ -302,10 +365,6 @@ private function buildSelectStatement(Sql $sql, array $collections): Sql
302365
foreach ($columns as $col) {
303366
$selectTable[] = $tableSpecifier . '_comp' . $composition . '.' . $col;
304367
}
305-
306-
$selectTable[] = $tableSpecifier . '_comp' . $composition . '.' .
307-
$this->style->identifier($composition) .
308-
' as ' . $composition . '_id';
309368
}
310369
}
311370

@@ -393,8 +452,13 @@ private function parseCompositions(Sql $sql, Collection $collection, string $ent
393452
}
394453

395454
foreach (array_keys($collection->compositions) as $comp) {
455+
$alias = $entity . '_comp' . $comp;
396456
$sql->innerJoin($comp);
397-
$sql->as($entity . '_comp' . $comp);
457+
$sql->as($alias);
458+
$sql->on([
459+
$alias . '.' . $this->style->remoteIdentifier($entity)
460+
=> $entity . '.' . $this->style->identifier($entity),
461+
]);
398462
}
399463
}
400464

tests/MapperTest.php

Lines changed: 135 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
use PDOStatement;
1313
use PHPUnit\Framework\Attributes\CoversClass;
1414
use PHPUnit\Framework\TestCase;
15+
use ReflectionProperty;
1516
use Respect\Data\Collections\Composite;
1617
use Respect\Data\Collections\Filtered;
1718
use Respect\Data\Collections\Typed;
@@ -176,6 +177,31 @@ public function testRollingBackTransaction(): void
176177
}
177178
}
178179

180+
public function testFailedFlushResetsPending(): void
181+
{
182+
// Force a flush failure via a UNIQUE constraint violation
183+
$this->conn->exec('CREATE UNIQUE INDEX author_name_unique ON author(name)');
184+
185+
$dupe = new Author();
186+
$dupe->name = 'Author 1'; // already seeded
187+
$this->mapper->author->persist($dupe);
188+
189+
try {
190+
$this->mapper->flush();
191+
$this->fail('Expected flush to throw on UNIQUE violation');
192+
} catch (Throwable) {
193+
// expected
194+
}
195+
196+
// Second flush with a valid entity should succeed without replaying the failed one
197+
$author = new Author();
198+
$author->name = 'Fresh Author';
199+
$this->mapper->author->persist($author);
200+
$this->mapper->flush();
201+
202+
$this->assertGreaterThan(0, $author->id);
203+
}
204+
179205
public function testIgnoringLastInsertIdErrors(): void
180206
{
181207
$conn = $this->createStub(PDO::class);
@@ -197,7 +223,7 @@ public function testIgnoringLastInsertIdErrors(): void
197223
$obj->name = 'bar';
198224
$mapper->author->persist($obj);
199225
$mapper->flush();
200-
$this->assertNull($obj->id);
226+
$this->assertFalse((new ReflectionProperty($obj, 'id'))->isInitialized($obj));
201227
$this->assertEquals('bar', $obj->name);
202228
}
203229

@@ -377,10 +403,12 @@ public function testNestedPersistCollectionWithChildrenShortcut(): void
377403
public function testSubCategory(): void
378404
{
379405
$mapper = $this->mapper;
406+
$parent = $mapper->category[2]->fetch();
407+
380408
$entity = new Category();
381409
$entity->id = 8;
382410
$entity->name = 'inserted';
383-
$entity->category_id = 2;
411+
$entity->category = $parent;
384412
$mapper->category->persist($entity);
385413
$mapper->flush();
386414
$result = $this->query('select * from category where id=8')
@@ -395,10 +423,12 @@ public function testSubCategory(): void
395423
public function testSubCategoryCondition(): void
396424
{
397425
$mapper = $this->mapper;
426+
$parent = $mapper->category[2]->fetch();
427+
398428
$entity = new Category();
399429
$entity->id = 8;
400430
$entity->name = 'inserted';
401-
$entity->category_id = 2;
431+
$entity->category = $parent;
402432
$mapper->category->persist($entity);
403433
$mapper->flush();
404434
$result = $this->query('select * from category where id=8')
@@ -866,6 +896,18 @@ public function testCompositesPersistDoesNotDropColumnsWithMatchingValues(): voi
866896
$this->assertEquals('Same Value', $result->text);
867897
}
868898

899+
public function testCompositeColumnOverridesParentOnNameCollision(): void
900+
{
901+
$mapper = $this->mapper;
902+
$mapper->postComment = Composite::post(['comment' => ['text']])->author();
903+
$post = $mapper->postComment->fetch();
904+
905+
// Both post and comment have a 'text' column.
906+
// The composite column (comment.text) should take precedence.
907+
$this->assertEquals('Comment Text', $post->text);
908+
$this->assertNotEquals('Post Text', $post->text);
909+
}
910+
869911
public function testTyped(): void
870912
{
871913
$mapper = new Mapper($this->conn, new EntityFactory(entityNamespace: '\Respect\Relational\\'));
@@ -1027,7 +1069,7 @@ public function testPersistNewEntityWithNoAutoIncrementId(): void
10271069
$obj->name = 'test';
10281070
$mapper->author->persist($obj);
10291071
$mapper->flush();
1030-
$this->assertNull($obj->id);
1072+
$this->assertFalse((new ReflectionProperty($obj, 'id'))->isInitialized($obj));
10311073
}
10321074

10331075
public function testFetchReturnsDbInstance(): void
@@ -1120,7 +1162,7 @@ public function testInsertedEntityIsRetrievableFromIdentityMap(): void
11201162
$this->mapper->flush();
11211163

11221164
// The entity should now have an auto-assigned id and be cached
1123-
$this->assertNotNull($entity->id);
1165+
$this->assertGreaterThan(0, $entity->id);
11241166

11251167
$fetched = $this->mapper->post($entity->id)->fetch();
11261168
$this->assertSame($entity, $fetched);
@@ -1233,6 +1275,94 @@ public function testPersistPureEntityTreeDerivesForeignKey(): void
12331275
$this->assertEquals(1, $row['author_id']);
12341276
}
12351277

1278+
public function testPersistWithUninitializedRelationSkipsCascade(): void
1279+
{
1280+
// Post has `Author $author` (uninitialized). Persist should not
1281+
// crash — it should skip the cascade for the missing relation.
1282+
$mapper = $this->mapper;
1283+
$post = new Post();
1284+
$post->title = 'No Author';
1285+
$post->text = 'Body';
1286+
1287+
$mapper->post->persist($post);
1288+
$mapper->flush();
1289+
1290+
$this->assertGreaterThan(0, $post->id);
1291+
$result = $this->query('select title from post where id=' . $post->id)
1292+
->fetch(PDO::FETCH_OBJ);
1293+
$this->assertEquals('No Author', $result->title);
1294+
}
1295+
1296+
public function testCompositeUpdateSkipsMissingSpecColumn(): void
1297+
{
1298+
// Composite spec asks for 'text' from comment, but we only change
1299+
// 'title' (a post column). The composite should not crash on the
1300+
// missing spec column — it should just skip it.
1301+
$mapper = $this->mapper;
1302+
$mapper->postComment = Composite::post(['comment' => ['text']])->author();
1303+
$post = $mapper->postComment->fetch();
1304+
1305+
// Only change a parent column, leave composite column unchanged
1306+
$post->title = 'Only Title Changed';
1307+
1308+
$mapper->postComment->persist($post);
1309+
$mapper->flush();
1310+
1311+
$result = $this->query('select title from post where id=5')
1312+
->fetch(PDO::FETCH_OBJ);
1313+
$this->assertEquals('Only Title Changed', $result->title);
1314+
// Comment text should remain untouched
1315+
$result = $this->query('select text from comment where id=7')
1316+
->fetch(PDO::FETCH_OBJ);
1317+
$this->assertEquals('Comment Text', $result->text);
1318+
}
1319+
1320+
public function testCompositeInsertWithNoMatchingColumnsSkipsChild(): void
1321+
{
1322+
// New entity where the composite spec columns are NOT set — the
1323+
// child INSERT should be skipped entirely (no empty INSERT).
1324+
$mapper = $this->mapper;
1325+
$mapper->postComment = Composite::post(['comment' => ['text']])->author();
1326+
1327+
$post = new Postcomment();
1328+
$post->title = 'Post Without Comment';
1329+
$author = new Author();
1330+
$author->name = 'Author X';
1331+
$post->author = $author;
1332+
// Note: $post->text is NOT set (uninitialized)
1333+
1334+
$mapper->postComment->persist($post);
1335+
$mapper->flush();
1336+
1337+
$result = $this->query('select title from post order by id desc')
1338+
->fetch(PDO::FETCH_OBJ);
1339+
$this->assertEquals('Post Without Comment', $result->title);
1340+
}
1341+
1342+
public function testFetchWithArrayConditions(): void
1343+
{
1344+
// Test multiple array conditions (hits the AND branch in parseConditions)
1345+
$result = $this->mapper->post[['title' => 'Post Title', 'author_id' => 1]]->fetchAll();
1346+
$this->assertCount(1, $result);
1347+
$this->assertEquals('Post Title', $result[0]->title);
1348+
}
1349+
1350+
public function testPersistCascadeSkipsNullChildRelation(): void
1351+
{
1352+
// Register a collection with children: post → author (child).
1353+
// Persist a post where $author is uninitialized.
1354+
// The cascade should skip the null child without crashing (L87-91).
1355+
$mapper = $this->mapper;
1356+
$this->expectNotToPerformAssertions();
1357+
$mapper->postsFromAuthorsWithComments->persist(new class {
1358+
public int $id;
1359+
1360+
public string $title = 'Orphan Post';
1361+
1362+
public string $text = '';
1363+
});
1364+
}
1365+
12361366
private function query(string $sql): PDOStatement
12371367
{
12381368
$stmt = $this->conn->query($sql);

0 commit comments

Comments
 (0)