diff --git a/src/Object/Hydrator.php b/src/Object/Hydrator.php index 006ae5ecc..6824d808c 100644 --- a/src/Object/Hydrator.php +++ b/src/Object/Hydrator.php @@ -51,13 +51,13 @@ public function __invoke(object $object, array $parameters): object continue; } - if (true === $this->forceProperties || \in_array($parameter, $this->forceProperties, true) || $value instanceof ForceValue) { + if ($this->shouldForceProperty($parameter, $value)) { if ($value instanceof ForceValue) { $value = $value->value; } try { - self::set($object, $parameter, $value); + self::forceSet($object, $parameter, $value); } catch (\InvalidArgumentException $e) { if (true !== $this->extraAttributes) { throw $e; @@ -67,10 +67,8 @@ public function __invoke(object $object, array $parameters): object continue; } - self::$accessor ??= new PropertyAccessor(); - try { - self::$accessor->setValue($object, $parameter, $value); + $this->propertyAccessor()->setValue($object, $parameter, $value); } catch (NoSuchPropertyException $e) { if (true !== $this->extraAttributes) { throw new \InvalidArgumentException(\sprintf('Cannot set attribute "%s" for object "%s" (not public and no setter).', $parameter, $object::class), previous: $e); @@ -97,7 +95,22 @@ public function alwaysForce(string ...$properties): self return $clone; } - public static function set(object $object, string $property, mixed $value, bool $catchErrors = false): void + public function setProperty(object $object, string $property, mixed $value, bool $catchErrors = false): void + { + if (!$this->shouldForceProperty($property, $value)) { + try { + $this->propertyAccessor()->setValue($object, $property, $value); + + return; + } catch (\Throwable) { + } + } + + self::forceSet($object, $property, $value, $catchErrors); + } + + // todo: rename + public static function forceSet(object $object, string $property, mixed $value, bool $catchErrors = false): void { $value = ForceValue::unwrap($value); @@ -117,7 +130,7 @@ public static function set(object $object, string $property, mixed $value, bool } } - public static function add(object $object, string $property, mixed $value): void + public function add(object $object, string $property, mixed $value): void { $inverseValue = self::get($object, $property); @@ -132,8 +145,12 @@ public static function add(object $object, string $property, mixed $value): void return; } + if ($inverseValue instanceof \Traversable) { + $inverseValue = \iterator_to_array($inverseValue); + } + $inverseValue[] = $value; - self::set($object, $property, $inverseValue, catchErrors: true); + $this->setProperty($object, $property, $inverseValue); } public static function get(object $object, string $property): mixed @@ -160,7 +177,7 @@ public static function hydrateFromOtherObject(object $object, object $other): vo } foreach ($properties as $property) { - self::set($object, $property, self::get($other, $property), catchErrors: true); + self::forceSet($object, $property, self::get($other, $property), catchErrors: true); } } @@ -213,4 +230,14 @@ private static function isDoctrineCollection(object $object, string $property): return false; } + + private function shouldForceProperty(int|string $parameter, mixed $value): bool + { + return true === $this->forceProperties || \in_array($parameter, $this->forceProperties, true) || $value instanceof ForceValue; + } + + private function propertyAccessor(): PropertyAccessor + { + return self::$accessor ??= new PropertyAccessor(); + } } diff --git a/src/Object/Instantiator.php b/src/Object/Instantiator.php index 64704cf99..30c0e0e10 100644 --- a/src/Object/Instantiator.php +++ b/src/Object/Instantiator.php @@ -111,6 +111,14 @@ public function disableHydration(): self return $clone; } + /** + * @internal + */ + public function hydrator(): Hydrator + { + return $this->hydrator; + } + /** * @template T of object * diff --git a/src/ObjectFactory.php b/src/ObjectFactory.php index ca8114a8d..6ab947271 100644 --- a/src/ObjectFactory.php +++ b/src/ObjectFactory.php @@ -13,6 +13,7 @@ use Zenstruck\Foundry\Object\Event\AfterInstantiate; use Zenstruck\Foundry\Object\Event\BeforeInstantiate; +use Zenstruck\Foundry\Object\Hydrator; use Zenstruck\Foundry\Object\Instantiator; use Zenstruck\Foundry\Persistence\ProxyGenerator; @@ -90,7 +91,21 @@ final public function instantiateWith(callable $instantiator): static */ final public function instantiator(): callable { - return $this->instantiator ?? Configuration::instance()->instantiator; + return $this->instantiator ??= Configuration::instance()->instantiator; + } + + /** + * @internal + */ + final protected function hydrator(): Hydrator + { + $instantiator = $this->instantiator(); + + if (!$instantiator instanceof Instantiator) { + return new Hydrator(); + } + + return $instantiator->hydrator(); } /** diff --git a/src/Persistence/IsProxy.php b/src/Persistence/IsProxy.php index 5d06eebef..534f32883 100644 --- a/src/Persistence/IsProxy.php +++ b/src/Persistence/IsProxy.php @@ -93,7 +93,7 @@ public function _set(string $property, mixed $value): static { $this->_autoRefresh(); - Hydrator::set($this->initializeLazyObject(), $property, $value); + Hydrator::forceSet($this->initializeLazyObject(), $property, $value); return $this; } diff --git a/src/Persistence/PersistentObjectFactory.php b/src/Persistence/PersistentObjectFactory.php index 1a423fe8b..c4cbab479 100644 --- a/src/Persistence/PersistentObjectFactory.php +++ b/src/Persistence/PersistentObjectFactory.php @@ -453,12 +453,16 @@ protected function normalizeCollection(string $field, FactoryCollection $collect \array_map(static fn($o) => get($o, $inverseRelationshipMetadata->collectionIndexedBy), $inverseObjects), \array_values($inverseObjects) ); - } - set($object, $field, $inverseObjects); + // using forceSet to prevent usage of PropertyAccessor, + // which will potentially call adders and lose index information + Hydrator::forceSet($object, $field, $inverseObjects); + } else { + $this->hydrator()->setProperty($object, $field, $inverseObjects); + } }; - // creation delegated to tempAfterInstantiate hook - return empty array here + // creation delegated to tempAfterInstantiate hook - return an empty array here return []; } @@ -466,7 +470,7 @@ protected function normalizeCollection(string $field, FactoryCollection $collect } /** - * This method will try to find entities in database if they are detached. + * This method will try to find entities in the database if they are detached. * * @internal */ @@ -489,14 +493,14 @@ protected function normalizeObject(string $field, object $object): object $inverseRelationship = $persistenceManager->bidirectionalRelationshipMetadata(static::class(), $object::class, $field); if ($inverseRelationship instanceof OneToOneRelationship) { - $this->inverseRelationshipCallbacks[] = static function(object $newObject) use ($object, $inverseRelationship) { - Hydrator::set($object, $inverseRelationship->inverseField(), $newObject, catchErrors: true); + $this->inverseRelationshipCallbacks[] = function(object $newObject) use ($object, $inverseRelationship) { + $this->hydrator()->setProperty($object, $inverseRelationship->inverseField(), $newObject, catchErrors: true); }; } if ($inverseRelationship instanceof ManyToOneRelationship) { - $this->inverseRelationshipCallbacks[] = static function(object $newObject) use ($object, $inverseRelationship) { - Hydrator::add($object, $inverseRelationship->inverseField(), $newObject); + $this->inverseRelationshipCallbacks[] = function (object $newObject) use ($object, $inverseRelationship) { + $this->hydrator()->add($object, $inverseRelationship->inverseField(), $newObject); }; } diff --git a/src/functions.php b/src/functions.php index aa8d56221..cf4e6121a 100644 --- a/src/functions.php +++ b/src/functions.php @@ -59,7 +59,7 @@ function object(string $class, array|callable $attributes = []): object */ function set(object $object, string $property, mixed $value): object { - Hydrator::set($object, $property, $value); + Hydrator::forceSet($object, $property, $value); return $object; } diff --git a/tests/Fixture/Entity/EdgeCases/OneToManyWithLifecycleCallback/ChildEntity.php b/tests/Fixture/Entity/EdgeCases/OneToManyWithLifecycleCallback/ChildEntity.php new file mode 100644 index 000000000..6084f810e --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/OneToManyWithLifecycleCallback/ChildEntity.php @@ -0,0 +1,28 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\OneToManyWithLifecycleCallback; + +use Doctrine\ORM\Mapping as ORM; +use Zenstruck\Foundry\Tests\Fixture\Model\Base; + +#[ORM\Entity] +#[ORM\Table('one_to_many_lifecycle_child')] +class ChildEntity extends Base +{ + #[ORM\ManyToOne(inversedBy: 'children')] + public ?ParentEntity $parent = null; + + #[ORM\Column(nullable: true)] + public ?string $name = null; +} diff --git a/tests/Fixture/Entity/EdgeCases/OneToManyWithLifecycleCallback/ParentEntity.php b/tests/Fixture/Entity/EdgeCases/OneToManyWithLifecycleCallback/ParentEntity.php new file mode 100644 index 000000000..26184b78e --- /dev/null +++ b/tests/Fixture/Entity/EdgeCases/OneToManyWithLifecycleCallback/ParentEntity.php @@ -0,0 +1,64 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\OneToManyWithLifecycleCallback; + +use Doctrine\Common\Collections\ArrayCollection; +use Doctrine\Common\Collections\Collection; +use Doctrine\ORM\Mapping as ORM; +use Zenstruck\Foundry\Tests\Fixture\Model\Base; + +#[ORM\Entity] +#[ORM\Table('one_to_many_lifecycle_parent')] +#[ORM\HasLifecycleCallbacks] +class ParentEntity extends Base +{ + /** @var Collection */ + #[ORM\OneToMany(targetEntity: ChildEntity::class, mappedBy: 'parent', cascade: ['persist', 'remove'])] + private Collection $children; + + #[ORM\Column()] + public int $childrenCount = 0; + + public function __construct() + { + $this->children = new ArrayCollection(); + } + + #[ORM\PrePersist] + public function computeChildrenCount(): void + { + $this->childrenCount = $this->children->count(); + } + + public function addChild(ChildEntity $child): void + { + if (!$this->children->contains($child)) { + $this->children->add($child); + $child->parent = $this; + } + } + + public function removeChild(ChildEntity $child): void + { + $this->children->removeElement($child); + } + + /** + * @return Collection + */ + public function getChildren(): Collection + { + return $this->children; + } +} diff --git a/tests/Integration/ORM/EdgeCasesRelationshipTest.php b/tests/Integration/ORM/EdgeCasesRelationshipTest.php index 3990ee4b9..827ae0b90 100644 --- a/tests/Integration/ORM/EdgeCasesRelationshipTest.php +++ b/tests/Integration/ORM/EdgeCasesRelationshipTest.php @@ -34,6 +34,7 @@ use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\InversedOneToOneWithSetter; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneToSelfReferencing; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\ManyToOneWithAutoGeneratedUlid; +use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\OneToManyWithLifecycleCallback; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\OneToManyWithUnionType; use Zenstruck\Foundry\Tests\Fixture\Entity\EdgeCases\RichDomainMandatoryRelationship; use Zenstruck\Foundry\Tests\Fixture\Factories\Entity\EdgeCases\MultipleMandatoryRelationshipToSameEntity; @@ -369,6 +370,24 @@ static function(EntityWithLifecycleCallback $e) { self::assertSame('pre-persist - after instantiate', $entity->prop1); } + /** @test */ + #[Test] + public function pre_persist_can_access_populated_one_to_many_collection(): void + { + $parentFactory = persistent_factory(OneToManyWithLifecycleCallback\ParentEntity::class); + $childFactory = persistent_factory(OneToManyWithLifecycleCallback\ChildEntity::class); + + $parent = $parentFactory->create([ + 'children' => $childFactory->with(fn(int $i) => ['name' => "child $i"])->many(3), + ]); + + $parentFactory::assert()->count(1); + $childFactory::assert()->count(3); + + self::assertSame(3, $parent->childrenCount); + self::assertCount(3, $parent->getChildren()); + } + /** * @test */ diff --git a/tests/Unit/Object/HydratorTest.php b/tests/Unit/Object/HydratorTest.php index 6715ef8a7..84d76fd1d 100644 --- a/tests/Unit/Object/HydratorTest.php +++ b/tests/Unit/Object/HydratorTest.php @@ -37,7 +37,7 @@ public function can_hydrate_scalar(): void public string $foo = ''; }; - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertSame($value, $object->foo); } @@ -54,7 +54,7 @@ public function can_hydrate_scalar_array(): void public array $foo = []; }; - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertSame($value, $object->foo); } @@ -76,7 +76,7 @@ public function __construct() $value = new Object1('foo'); - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertSame($value, $object->foo); } @@ -97,7 +97,7 @@ public function can_hydrate_object_array(): void new Object1('bar'), ]; - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertSame($value, $object->foo); } @@ -123,7 +123,7 @@ public function __construct() new Object1('bar'), ]; - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertInstanceOf(ArrayCollection::class, $object->foo); $this->assertSame($value, $object->foo->toArray()); @@ -150,7 +150,7 @@ public function __construct() new Object1('bar'), ]; - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertInstanceOf(ArrayCollection::class, $object->foo); $this->assertSame($value, $object->foo->toArray()); @@ -177,7 +177,7 @@ public function __construct() new Object1('bar'), ]; - Hydrator::set($object, 'foo', $value); + Hydrator::forceSet($object, 'foo', $value); $this->assertInstanceOf(ArrayCollection::class, $object->foo); $this->assertSame($value, $object->foo->toArray()); @@ -218,7 +218,7 @@ public function getFoo(): string } }; - Hydrator::set($object, 'foo', new ForceValue('foo')); + Hydrator::forceSet($object, 'foo', new ForceValue('foo')); $this->assertSame('foo', $object->getFoo()); }