From 3aab4a12709d5bd827cebffe970f712226f22ca9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Tue, 3 Jul 2018 16:44:43 +0200 Subject: [PATCH] [DoctrineBridge] Inject the entity manager instead of the class metadata factory in DoctrineExtractor --- UPGRADE-5.0.md | 7 +- src/Symfony/Bridge/Doctrine/CHANGELOG.md | 6 ++ .../PropertyInfo/DoctrineExtractor.php | 24 +++-- .../PropertyInfo/DoctrineExtractorTest.php | 88 +++++++++++++++---- 4 files changed, 103 insertions(+), 22 deletions(-) diff --git a/UPGRADE-5.0.md b/UPGRADE-5.0.md index b0f6e99c63..c9db89edee 100644 --- a/UPGRADE-5.0.md +++ b/UPGRADE-5.0.md @@ -36,13 +36,18 @@ Console $processHelper->run($output, Process::fromShellCommandline('ls -l')); ``` - DependencyInjection ------------------- * Removed the `TypedReference::canBeAutoregistered()` and `TypedReference::getRequiringClass()` methods. * Removed support for auto-discovered extension configuration class which does not implement `ConfigurationInterface`. +DoctrineBridge +-------------- + + * Deprecated injecting `ClassMetadataFactory` in `DoctrineExtractor`, an instance of `EntityManagerInterface` should be + injected instead + EventDispatcher --------------- diff --git a/src/Symfony/Bridge/Doctrine/CHANGELOG.md b/src/Symfony/Bridge/Doctrine/CHANGELOG.md index a3865db662..873d1e71f2 100644 --- a/src/Symfony/Bridge/Doctrine/CHANGELOG.md +++ b/src/Symfony/Bridge/Doctrine/CHANGELOG.md @@ -1,6 +1,12 @@ CHANGELOG ========= +4.2.0 +----- + + * deprecated injecting `ClassMetadataFactory` in `DoctrineExtractor`, + an instance of `EntityManagerInterface` should be injected instead + 4.1.0 ----- diff --git a/src/Symfony/Bridge/Doctrine/PropertyInfo/DoctrineExtractor.php b/src/Symfony/Bridge/Doctrine/PropertyInfo/DoctrineExtractor.php index 2a41422e00..030b09ecca 100644 --- a/src/Symfony/Bridge/Doctrine/PropertyInfo/DoctrineExtractor.php +++ b/src/Symfony/Bridge/Doctrine/PropertyInfo/DoctrineExtractor.php @@ -14,6 +14,7 @@ namespace Symfony\Bridge\Doctrine\PropertyInfo; use Doctrine\Common\Persistence\Mapping\ClassMetadataFactory; use Doctrine\Common\Persistence\Mapping\MappingException; use Doctrine\DBAL\Types\Type as DBALType; +use Doctrine\ORM\EntityManagerInterface; use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\ORM\Mapping\MappingException as OrmMappingException; use Symfony\Component\PropertyInfo\PropertyListExtractorInterface; @@ -27,11 +28,22 @@ use Symfony\Component\PropertyInfo\Type; */ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeExtractorInterface { + private $entityManager; private $classMetadataFactory; - public function __construct(ClassMetadataFactory $classMetadataFactory) + /** + * @param EntityManagerInterface $entityManager + */ + public function __construct($entityManager) { - $this->classMetadataFactory = $classMetadataFactory; + if ($entityManager instanceof EntityManagerInterface) { + $this->entityManager = $entityManager; + } elseif ($entityManager instanceof ClassMetadataFactory) { + @trigger_error(sprintf('Injecting an instance of "%s" in "%s" is deprecated since Symfony 4.2, inject an instance of "%s" instead.', ClassMetadataFactory::class, __CLASS__, EntityManagerInterface::class), E_USER_DEPRECATED); + $this->classMetadataFactory = $entityManager; + } else { + throw new \InvalidArgumentException(sprintf('$entityManager must be an instance of "%s", "%s" given.', EntityManagerInterface::class, \is_object($entityManager) ? \get_class($entityManager) : \gettype($entityManager))); + } } /** @@ -40,7 +52,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE public function getProperties($class, array $context = array()) { try { - $metadata = $this->classMetadataFactory->getMetadataFor($class); + $metadata = $this->entityManager ? $this->entityManager->getClassMetadata($class) : $this->classMetadataFactory->getMetadataFor($class); } catch (MappingException $exception) { return; } catch (OrmMappingException $exception) { @@ -66,7 +78,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE public function getTypes($class, $property, array $context = array()) { try { - $metadata = $this->classMetadataFactory->getMetadataFor($class); + $metadata = $this->entityManager ? $this->entityManager->getClassMetadata($class) : $this->classMetadataFactory->getMetadataFor($class); } catch (MappingException $exception) { return; } catch (OrmMappingException $exception) { @@ -96,7 +108,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE if (isset($associationMapping['indexBy'])) { $indexProperty = $associationMapping['indexBy']; /** @var ClassMetadataInfo $subMetadata */ - $subMetadata = $this->classMetadataFactory->getMetadataFor($associationMapping['targetEntity']); + $subMetadata = $this->entityManager ? $this->entityManager->getClassMetadata($associationMapping['targetEntity']) : $this->classMetadataFactory->getMetadataFor($associationMapping['targetEntity']); $typeOfField = $subMetadata->getTypeOfField($indexProperty); if (null === $typeOfField) { @@ -104,7 +116,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE /** @var ClassMetadataInfo $subMetadata */ $indexProperty = $subMetadata->getSingleAssociationReferencedJoinColumnName($indexProperty); - $subMetadata = $this->classMetadataFactory->getMetadataFor($associationMapping['targetEntity']); + $subMetadata = $this->entityManager ? $this->entityManager->getClassMetadata($associationMapping['targetEntity']) : $this->classMetadataFactory->getMetadataFor($associationMapping['targetEntity']); $typeOfField = $subMetadata->getTypeOfField($indexProperty); } diff --git a/src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/DoctrineExtractorTest.php b/src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/DoctrineExtractorTest.php index f212ab25e6..498c5ecbc9 100644 --- a/src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/DoctrineExtractorTest.php +++ b/src/Symfony/Bridge/Doctrine/Tests/PropertyInfo/DoctrineExtractorTest.php @@ -23,14 +23,9 @@ use Symfony\Component\PropertyInfo\Type; */ class DoctrineExtractorTest extends TestCase { - /** - * @var DoctrineExtractor - */ - private $extractor; - - protected function setUp() + private function createExtractor(bool $legacy = false) { - $config = Setup::createAnnotationMetadataConfiguration(array(__DIR__.DIRECTORY_SEPARATOR.'Fixtures'), true); + $config = Setup::createAnnotationMetadataConfiguration(array(__DIR__.\DIRECTORY_SEPARATOR.'Fixtures'), true); $entityManager = EntityManager::create(array('driver' => 'pdo_sqlite'), $config); if (!DBALType::hasType('foo')) { @@ -38,10 +33,20 @@ class DoctrineExtractorTest extends TestCase $entityManager->getConnection()->getDatabasePlatform()->registerDoctrineTypeMapping('custom_foo', 'foo'); } - $this->extractor = new DoctrineExtractor($entityManager->getMetadataFactory()); + return new DoctrineExtractor($legacy ? $entityManager->getMetadataFactory() : $entityManager); } public function testGetProperties() + { + $this->doTestGetProperties(false); + } + + public function testLegacyGetProperties() + { + $this->doTestGetProperties(true); + } + + private function doTestGetProperties(bool $legacy) { $this->assertEquals( array( @@ -63,11 +68,21 @@ class DoctrineExtractorTest extends TestCase 'indexedBar', 'indexedFoo', ), - $this->extractor->getProperties('Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineDummy') + $this->createExtractor($legacy)->getProperties('Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineDummy') ); } - public function testGetPropertiesWithEmbedded() + public function testTestGetPropertiesWithEmbedded() + { + $this->doTestGetPropertiesWithEmbedded(false); + } + + public function testLegacyTestGetPropertiesWithEmbedded() + { + $this->doTestGetPropertiesWithEmbedded(true); + } + + private function doTestGetPropertiesWithEmbedded(bool $legacy) { if (!class_exists('Doctrine\ORM\Mapping\Embedded')) { $this->markTestSkipped('@Embedded is not available in Doctrine ORM lower than 2.5.'); @@ -78,7 +93,7 @@ class DoctrineExtractorTest extends TestCase 'id', 'embedded', ), - $this->extractor->getProperties('Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineWithEmbedded') + $this->createExtractor($legacy)->getProperties('Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineWithEmbedded') ); } @@ -87,10 +102,33 @@ class DoctrineExtractorTest extends TestCase */ public function testExtract($property, array $type = null) { - $this->assertEquals($type, $this->extractor->getTypes('Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineDummy', $property, array())); + $this->doTestExtract(false, $property, $type); + } + + /** + * @dataProvider typesProvider + */ + public function testLegacyExtract($property, array $type = null) + { + $this->doTestExtract(true, $property, $type); + } + + private function doTestExtract(bool $legacy, $property, array $type = null) + { + $this->assertEquals($type, $this->createExtractor($legacy)->getTypes('Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineDummy', $property, array())); } public function testExtractWithEmbedded() + { + $this->doTestExtractWithEmbedded(false); + } + + public function testLegacyExtractWithEmbedded() + { + $this->doTestExtractWithEmbedded(true); + } + + private function doTestExtractWithEmbedded(bool $legacy) { if (!class_exists('Doctrine\ORM\Mapping\Embedded')) { $this->markTestSkipped('@Embedded is not available in Doctrine ORM lower than 2.5.'); @@ -102,7 +140,7 @@ class DoctrineExtractorTest extends TestCase 'Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineEmbeddable' )); - $actualTypes = $this->extractor->getTypes( + $actualTypes = $this->createExtractor($legacy)->getTypes( 'Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineWithEmbedded', 'embedded', array() @@ -158,11 +196,31 @@ class DoctrineExtractorTest extends TestCase public function testGetPropertiesCatchException() { - $this->assertNull($this->extractor->getProperties('Not\Exist')); + $this->doTestGetPropertiesCatchException(false); + } + + public function testLegacyGetPropertiesCatchException() + { + $this->doTestGetPropertiesCatchException(true); + } + + private function doTestGetPropertiesCatchException(bool $legacy) + { + $this->assertNull($this->createExtractor($legacy)->getProperties('Not\Exist')); } public function testGetTypesCatchException() { - $this->assertNull($this->extractor->getTypes('Not\Exist', 'baz')); + return $this->doTestGetTypesCatchException(false); + } + + public function testLegacyGetTypesCatchException() + { + return $this->doTestGetTypesCatchException(true); + } + + private function doTestGetTypesCatchException(bool $legacy) + { + $this->assertNull($this->createExtractor($legacy)->getTypes('Not\Exist', 'baz')); } }