diff --git a/UPGRADE.md b/UPGRADE.md index 99ac94d157..70269ce1d0 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -1,5 +1,13 @@ # Upgrade to 2.12 +## Deprecate custom repository classes that don't extend `EntityRepository` + +Although undocumented, it is currently possible to configure a custom repository +class that implements `ObjectRepository` but does not extend the +`EntityRepository` base class. + +This is now deprecated. Please extend `EntityRepository` instead. + ## Deprecated more APIs related to entity namespace aliases ```diff diff --git a/lib/Doctrine/ORM/Configuration.php b/lib/Doctrine/ORM/Configuration.php index 40ff3cb271..200298e236 100644 --- a/lib/Doctrine/ORM/Configuration.php +++ b/lib/Doctrine/ORM/Configuration.php @@ -45,9 +45,9 @@ use Doctrine\Persistence\ObjectRepository; use LogicException; use Psr\Cache\CacheItemPoolInterface; -use ReflectionClass; use function class_exists; +use function is_a; use function method_exists; use function sprintf; use function strtolower; @@ -843,6 +843,7 @@ public function getFilterClassName($name) * Sets default repository class. * * @param string $className + * @psalm-param class-string $className * * @return void * @@ -850,12 +851,20 @@ public function getFilterClassName($name) */ public function setDefaultRepositoryClassName($className) { - $reflectionClass = new ReflectionClass($className); - - if (! $reflectionClass->implementsInterface(ObjectRepository::class)) { + if (! class_exists($className) || ! is_a($className, ObjectRepository::class, true)) { throw InvalidEntityRepository::fromClassName($className); } + if (! is_a($className, EntityRepository::class, true)) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/9533', + 'Configuring %s as default repository class is deprecated because it does not extend %s.', + $className, + EntityRepository::class + ); + } + $this->_attributes['defaultRepositoryClassName'] = $className; } @@ -863,7 +872,7 @@ public function setDefaultRepositoryClassName($className) * Get default repository class. * * @return string - * @psalm-return class-string + * @psalm-return class-string */ public function getDefaultRepositoryClassName() { diff --git a/lib/Doctrine/ORM/EntityManager.php b/lib/Doctrine/ORM/EntityManager.php index 40f5c763a0..5f5130e090 100644 --- a/lib/Doctrine/ORM/EntityManager.php +++ b/lib/Doctrine/ORM/EntityManager.php @@ -803,7 +803,18 @@ public function getRepository($entityName) } } - return $this->repositoryFactory->getRepository($this, $entityName); + $repository = $this->repositoryFactory->getRepository($this, $entityName); + if (! $repository instanceof EntityRepository) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/9533', + 'Not returning an instance of %s from %s::getRepository() is deprecated and will cause a TypeError on 3.0.', + EntityRepository::class, + get_debug_type($this->repositoryFactory) + ); + } + + return $repository; } /** diff --git a/lib/Doctrine/ORM/Exception/InvalidEntityRepository.php b/lib/Doctrine/ORM/Exception/InvalidEntityRepository.php index b7d27734a5..47987da445 100644 --- a/lib/Doctrine/ORM/Exception/InvalidEntityRepository.php +++ b/lib/Doctrine/ORM/Exception/InvalidEntityRepository.php @@ -4,14 +4,14 @@ namespace Doctrine\ORM\Exception; -use Doctrine\Persistence\ObjectRepository; +use Doctrine\ORM\EntityRepository; final class InvalidEntityRepository extends ORMException implements ConfigurationException { public static function fromClassName(string $className): self { return new self( - "Invalid repository class '" . $className . "'. It must be a " . ObjectRepository::class . '.' + "Invalid repository class '" . $className . "'. It must be a " . EntityRepository::class . '.' ); } } diff --git a/lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php b/lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php index 82558a520a..66f42470f4 100644 --- a/lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php +++ b/lib/Doctrine/ORM/Repository/DefaultRepositoryFactory.php @@ -4,7 +4,9 @@ namespace Doctrine\ORM\Repository; +use Doctrine\Deprecations\Deprecation; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\EntityRepository; use Doctrine\Persistence\ObjectRepository; use function spl_object_id; @@ -18,6 +20,7 @@ final class DefaultRepositoryFactory implements RepositoryFactory * The list of EntityRepository instances. * * @var ObjectRepository[] + * @psalm-var array */ private $repositoryList = []; @@ -49,6 +52,17 @@ private function createRepository( $repositoryClassName = $metadata->customRepositoryClassName ?: $entityManager->getConfiguration()->getDefaultRepositoryClassName(); - return new $repositoryClassName($entityManager, $metadata); + $repository = new $repositoryClassName($entityManager, $metadata); + if (! $repository instanceof EntityRepository) { + Deprecation::trigger( + 'doctrine/orm', + 'https://github.com/doctrine/orm/pull/9533', + 'Configuring %s as repository class is deprecated because it does not extend %s.', + $repositoryClassName, + EntityRepository::class + ); + } + + return $repository; } } diff --git a/lib/Doctrine/ORM/Repository/RepositoryFactory.php b/lib/Doctrine/ORM/Repository/RepositoryFactory.php index e792e3897b..5b00ca01d4 100644 --- a/lib/Doctrine/ORM/Repository/RepositoryFactory.php +++ b/lib/Doctrine/ORM/Repository/RepositoryFactory.php @@ -18,7 +18,7 @@ interface RepositoryFactory * @param EntityManagerInterface $entityManager The EntityManager instance. * @param class-string $entityName The name of the entity. * - * @return ObjectRepository + * @return ObjectRepository This type will change to {@see EntityRepository} in 3.0. * * @template T of object */ diff --git a/psalm-baseline.xml b/psalm-baseline.xml index d1dc14ecfc..2e99eb0f58 100644 --- a/psalm-baseline.xml +++ b/psalm-baseline.xml @@ -224,8 +224,7 @@ - - $className + $className @@ -297,15 +296,9 @@ getPartialReference getReference - - $this->repositoryFactory->getRepository($this, $entityName) - wrapInTransaction - - EntityRepository<T> - $entity $entity @@ -2677,12 +2670,12 @@ - + + $repository instanceof EntityRepository + + new $repositoryClassName($entityManager, $metadata) - - - ObjectRepository - + diff --git a/tests/Doctrine/Performance/Hydration/SimpleHydrationBench.php b/tests/Doctrine/Performance/Hydration/SimpleHydrationBench.php index 85b66d9215..5d67ac1d76 100644 --- a/tests/Doctrine/Performance/Hydration/SimpleHydrationBench.php +++ b/tests/Doctrine/Performance/Hydration/SimpleHydrationBench.php @@ -5,8 +5,8 @@ namespace Doctrine\Performance\Hydration; use Doctrine\ORM\EntityManagerInterface; +use Doctrine\ORM\EntityRepository; use Doctrine\Performance\EntityManagerFactory; -use Doctrine\Persistence\ObjectRepository; use Doctrine\Tests\Models\CMS; /** @@ -17,7 +17,7 @@ final class SimpleHydrationBench /** @var EntityManagerInterface */ private $entityManager; - /** @var ObjectRepository */ + /** @var EntityRepository */ private $repository; public function init(): void diff --git a/tests/Doctrine/Performance/Hydration/SingleTableInheritanceHydrationPerformanceBench.php b/tests/Doctrine/Performance/Hydration/SingleTableInheritanceHydrationPerformanceBench.php index 6dd9f780ee..a816117f11 100644 --- a/tests/Doctrine/Performance/Hydration/SingleTableInheritanceHydrationPerformanceBench.php +++ b/tests/Doctrine/Performance/Hydration/SingleTableInheritanceHydrationPerformanceBench.php @@ -4,8 +4,8 @@ namespace Doctrine\Performance\Hydration; +use Doctrine\ORM\EntityRepository; use Doctrine\Performance\EntityManagerFactory; -use Doctrine\Persistence\ObjectRepository; use Doctrine\Tests\Models\Company; use PhpBench\Benchmark\Metadata\Annotations\BeforeMethods; @@ -14,16 +14,16 @@ */ final class SingleTableInheritanceHydrationPerformanceBench { - /** @var ObjectRepository */ + /** @var EntityRepository */ private $contractsRepository; - /** @var ObjectRepository */ + /** @var EntityRepository */ private $fixContractsRepository; - /** @var ObjectRepository */ + /** @var EntityRepository */ private $flexContractRepository; - /** @var ObjectRepository */ + /** @var EntityRepository */ private $ultraContractRepository; public function init(): void diff --git a/tests/Doctrine/Tests/ORM/ConfigurationTest.php b/tests/Doctrine/Tests/ORM/ConfigurationTest.php index ad0099c28b..530cb2f6d5 100644 --- a/tests/Doctrine/Tests/ORM/ConfigurationTest.php +++ b/tests/Doctrine/Tests/ORM/ConfigurationTest.php @@ -16,6 +16,7 @@ use Doctrine\ORM\Cache\Exception\QueryCacheUsesNonPersistentCache; use Doctrine\ORM\Configuration; use Doctrine\ORM\EntityRepository; +use Doctrine\ORM\Exception\InvalidEntityRepository; use Doctrine\ORM\Exception\ORMException; use Doctrine\ORM\Exception\ProxyClassesAlwaysRegenerating; use Doctrine\ORM\Mapping as AnnotationNamespace; @@ -25,10 +26,12 @@ use Doctrine\ORM\Mapping\QuoteStrategy; use Doctrine\ORM\Query\ResultSetMapping; use Doctrine\Persistence\Mapping\Driver\MappingDriver; +use Doctrine\Persistence\ObjectRepository; use Doctrine\Tests\DoctrineTestCase; use Doctrine\Tests\Models\DDC753\DDC753CustomRepository; use Psr\Cache\CacheItemPoolInterface; use ReflectionClass; +use stdClass; use Symfony\Component\Cache\Adapter\ArrayAdapter; use function class_exists; @@ -388,10 +391,19 @@ public function setDefaultRepositoryClassName(): void self::assertSame(EntityRepository::class, $this->configuration->getDefaultRepositoryClassName()); $this->configuration->setDefaultRepositoryClassName(DDC753CustomRepository::class); self::assertSame(DDC753CustomRepository::class, $this->configuration->getDefaultRepositoryClassName()); - $this->expectException(ORMException::class); + $this->expectException(InvalidEntityRepository::class); + $this->expectExceptionMessage('Invalid repository class \'Doctrine\Tests\ORM\ConfigurationTest\'. It must be a Doctrine\ORM\EntityRepository.'); $this->configuration->setDefaultRepositoryClassName(self::class); } + public function testSetDeprecatedDefaultRepositoryClassName(): void + { + $this->expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/9533'); + + $this->configuration->setDefaultRepositoryClassName(DeprecatedRepository::class); + self::assertSame(DeprecatedRepository::class, $this->configuration->getDefaultRepositoryClassName()); + } + public function testSetGetNamingStrategy(): void { self::assertInstanceOf(NamingStrategy::class, $this->configuration->getNamingStrategy()); @@ -445,3 +457,37 @@ public function namespacedAnnotationMethod(): void { } } + +class DeprecatedRepository implements ObjectRepository +{ + /** + * {@inheritdoc} + */ + public function find($id) + { + return null; + } + + public function findAll(): array + { + return []; + } + + public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null): array + { + return []; + } + + /** + * {@inheritdoc} + */ + public function findOneBy(array $criteria) + { + return null; + } + + public function getClassName(): string + { + return stdClass::class; + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/CustomRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/CustomRepositoryTest.php new file mode 100644 index 0000000000..5861c23036 --- /dev/null +++ b/tests/Doctrine/Tests/ORM/Functional/CustomRepositoryTest.php @@ -0,0 +1,151 @@ +expectDeprecationWithIdentifier('https://github.com/doctrine/orm/pull/9533'); + $this->createSchemaForModels(MinimalEntity::class, OtherMinimalEntity::class); + } + + public function testMinimalRepository(): void + { + $this->_em->persist(new MinimalEntity('foo')); + $this->_em->persist(new MinimalEntity('bar')); + $this->_em->flush(); + $this->_em->clear(); + + $repository = $this->_em->getRepository(MinimalEntity::class); + self::assertInstanceOf(MinimalRepository::class, $repository); + self::assertSame('foo', $repository->find('foo')->id); + } + + public function testMinimalDefaultRepository(): void + { + $this->_em->getConfiguration()->setDefaultRepositoryClassName(MinimalRepository::class); + + $this->_em->persist(new OtherMinimalEntity('foo')); + $this->_em->persist(new OtherMinimalEntity('bar')); + $this->_em->flush(); + $this->_em->clear(); + + $repository = $this->_em->getRepository(OtherMinimalEntity::class); + self::assertInstanceOf(MinimalRepository::class, $repository); + self::assertSame('foo', $repository->find('foo')->id); + } +} + +/** + * @ORM\Entity(repositoryClass="MinimalRepository") + */ +class MinimalEntity +{ + /** + * @ORM\Column + * @ORM\Id + * + * @var string + */ + public $id; + + public function __construct(string $id) + { + $this->id = $id; + } +} + +/** + * @ORM\Entity + */ +class OtherMinimalEntity +{ + /** + * @ORM\Column + * @ORM\Id + * + * @var string + */ + public $id; + + public function __construct(string $id) + { + $this->id = $id; + } +} + +/** + * @template TEntity of object + * @implements ObjectRepository + */ +class MinimalRepository implements ObjectRepository +{ + /** @var EntityManagerInterface */ + private $em; + /** @var ClassMetadata */ + private $class; + + /** + * @psalm-param ClassMetadata $class + */ + public function __construct(EntityManagerInterface $em, ClassMetadata $class) + { + $this->em = $em; + $this->class = $class; + } + + /** + * {@inheritdoc} + */ + public function find($id) + { + return $this->em->find($this->class->name, $id); + } + + /** + * {@inheritdoc} + */ + public function findAll(): array + { + return $this->findBy([]); + } + + /** + * {@inheritdoc} + */ + public function findBy(array $criteria, ?array $orderBy = null, $limit = null, $offset = null): array + { + $persister = $this->em->getUnitOfWork()->getEntityPersister($this->class->name); + + return $persister->loadAll($criteria, $orderBy, $limit, $offset); + } + + /** + * {@inheritdoc} + */ + public function findOneBy(array $criteria) + { + $persister = $this->em->getUnitOfWork()->getEntityPersister($this->class->name); + + return $persister->load($criteria, null, null, [], null, 1); + } + + public function getClassName(): string + { + return $this->class->name; + } +} diff --git a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php index 0506458b62..3cfec064d2 100644 --- a/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php +++ b/tests/Doctrine/Tests/ORM/Functional/EntityRepositoryTest.php @@ -14,7 +14,6 @@ use Doctrine\DBAL\ParameterType; use Doctrine\Deprecations\PHPUnit\VerifyDeprecations; use Doctrine\ORM\EntityRepository; -use Doctrine\ORM\Exception\InvalidEntityRepository; use Doctrine\ORM\Exception\NotSupported; use Doctrine\ORM\Exception\ORMException; use Doctrine\ORM\Exception\UnrecognizedIdentifierFields; @@ -33,7 +32,6 @@ use Doctrine\Tests\Models\DDC753\DDC753DefaultRepository; use Doctrine\Tests\Models\DDC753\DDC753EntityWithCustomRepository; use Doctrine\Tests\Models\DDC753\DDC753EntityWithDefaultCustomRepository; -use Doctrine\Tests\Models\DDC753\DDC753InvalidRepository; use Doctrine\Tests\OrmFunctionalTestCase; use function array_fill; @@ -663,17 +661,6 @@ public function testDefaultRepositoryClassName(): void self::assertEquals($this->_em->getConfiguration()->getDefaultRepositoryClassName(), EntityRepository::class); } - /** - * @group DDC-753 - */ - public function testSetDefaultRepositoryInvalidClassError(): void - { - $this->expectException(InvalidEntityRepository::class); - $this->expectExceptionMessage('Invalid repository class \'Doctrine\Tests\Models\DDC753\DDC753InvalidRepository\'. It must be a Doctrine\Persistence\ObjectRepository.'); - self::assertEquals($this->_em->getConfiguration()->getDefaultRepositoryClassName(), EntityRepository::class); - $this->_em->getConfiguration()->setDefaultRepositoryClassName(DDC753InvalidRepository::class); - } - /** * @group DDC-3257 */