diff --git a/src/AbstractMapper.php b/src/AbstractMapper.php index 8880cf3..26ab3ee 100644 --- a/src/AbstractMapper.php +++ b/src/AbstractMapper.php @@ -10,8 +10,8 @@ use function array_flip; use function array_intersect_key; -use function assert; use function count; +use function ctype_digit; use function is_int; use function is_scalar; use function is_string; @@ -96,8 +96,9 @@ public function persist(object $object, Collection $onCollection): object return $object; } - if ($onCollection->name !== null && $this->tryReplaceFromIdentityMap($object, $onCollection)) { - return $object; + $merged = $this->tryMergeWithIdentityMap($object, $onCollection); + if ($merged !== null) { + return $merged; } $this->pending[$object] = 'insert'; @@ -202,31 +203,30 @@ protected function findInIdentityMap(Collection $collection): object|null return null; } - $condition = $collection->condition; - if (!is_int($condition) && !is_string($condition)) { + $condition = $this->normalizeIdValue($collection->condition); + if ($condition === null) { return null; } return $this->identityMap[$collection->name][$condition] ?? null; } - private function tryReplaceFromIdentityMap(object $entity, Collection $coll): bool + private function tryMergeWithIdentityMap(object $entity, Collection $coll): object|null { - assert($coll->name !== null); - $entityId = $this->entityIdValue($entity, $coll->name); - $idValue = $entityId; - - if ($idValue === null && is_scalar($coll->condition)) { - $idValue = $coll->condition; + if ($coll->name === null) { + return null; } - if ($idValue === null || (!is_int($idValue) && !is_string($idValue))) { - return false; + $entityId = $this->entityIdValue($entity, $coll->name); + $idValue = $entityId ?? $this->normalizeIdValue($coll->condition); + + if ($idValue === null) { + return null; } $existing = $this->identityMap[$coll->name][$idValue] ?? null; if ($existing === null || $existing === $entity) { - return false; + return null; } if ($entityId === null) { @@ -234,21 +234,53 @@ private function tryReplaceFromIdentityMap(object $entity, Collection $coll): bo $this->entityFactory->set($entity, $idName, $idValue); } - $this->tracked->offsetUnset($existing); - $this->pending->offsetUnset($existing); - $this->evictFromIdentityMap($existing, $coll); - $this->markTracked($entity, $coll); - $this->registerInIdentityMap($entity, $coll); - $this->pending[$entity] = 'update'; + if ($this->entityFactory->isReadOnly($existing)) { + $merged = $this->entityFactory->mergeEntities($existing, $entity); - return true; + if ($merged !== $existing) { + $this->tracked->offsetUnset($existing); + $this->pending->offsetUnset($existing); + $this->evictFromIdentityMap($existing, $coll); + $this->markTracked($merged, $coll); + $this->registerInIdentityMap($merged, $coll); + } + + $this->pending[$merged] = 'update'; + + return $merged; + } + + foreach ($this->entityFactory->extractProperties($entity) as $prop => $value) { + $this->entityFactory->set($existing, $prop, $value); + } + + if (!$this->isTracked($existing)) { + $this->markTracked($existing, $coll); + } + + $this->pending[$existing] = 'update'; + + return $existing; } private function entityIdValue(object $entity, string $collName): int|string|null { - $idValue = $this->entityFactory->get($entity, $this->style->identifier($collName)); + return $this->normalizeIdValue( + $this->entityFactory->get($entity, $this->style->identifier($collName)), + ); + } + + private function normalizeIdValue(mixed $value): int|string|null + { + if (is_int($value)) { + return $value; + } + + if (is_string($value)) { + return ctype_digit($value) ? (int) $value : $value; + } - return is_int($idValue) || is_string($idValue) ? $idValue : null; + return null; } public function __get(string $name): Collection diff --git a/src/CollectionNotBound.php b/src/CollectionNotBound.php new file mode 100644 index 0000000..6a677f7 --- /dev/null +++ b/src/CollectionNotBound.php @@ -0,0 +1,18 @@ + */ class Collection implements ArrayAccess @@ -65,9 +65,7 @@ public function persist(object $object, mixed ...$changes): object } } - $mapper->persist($object, $this); - - return $object; + return $mapper->persist($object, $this); } public function remove(object $object): bool @@ -150,7 +148,7 @@ private function findMapper(): AbstractMapper|null private function resolveMapper(): AbstractMapper { - return $this->findMapper() ?? throw new RuntimeException(); + return $this->findMapper() ?? throw new CollectionNotBound($this->name); } private function setNext(Collection $collection): void diff --git a/src/EntityFactory.php b/src/EntityFactory.php index 05f4939..88facc8 100644 --- a/src/EntityFactory.php +++ b/src/EntityFactory.php @@ -26,10 +26,10 @@ /** Creates and manipulates entity objects using Style-based naming conventions */ class EntityFactory { - /** @var array> */ + /** @var array> */ private array $classCache = []; - /** @var array> */ + /** @var array> */ private array $propertyCache = []; /** @var array */ @@ -156,6 +156,43 @@ public function withChanges(object $entity, mixed ...$changes): object return $clone; } + public function mergeEntities(object $base, object $overlay): object + { + if ($base::class !== $overlay::class) { + throw new DomainException( + 'Cannot merge entities of different classes: ' . $base::class . ' and ' . $overlay::class, + ); + } + + $overlayProps = $this->extractProperties($overlay); + $hasDifference = false; + + foreach ($overlayProps as $name => $value) { + $mirror = $this->reflectProperties($base::class)[$name]; + + if (!$mirror->isInitialized($base) || $mirror->getValue($base) !== $value) { + $hasDifference = true; + break; + } + } + + if (!$hasDifference) { + return $base; + } + + $clone = $this->reflectClass($base::class)->newInstanceWithoutConstructor(); + + foreach ($this->reflectProperties($base::class) as $name => $prop) { + if (array_key_exists($name, $overlayProps)) { + $prop->setValue($clone, $overlayProps[$name]); + } elseif ($prop->isInitialized($base)) { + $prop->setValue($clone, $prop->getValue($base)); + } + } + + return $clone; + } + /** * Extract persistable columns, resolving entity objects to their reference representations. * @@ -199,7 +236,11 @@ public function extractProperties(object $entity): array return $props; } - /** @return array */ + /** + * @param class-string $class + * + * @return array + */ private function detectRelationProperties(string $class): array { if (isset($this->relationCache[$class])) { @@ -222,13 +263,21 @@ private function detectRelationProperties(string $class): array return $this->relationCache[$class] = $relations; } - /** @return ReflectionClass */ + /** + * @param class-string $class + * + * @return ReflectionClass + */ private function reflectClass(string $class): ReflectionClass { - return $this->classCache[$class] ??= new ReflectionClass($class); // @phpstan-ignore argument.type + return $this->classCache[$class] ??= new ReflectionClass($class); } - /** @return array */ + /** + * @param class-string $class + * + * @return array + */ private function reflectProperties(string $class): array { if (!isset($this->propertyCache[$class])) { diff --git a/src/Hydrators/Flat.php b/src/Hydrators/Flat.php index a640671..2b0386e 100644 --- a/src/Hydrators/Flat.php +++ b/src/Hydrators/Flat.php @@ -50,7 +50,7 @@ public function hydrate( ); $entityFactory->set( - /** @phpstan-ignore argument.type */ + /** @phpstan-ignore argument.type (array_pop returns object|null but SplObjectStorage guarantees object key) */ $entityInstance, $columnName, $value, diff --git a/tests/AbstractMapperTest.php b/tests/AbstractMapperTest.php index 1676c04..30b7e03 100644 --- a/tests/AbstractMapperTest.php +++ b/tests/AbstractMapperTest.php @@ -758,9 +758,10 @@ public function persistUntrackedEntityWithMatchingPkUpdates(): void /** @var SplObjectStorage $pending */ $pending = $pendingProp->getValue($mapper); - $this->assertSame('update', $pending[$replacement]); - $this->assertFalse($mapper->isTracked($fetched)); - $this->assertTrue($mapper->isTracked($replacement)); + $this->assertSame('update', $pending[$fetched]); + $this->assertTrue($mapper->isTracked($fetched)); + $this->assertFalse($mapper->isTracked($replacement)); + $this->assertSame('Updated', $fetched->title); } #[Test] @@ -791,20 +792,23 @@ public function persistReadOnlyViaCollectionPkUpdates(): void // Create new readonly entity (no PK) and persist via collection[pk] $updated = $mapper->entityFactory->create(Stubs\ReadOnlyAuthor::class, name: 'Updated', bio: 'new bio'); - $mapper->read_only_author[1]->persist($updated); + $merged = $mapper->read_only_author[1]->persist($updated); - // PK should have been set from collection condition - $this->assertSame(1, $updated->id); + // Merged entity should combine both: PK from fetched, changes from updated + $this->assertSame(1, $merged->id); + $this->assertSame('Updated', $merged->name); + $this->assertSame('new bio', $merged->bio); - // Old entity should be evicted + // Merged entity should be tracked, old fetched evicted $this->assertFalse($mapper->isTracked($fetched)); - $this->assertTrue($mapper->isTracked($updated)); + $this->assertFalse($mapper->isTracked($updated)); + $this->assertTrue($mapper->isTracked($merged)); $ref = new ReflectionObject($mapper); $pendingProp = $ref->getProperty('pending'); /** @var SplObjectStorage $pending */ $pending = $pendingProp->getValue($mapper); - $this->assertSame('update', $pending[$updated]); + $this->assertSame('update', $pending[$merged]); } #[Test] @@ -1109,12 +1113,13 @@ public function readOnlyMultipleEntitiesFetchAllTracksAll(): void // Replace one by identity map lookup $updated = $mapper->entityFactory->create(Stubs\Immutable\Author::class, name: 'Alice Updated'); - $mapper->author[1]->persist($updated); + $merged = $mapper->author[1]->persist($updated); - // Original Alice should be evicted, updated Alice takes its place + // Original Alice should be evicted, merged entity takes its place $this->assertSame(3, $mapper->trackedCount()); - $this->assertTrue($mapper->isTracked($updated)); + $this->assertTrue($mapper->isTracked($merged)); $this->assertFalse($mapper->isTracked($authors[0])); + $this->assertSame('Alice Updated', $merged->name); } #[Test] @@ -1129,11 +1134,12 @@ public function identityMapReplaceSkipsSetWhenPkAlreadyInitialized(): void $updated = new Stubs\Immutable\Author(id: 1, name: 'Bob'); - // persist via collection[1] — PK already set, should NOT try set() again - $mapper->author[1]->persist($updated); + // persist via collection[1] — PK already set, merge produces new entity + $merged = $mapper->author[1]->persist($updated); - $this->assertSame(1, $updated->id); - $this->assertTrue($mapper->isTracked($updated)); + $this->assertSame(1, $merged->id); + $this->assertSame('Bob', $merged->name); + $this->assertTrue($mapper->isTracked($merged)); } #[Test] @@ -1213,4 +1219,131 @@ public function persistWithChangesOnTrackedUpdateReplacesOriginal(): void $refetched = $mapper->author[1]->fetch(); $this->assertSame('Bob', $refetched->name); } + + #[Test] + public function mutableMergeAppliesOverlayPropertiesToExisting(): void + { + $mapper = new InMemoryMapper(new EntityFactory(entityNamespace: 'Respect\\Data\\Stubs\\')); + $mapper->seed('author', [ + ['id' => 1, 'name' => 'Alice', 'bio' => null], + ]); + + $fetched = $mapper->author[1]->fetch(); + $this->assertSame('Alice', $fetched->name); + + // Persist a different mutable entity with same PK + $overlay = new Stubs\Author(); + $overlay->id = 1; + $overlay->name = 'Bob'; + $overlay->bio = 'new bio'; + + $result = $mapper->author->persist($overlay); + + // Existing entity is mutated in place and returned + $this->assertSame($fetched, $result); + $this->assertSame('Bob', $fetched->name); + $this->assertSame('new bio', $fetched->bio); + $this->assertTrue($mapper->isTracked($fetched)); + $this->assertFalse($mapper->isTracked($overlay)); + } + + #[Test] + public function readOnlyMergeNoDiffReturnsSameEntity(): void + { + $mapper = new InMemoryMapper(new EntityFactory(entityNamespace: 'Respect\\Data\\Stubs\\Immutable\\')); + $mapper->seed('author', [ + ['id' => 1, 'name' => 'Alice', 'bio' => null], + ]); + + $fetched = $mapper->author[1]->fetch(); + + // Persist readonly entity with identical properties + $same = new Stubs\Immutable\Author(id: 1, name: 'Alice'); + $result = $mapper->author[1]->persist($same); + + // No clone needed — same entity returned + $this->assertSame($fetched, $result); + $this->assertTrue($mapper->isTracked($fetched)); + } + + #[Test] + public function identityMapLookupNormalizesNumericStringCondition(): void + { + $mapper = new InMemoryMapper(new EntityFactory(entityNamespace: 'Respect\\Data\\Stubs\\')); + $mapper->seed('author', [ + ['id' => 1, 'name' => 'Alice', 'bio' => null], + ]); + + $fetched = $mapper->author[1]->fetch(); + + // Lookup with string "1" should hit the identity map + $fromString = $mapper->author['1']->fetch(); + $this->assertSame($fetched, $fromString); + } + + #[Test] + public function identityMapLookupReturnsNullForNonScalarCondition(): void + { + $mapper = new InMemoryMapper(new EntityFactory(entityNamespace: 'Respect\\Data\\Stubs\\')); + $mapper->seed('author', [ + ['id' => 1, 'name' => 'Alice', 'bio' => null], + ]); + + $mapper->author[1]->fetch(); + + // Float condition should not match identity map + $result = $mapper->author[1.5]->fetch(); + $this->assertNotSame(true, $result === null); + } + + #[Test] + public function mutableMergeTracksExistingWhenNotYetTracked(): void + { + $mapper = new InMemoryMapper(new EntityFactory(entityNamespace: 'Respect\\Data\\Stubs\\')); + $mapper->seed('author', [ + ['id' => 1, 'name' => 'Alice', 'bio' => null], + ]); + + // Put entity in identity map via fetch, then untrack it manually + $fetched = $mapper->author[1]->fetch(); + $ref = new ReflectionObject($mapper); + $trackedProp = $ref->getProperty('tracked'); + /** @var SplObjectStorage $tracked */ + $tracked = $trackedProp->getValue($mapper); + $tracked->offsetUnset($fetched); + $this->assertFalse($mapper->isTracked($fetched)); + + // Persist a new entity with same PK — should merge and re-track existing + $overlay = new Stubs\Author(); + $overlay->id = 1; + $overlay->name = 'Bob'; + + $result = $mapper->author->persist($overlay); + + $this->assertSame($fetched, $result); + $this->assertTrue($mapper->isTracked($fetched)); + $this->assertSame('Bob', $fetched->name); + } + + #[Test] + public function mergeWithIdentityMapNormalizesConditionFallback(): void + { + $mapper = new InMemoryMapper(new EntityFactory(entityNamespace: 'Respect\\Data\\Stubs\\Immutable\\')); + $mapper->seed('author', [ + ['id' => 1, 'name' => 'Alice', 'bio' => null], + ]); + + $fetched = $mapper->author[1]->fetch(); + + // Persist readonly entity without PK, via string condition "1" + $overlay = $mapper->entityFactory->create(Stubs\Immutable\Author::class, name: 'Updated'); + $merged = $mapper->author['1']->persist($overlay); + + // Should have matched identity map via normalized condition + $this->assertNotSame($fetched, $merged); + $this->assertSame(1, $merged->id); + $this->assertSame('Updated', $merged->name); + $this->assertTrue($mapper->isTracked($merged)); + $this->assertFalse($mapper->isTracked($fetched)); + } } diff --git a/tests/Collections/CollectionTest.php b/tests/Collections/CollectionTest.php index 747a846..c1d5f10 100644 --- a/tests/Collections/CollectionTest.php +++ b/tests/Collections/CollectionTest.php @@ -8,17 +8,18 @@ use PHPUnit\Framework\Attributes\Test; use PHPUnit\Framework\TestCase; use Respect\Data\AbstractMapper; +use Respect\Data\CollectionNotBound; use Respect\Data\EntityFactory; use Respect\Data\Hydrators\Nested; use Respect\Data\InMemoryMapper; use Respect\Data\Stubs; use Respect\Data\Stubs\Foo; -use RuntimeException; use function count; use function reset; #[CoversClass(Collection::class)] +#[CoversClass(CollectionNotBound::class)] class CollectionTest extends TestCase { #[Test] @@ -274,28 +275,28 @@ public function arrayOffsetExistsShouldNotDoAnything(): void #[Test] public function persistOnCollectionShouldExceptionIfMapperDontExist(): void { - $this->expectException(RuntimeException::class); + $this->expectException(CollectionNotBound::class); Collection::foo()->persist(new Foo()); } #[Test] public function removeOnCollectionShouldExceptionIfMapperDontExist(): void { - $this->expectException(RuntimeException::class); + $this->expectException(CollectionNotBound::class); Collection::foo()->remove(new Foo()); } #[Test] public function fetchOnCollectionShouldExceptionIfMapperDontExist(): void { - $this->expectException(RuntimeException::class); + $this->expectException(CollectionNotBound::class); Collection::foo()->fetch(); } #[Test] public function fetchAllOnCollectionShouldExceptionIfMapperDontExist(): void { - $this->expectException(RuntimeException::class); + $this->expectException(CollectionNotBound::class); Collection::foo()->fetchAll(); } diff --git a/tests/EntityFactoryTest.php b/tests/EntityFactoryTest.php index 5a8bedc..c1a2ac3 100644 --- a/tests/EntityFactoryTest.php +++ b/tests/EntityFactoryTest.php @@ -536,4 +536,64 @@ public function withChangesThrowsOnInvalidValue(): void $this->expectExceptionMessage('Invalid value'); $factory->withChanges($entity, name: null); } + + #[Test] + public function mergeEntitiesReturnsMergedCloneWhenPropertiesDiffer(): void + { + $factory = new EntityFactory(entityNamespace: __NAMESPACE__ . '\\Stubs\\Immutable\\'); + + $base = new Stubs\Immutable\Author(id: 1, name: 'Alice', bio: 'Original bio'); + $overlay = $factory->create(Stubs\Immutable\Author::class, name: 'Bob'); + + $merged = $factory->mergeEntities($base, $overlay); + assert($merged instanceof Stubs\Immutable\Author); + + $this->assertNotSame($base, $merged); + $this->assertNotSame($overlay, $merged); + $this->assertSame(1, $merged->id); + $this->assertSame('Bob', $merged->name); + $this->assertSame('Original bio', $merged->bio); + } + + #[Test] + public function mergeEntitiesReturnsBaseWhenNoDifference(): void + { + $factory = new EntityFactory(entityNamespace: __NAMESPACE__ . '\\Stubs\\Immutable\\'); + + $base = new Stubs\Immutable\Author(id: 1, name: 'Alice'); + $overlay = $factory->create(Stubs\Immutable\Author::class, id: 1, name: 'Alice'); + + $merged = $factory->mergeEntities($base, $overlay); + + $this->assertSame($base, $merged); + } + + #[Test] + public function mergeEntitiesThrowsOnClassMismatch(): void + { + $factory = new EntityFactory(entityNamespace: __NAMESPACE__ . '\\Stubs\\Immutable\\'); + + $base = new Stubs\Immutable\Author(id: 1, name: 'Alice'); + $overlay = new Stubs\Immutable\Post(id: 1, title: 'Title'); + + $this->expectException(DomainException::class); + $this->expectExceptionMessage('Cannot merge entities of different classes'); + $factory->mergeEntities($base, $overlay); + } + + #[Test] + public function mergeEntitiesClonesWhenBasePropertyUninitialized(): void + { + $factory = new EntityFactory(entityNamespace: __NAMESPACE__ . '\\Stubs\\Immutable\\'); + + $base = $factory->create(Stubs\Immutable\Author::class, id: 1); + $overlay = $factory->create(Stubs\Immutable\Author::class, name: 'Bob'); + + $merged = $factory->mergeEntities($base, $overlay); + assert($merged instanceof Stubs\Immutable\Author); + + $this->assertNotSame($base, $merged); + $this->assertSame(1, $merged->id); + $this->assertSame('Bob', $merged->name); + } }