feature #27829 [DoctrineBridge] Inject the entity manager instead of the class metadata factory in DoctrineExtractor (dunglas)

This PR was squashed before being merged into the 4.2-dev branch (closes #27829).

Discussion
----------

[DoctrineBridge] Inject the entity manager instead of the class metadata factory in DoctrineExtractor

| Q             | A
| ------------- | ---
| Branch?       | master
| Bug fix?      | no
| New feature?  |no <!-- don't forget to update src/**/CHANGELOG.md files -->
| BC breaks?    | no     <!-- see https://symfony.com/bc -->
| Deprecations? | yes <!-- don't forget to update UPGRADE-*.md and src/**/CHANGELOG.md files -->
| Tests pass?   | yes    <!-- please add some, will be required by reviewers -->
| Fixed tickets | n/a   <!-- #-prefixed issue number(s), if any -->
| License       | MIT
| Doc PR        | n/a

As explained by @stof in https://github.com/symfony/symfony/pull/27735#discussion_r199797412, injecting the `ClassMetadataFactory` directly can lead to issues when resetting the EntityManager.

This PR deprecates this usage and encourages to inject the entity manager directly.

Commits
-------

3aab4a1270 [DoctrineBridge] Inject the entity manager instead of the class metadata factory in DoctrineExtractor
This commit is contained in:
Fabien Potencier 2018-07-13 22:22:21 +02:00
commit eb112a5288
4 changed files with 103 additions and 22 deletions

View File

@ -36,13 +36,18 @@ Console
$processHelper->run($output, Process::fromShellCommandline('ls -l')); $processHelper->run($output, Process::fromShellCommandline('ls -l'));
``` ```
DependencyInjection DependencyInjection
------------------- -------------------
* Removed the `TypedReference::canBeAutoregistered()` and `TypedReference::getRequiringClass()` methods. * Removed the `TypedReference::canBeAutoregistered()` and `TypedReference::getRequiringClass()` methods.
* Removed support for auto-discovered extension configuration class which does not implement `ConfigurationInterface`. * 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 EventDispatcher
--------------- ---------------

View File

@ -1,6 +1,12 @@
CHANGELOG CHANGELOG
========= =========
4.2.0
-----
* deprecated injecting `ClassMetadataFactory` in `DoctrineExtractor`,
an instance of `EntityManagerInterface` should be injected instead
4.1.0 4.1.0
----- -----

View File

@ -14,6 +14,7 @@ namespace Symfony\Bridge\Doctrine\PropertyInfo;
use Doctrine\Common\Persistence\Mapping\ClassMetadataFactory; use Doctrine\Common\Persistence\Mapping\ClassMetadataFactory;
use Doctrine\Common\Persistence\Mapping\MappingException; use Doctrine\Common\Persistence\Mapping\MappingException;
use Doctrine\DBAL\Types\Type as DBALType; use Doctrine\DBAL\Types\Type as DBALType;
use Doctrine\ORM\EntityManagerInterface;
use Doctrine\ORM\Mapping\ClassMetadataInfo; use Doctrine\ORM\Mapping\ClassMetadataInfo;
use Doctrine\ORM\Mapping\MappingException as OrmMappingException; use Doctrine\ORM\Mapping\MappingException as OrmMappingException;
use Symfony\Component\PropertyInfo\PropertyListExtractorInterface; use Symfony\Component\PropertyInfo\PropertyListExtractorInterface;
@ -27,11 +28,22 @@ use Symfony\Component\PropertyInfo\Type;
*/ */
class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeExtractorInterface class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeExtractorInterface
{ {
private $entityManager;
private $classMetadataFactory; 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()) public function getProperties($class, array $context = array())
{ {
try { try {
$metadata = $this->classMetadataFactory->getMetadataFor($class); $metadata = $this->entityManager ? $this->entityManager->getClassMetadata($class) : $this->classMetadataFactory->getMetadataFor($class);
} catch (MappingException $exception) { } catch (MappingException $exception) {
return; return;
} catch (OrmMappingException $exception) { } catch (OrmMappingException $exception) {
@ -66,7 +78,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE
public function getTypes($class, $property, array $context = array()) public function getTypes($class, $property, array $context = array())
{ {
try { try {
$metadata = $this->classMetadataFactory->getMetadataFor($class); $metadata = $this->entityManager ? $this->entityManager->getClassMetadata($class) : $this->classMetadataFactory->getMetadataFor($class);
} catch (MappingException $exception) { } catch (MappingException $exception) {
return; return;
} catch (OrmMappingException $exception) { } catch (OrmMappingException $exception) {
@ -96,7 +108,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE
if (isset($associationMapping['indexBy'])) { if (isset($associationMapping['indexBy'])) {
$indexProperty = $associationMapping['indexBy']; $indexProperty = $associationMapping['indexBy'];
/** @var ClassMetadataInfo $subMetadata */ /** @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); $typeOfField = $subMetadata->getTypeOfField($indexProperty);
if (null === $typeOfField) { if (null === $typeOfField) {
@ -104,7 +116,7 @@ class DoctrineExtractor implements PropertyListExtractorInterface, PropertyTypeE
/** @var ClassMetadataInfo $subMetadata */ /** @var ClassMetadataInfo $subMetadata */
$indexProperty = $subMetadata->getSingleAssociationReferencedJoinColumnName($indexProperty); $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); $typeOfField = $subMetadata->getTypeOfField($indexProperty);
} }

View File

@ -23,14 +23,9 @@ use Symfony\Component\PropertyInfo\Type;
*/ */
class DoctrineExtractorTest extends TestCase class DoctrineExtractorTest extends TestCase
{ {
/** private function createExtractor(bool $legacy = false)
* @var DoctrineExtractor
*/
private $extractor;
protected function setUp()
{ {
$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); $entityManager = EntityManager::create(array('driver' => 'pdo_sqlite'), $config);
if (!DBALType::hasType('foo')) { if (!DBALType::hasType('foo')) {
@ -38,10 +33,20 @@ class DoctrineExtractorTest extends TestCase
$entityManager->getConnection()->getDatabasePlatform()->registerDoctrineTypeMapping('custom_foo', 'foo'); $entityManager->getConnection()->getDatabasePlatform()->registerDoctrineTypeMapping('custom_foo', 'foo');
} }
$this->extractor = new DoctrineExtractor($entityManager->getMetadataFactory()); return new DoctrineExtractor($legacy ? $entityManager->getMetadataFactory() : $entityManager);
} }
public function testGetProperties() public function testGetProperties()
{
$this->doTestGetProperties(false);
}
public function testLegacyGetProperties()
{
$this->doTestGetProperties(true);
}
private function doTestGetProperties(bool $legacy)
{ {
$this->assertEquals( $this->assertEquals(
array( array(
@ -63,11 +68,21 @@ class DoctrineExtractorTest extends TestCase
'indexedBar', 'indexedBar',
'indexedFoo', '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')) { if (!class_exists('Doctrine\ORM\Mapping\Embedded')) {
$this->markTestSkipped('@Embedded is not available in Doctrine ORM lower than 2.5.'); $this->markTestSkipped('@Embedded is not available in Doctrine ORM lower than 2.5.');
@ -78,7 +93,7 @@ class DoctrineExtractorTest extends TestCase
'id', 'id',
'embedded', '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) 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() public function testExtractWithEmbedded()
{
$this->doTestExtractWithEmbedded(false);
}
public function testLegacyExtractWithEmbedded()
{
$this->doTestExtractWithEmbedded(true);
}
private function doTestExtractWithEmbedded(bool $legacy)
{ {
if (!class_exists('Doctrine\ORM\Mapping\Embedded')) { if (!class_exists('Doctrine\ORM\Mapping\Embedded')) {
$this->markTestSkipped('@Embedded is not available in Doctrine ORM lower than 2.5.'); $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' 'Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineEmbeddable'
)); ));
$actualTypes = $this->extractor->getTypes( $actualTypes = $this->createExtractor($legacy)->getTypes(
'Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineWithEmbedded', 'Symfony\Bridge\Doctrine\Tests\PropertyInfo\Fixtures\DoctrineWithEmbedded',
'embedded', 'embedded',
array() array()
@ -158,11 +196,31 @@ class DoctrineExtractorTest extends TestCase
public function testGetPropertiesCatchException() 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() 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'));
} }
} }