diff --git a/docs/en/reference/advanced-configuration.rst b/docs/en/reference/advanced-configuration.rst index 42a0833aca..8758c8ccef 100644 --- a/docs/en/reference/advanced-configuration.rst +++ b/docs/en/reference/advanced-configuration.rst @@ -342,7 +342,7 @@ for the ``$identifier`` parameter is passed. ``$identifier`` values are not checked and there is no guarantee that the requested entity instance even exists – the method will still return a proxy object. -Its only when the proxy has to be fully initialized or associations cannot +It is only when the proxy has to be fully initialized or associations cannot be written to the database that invalid ``$identifier`` values may lead to exceptions. diff --git a/src/EntityManager.php b/src/EntityManager.php index f7d47d7b12..206080b929 100644 --- a/src/EntityManager.php +++ b/src/EntityManager.php @@ -434,50 +434,9 @@ public function find($className, $id, $lockMode = null, $lockVersion = null) $this->checkLockRequirements($lockMode, $class); } - if (! is_array($id)) { - if ($class->isIdentifierComposite) { - throw ORMInvalidArgumentException::invalidCompositeIdentifier(); - } - - $id = [$class->identifier[0] => $id]; - } - - foreach ($id as $i => $value) { - if (is_object($value)) { - $className = DefaultProxyClassNameResolver::getClass($value); - if ($this->metadataFactory->hasMetadataFor($className)) { - $id[$i] = $this->unitOfWork->getSingleIdentifierValue($value); - - if ($id[$i] === null) { - throw ORMInvalidArgumentException::invalidIdentifierBindingEntity($className); - } - } - } - } - - $sortedId = []; - - foreach ($class->identifier as $identifier) { - if (! isset($id[$identifier])) { - throw MissingIdentifierField::fromFieldAndClass($identifier, $class->name); - } - - if ($id[$identifier] instanceof BackedEnum) { - $sortedId[$identifier] = $id[$identifier]->value; - } else { - $sortedId[$identifier] = $id[$identifier]; - } - - unset($id[$identifier]); - } - - if ($id) { - throw UnrecognizedIdentifierFields::fromClassAndFieldNames($class->name, array_keys($id)); - } - - $unitOfWork = $this->getUnitOfWork(); + $canonicalId = $this->getCanonicalId($class, $id); - $entity = $unitOfWork->tryGetById($sortedId, $class->rootEntityName); + $entity = $this->unitOfWork->tryGetById($canonicalId, $class->rootEntityName); // Check identity map first if ($entity !== false) { @@ -493,32 +452,32 @@ public function find($className, $id, $lockMode = null, $lockVersion = null) case $lockMode === LockMode::NONE: case $lockMode === LockMode::PESSIMISTIC_READ: case $lockMode === LockMode::PESSIMISTIC_WRITE: - $persister = $unitOfWork->getEntityPersister($class->name); - $persister->refresh($sortedId, $entity, $lockMode); + $persister = $this->unitOfWork->getEntityPersister($class->name); + $persister->refresh($canonicalId, $entity, $lockMode); break; } return $entity; // Hit! } - $persister = $unitOfWork->getEntityPersister($class->name); + $persister = $this->unitOfWork->getEntityPersister($class->name); switch (true) { case $lockMode === LockMode::OPTIMISTIC: - $entity = $persister->load($sortedId); + $entity = $persister->load($canonicalId); if ($entity !== null) { - $unitOfWork->lock($entity, $lockMode, $lockVersion); + $this->unitOfWork->lock($entity, $lockMode, $lockVersion); } return $entity; case $lockMode === LockMode::PESSIMISTIC_READ: case $lockMode === LockMode::PESSIMISTIC_WRITE: - return $persister->load($sortedId, null, null, [], $lockMode); + return $persister->load($canonicalId, null, null, [], $lockMode); default: - return $persister->loadById($sortedId); + return $persister->loadById($canonicalId); } } @@ -529,26 +488,9 @@ public function getReference($entityName, $id) { $class = $this->metadataFactory->getMetadataFor(ltrim($entityName, '\\')); - if (! is_array($id)) { - $id = [$class->identifier[0] => $id]; - } - - $sortedId = []; - - foreach ($class->identifier as $identifier) { - if (! isset($id[$identifier])) { - throw MissingIdentifierField::fromFieldAndClass($identifier, $class->name); - } - - $sortedId[$identifier] = $id[$identifier]; - unset($id[$identifier]); - } - - if ($id) { - throw UnrecognizedIdentifierFields::fromClassAndFieldNames($class->name, array_keys($id)); - } + $canonicalId = $this->getCanonicalId($class, $id); - $entity = $this->unitOfWork->tryGetById($sortedId, $class->rootEntityName); + $entity = $this->unitOfWork->tryGetById($canonicalId, $class->rootEntityName); // Check identity map first, if its already in there just return it. if ($entity !== false) { @@ -556,12 +498,12 @@ public function getReference($entityName, $id) } if ($class->subClasses) { - return $this->find($entityName, $sortedId); + return $this->find($entityName, $canonicalId); } - $entity = $this->proxyFactory->getProxy($class->name, $sortedId); + $entity = $this->proxyFactory->getProxy($class->name, $canonicalId); - $this->unitOfWork->registerManaged($entity, $sortedId, []); + $this->unitOfWork->registerManaged($entity, $canonicalId, []); return $entity; } @@ -1117,4 +1059,59 @@ private function configureLegacyMetadataCache(): void // Wrap doctrine/cache to provide PSR-6 interface $this->metadataFactory->setCache(CacheAdapter::wrap($metadataCache)); } + + /** + * @param mixed $id The identitifiers to sort. + * + * @return mixed The sorted identifiers. + * + * @throws ORMInvalidArgumentException + * @throws MissingIdentifierField + * @throws UnrecognizedIdentifierFields + */ + private function getCanonicalId(ClassMetadata $class, $id) + { + if (! is_array($id)) { + if ($class->isIdentifierComposite) { + throw ORMInvalidArgumentException::invalidCompositeIdentifier(); + } + + $id = [$class->identifier[0] => $id]; + } + + foreach ($id as $i => $value) { + if (is_object($value)) { + $className = DefaultProxyClassNameResolver::getClass($value); + if ($this->metadataFactory->hasMetadataFor($className)) { + $id[$i] = $this->unitOfWork->getSingleIdentifierValue($value); + + if ($id[$i] === null) { + throw ORMInvalidArgumentException::invalidIdentifierBindingEntity($className); + } + } + } + } + + $canonicalId = []; + + foreach ($class->identifier as $identifier) { + if (! isset($id[$identifier])) { + throw MissingIdentifierField::fromFieldAndClass($identifier, $class->name); + } + + if ($id[$identifier] instanceof BackedEnum) { + $canonicalId[$identifier] = $id[$identifier]->value; + } else { + $canonicalId[$identifier] = $id[$identifier]; + } + + unset($id[$identifier]); + } + + if ($id) { + throw UnrecognizedIdentifierFields::fromClassAndFieldNames($class->name, array_keys($id)); + } + + return $canonicalId; + } } diff --git a/src/Proxy/ProxyFactory.php b/src/Proxy/ProxyFactory.php index dc8a72bfce..9ee7e9e55b 100644 --- a/src/Proxy/ProxyFactory.php +++ b/src/Proxy/ProxyFactory.php @@ -26,6 +26,7 @@ use function array_combine; use function array_flip; use function array_intersect_key; +use function assert; use function bin2hex; use function chmod; use function class_exists; @@ -465,8 +466,9 @@ private function getProxyFactory(string $className): Closure $initializer = $this->createLazyInitializer($class, $entityPersister, $this->identifierFlattener); $proxyClassName = $this->loadProxyClass($class); $identifierFields = array_intersect_key($class->getReflectionProperties(), $identifiers); + $em = $this->em; - $proxyFactory = Closure::bind(static function (array $identifier) use ($initializer, $skippedProperties, $identifierFields, $className): InternalProxy { + $proxyFactory = Closure::bind(static function (array $identifier) use ($initializer, $skippedProperties, $identifierFields, $className, $class, $em): InternalProxy { $proxy = self::createLazyGhost(static function (InternalProxy $object) use ($initializer, $identifier): void { $initializer($object, $identifier); }, $skippedProperties); @@ -476,7 +478,17 @@ private function getProxyFactory(string $className): Closure throw ORMInvalidArgumentException::missingPrimaryKeyValue($className, $idField); } - $reflector->setValue($proxy, $identifier[$idField]); + assert($reflector !== null); + + $idValue = $identifier[$idField]; + if ($class->hasAssociation($idField)) { + $idValue = $em->getReference( + $class->getAssociationTargetClass($idField), + $idValue + ); + } + + $reflector->setValue($proxy, $idValue); } return $proxy; diff --git a/tests/Tests/Models/RelationAsId/Group.php b/tests/Tests/Models/RelationAsId/Group.php new file mode 100644 index 0000000000..c15159aeed --- /dev/null +++ b/tests/Tests/Models/RelationAsId/Group.php @@ -0,0 +1,29 @@ +useModelSet('relation_as_id'); + + parent::setUp(); + + $u = new User(); + $u->id = 1; + $u->name = 'Athos'; + + $p = new Profile(); + $p->user = $u; + $p->url = 'https://example.com'; + + $g = new Group(); + $g->id = 11; + $g->name = 'Mousquetaires'; + + $m = new Membership(); + $m->user = $u; + $m->group = $g; + $m->role = 'cadet'; + + $u2 = new User(); + $u2->id = 2; + $u2->name = 'Portos'; + + $this->_em->persist($u); + $this->_em->persist($p); + $this->_em->persist($g); + $this->_em->persist($m); + $this->_em->persist($u2); + $this->_em->flush(); + $this->_em->clear(); + } + + public function testCanGetByValue(): void + { + $profile = $this->_em->getReference(Profile::class, 1); + + if ($this->_em->getConfiguration()->isLazyGhostObjectEnabled()) { + self::assertInstanceOf(UserProxy::class, $profile->user); + self::assertEquals('Athos', $profile->user->name); + } else { + if (PHP_VERSION_ID >= 80100) { + $this->markTestIncomplete(); + } + self::assertEquals(1, $profile->user); + } + } + + public function testThrowsSensiblyIfNotFoundByValue(): void + { + self::markTestSkipped('unable'); + $profile = $this->_em->getReference(Profile::class, 999); + + $this->expectException(EntityNotFoundException::class); + $profile->url; + } + + public function testCanGetByRelatedEntity(): void + { + $user = $this->_em->find(User::class, 1); + $profile = $this->_em->getReference(Profile::class, $user); + + if ($this->_em->getConfiguration()->isLazyGhostObjectEnabled()) { + self::assertInstanceOf(User::class, $profile->user); + self::assertEquals('Athos', $profile->user->name); + } else { + if (PHP_VERSION_ID >= 80100) { + $this->markTestIncomplete(); + } + self::assertEquals(1, $profile->user); + } + } + + public function testThrowsSensiblyIfNotFoundByValidRelatedEntity(): void + { + self::markTestSkipped('unable'); + $user = $this->_em->find(User::class, 2); + $profile = $this->_em->getReference(Profile::class, $user); + + $this->expectException(EntityNotFoundException::class); + $profile->url; + } + + public function testThrowsSensiblyIfNotFoundByBrokenRelatedEntity(): void + { + self::markTestSkipped('unable'); + $user = new User(); + $user->id = 999; + $profile = $this->_em->getReference(Profile::class, $user); + + $this->expectException(EntityNotFoundException::class); + $profile->url; + } + + public function testCanGetByRelatedProxy(): void + { + $user = $this->_em->getReference(User::class, 1); + $profile = $this->_em->getReference(Profile::class, $user); + + if ($this->_em->getConfiguration()->isLazyGhostObjectEnabled()) { + self::assertInstanceOf(UserProxy::class, $profile->user); + self::assertEquals('Athos', $profile->user->name); + } else { + if (PHP_VERSION_ID >= 80100) { + $this->markTestIncomplete(); + } + self::assertEquals(1, $profile->user); + } + } + + public function testThrowsSensiblyIfNotFoundByValidProxy(): void + { + self::markTestSkipped('unable'); + $user = $this->_em->getReference(User::class, 2); + $profile = $this->_em->getReference(Profile::class, $user); + + $this->expectException(EntityNotFoundException::class); + $profile->url; + } + + public function testThrowsSensiblyIfNotFoundByBrokenProxy(): void + { + self::markTestSkipped('unable'); + $user = $this->_em->getReference(User::class, 999); + $profile = $this->_em->getReference(Profile::class, $user); + + $this->expectException(EntityNotFoundException::class); + $profile->url; + } + + public function testCanGetOnCompositeIdByValueOrRelatedProxy(): void + { + $membership = $this->_em->getReference(Membership::class, [ + 'user' => 1, + 'group' => 11, + ]); + + if ($this->_em->getConfiguration()->isLazyGhostObjectEnabled()) { + self::assertInstanceOf(MembershipProxy::class, $membership); + self::assertEquals('Mousquetaires', $membership->group->name); + } else { + if (PHP_VERSION_ID >= 80100) { + $this->markTestIncomplete(); + } + self::assertEquals(11, $membership->group); + } + + $this->_em->clear(); + + $group = $this->_em->getReference(Group::class, 11); + $membership = $this->_em->getReference(Membership::class, [ + 'user' => 1, + 'group' => $group, + ]); + + if ($this->_em->getConfiguration()->isLazyGhostObjectEnabled()) { + self::assertInstanceOf(MembershipProxy::class, $membership); + self::assertEquals('Mousquetaires', $membership->group->name); + } else { + if (PHP_VERSION_ID >= 80100) { + $this->markTestIncomplete(); + } + self::assertEquals(11, $membership->group); + } + } + + public function testCanUpdateProperty(): void + { + $porthos = $this->_em->getReference(User::class, 2); + + $porthos->name = 'Porthos'; + $this->_em->persist($porthos); + $this->_em->flush(); + $this->_em->clear(); + + $fixedPorthos = $this->_em->find(User::class, 2); + self::assertEquals('Porthos', $fixedPorthos->name); + } + + public function testCanUpdateIdRegularId(): void + { + // "Proxy objects should be transparent to your code." + self::markTestSkipped('not a regression?'); + + $porthos = $this->_em->getReference(User::class, 2); + $porthos->id = 9; + $this->_em->persist($porthos); + $this->_em->flush(); + $this->_em->clear(); + + $reindexedPorthos = $this->_em->find(User::class, 9); + self::assertNotNull($reindexedPorthos); + } + + public function testCanUpdateIdByRelatedProxy(): void + { + // "Proxy objects should be transparent to your code." + self::markTestSkipped('not a regression?'); + + $porthos = $this->_em->getReference(User::class, 2); + $profile = $this->_em->getReference(Profile::class, 1); + $profile->user = $porthos; + $this->_em->persist($profile); + $this->_em->flush(); + $this->_em->clear(); + + $profile = $this->_em->find(Profile::class, 2); + self::assertNotNull($profile); + } + + public function testCanUpdateIdByEntity(): void + { + // "Proxy objects should be transparent to your code." + self::markTestSkipped('not a regression?'); + + $porthos = $this->_em->find(User::class, 2); + $profile = $this->_em->getReference(Profile::class, 1); + $profile->user = $porthos; + $this->_em->persist($profile); + $this->_em->flush(); + $this->_em->clear(); + + $profile = $this->_em->find(Profile::class, 2); + self::assertNotNull($profile); + } + + public function testCanUpdateIdCompositeId(): void + { + // "Proxy objects should be transparent to your code." + self::markTestSkipped('not a regression?'); + + $membership = $this->_em->getReference(Membership::class, [ + 'user' => 1, + 'group' => 11, + ]); + + $g2 = new Group(); + $g2->id = 12; + $g2->name = 'Ordre de la Jarretière'; + $membership->group = $g2; + $this->_em->persist($g2); + $this->_em->persist($membership); + $this->_em->flush(); + $this->_em->clear(); + + $membership = $this->_em->find(Membership::class, [ + 'user' => 1, + 'group' => 12, + ]); + self::assertEquals('admin', $membership->role); + } +} diff --git a/tests/Tests/OrmFunctionalTestCase.php b/tests/Tests/OrmFunctionalTestCase.php index f81eddc7d5..11438befee 100644 --- a/tests/Tests/OrmFunctionalTestCase.php +++ b/tests/Tests/OrmFunctionalTestCase.php @@ -346,6 +346,13 @@ abstract class OrmFunctionalTestCase extends OrmTestCase Models\Issue9300\Issue9300Child::class, Models\Issue9300\Issue9300Parent::class, ], + 'relation_as_id' => [ + Models\RelationAsId\User::class, + Models\RelationAsId\Profile::class, + Models\RelationAsId\Group::class, + Models\RelationAsId\Membership::class, + + ], ]; /** @param class-string ...$models */ @@ -675,6 +682,13 @@ protected function tearDown(): void $conn->executeStatement('DELETE FROM issue5989_managers'); } + if (isset($this->_usedModelSets['relation_as_id'])) { + $conn->executeStatement('DELETE FROM relation_as_id_membership'); + $conn->executeStatement('DELETE FROM relation_as_id_profile'); + $conn->executeStatement('DELETE FROM relation_as_id_group'); + $conn->executeStatement('DELETE FROM relation_as_id_user'); + } + $this->_em->clear(); }