From 6098085238e83d1d9f9b5c8b2bda57e3a5645713 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Sun, 7 Apr 2019 11:44:21 +0200 Subject: [PATCH 1/8] Add ContainerEntityListenerResolver + load listeners lazy by default --- .../Compiler/EntityListenerPass.php | 44 ++++--- .../ContainerAwareEntityListenerResolver.php | 103 +--------------- Mapping/ContainerEntityListenerResolver.php | 111 ++++++++++++++++++ Resources/config/orm.xml | 2 +- .../AbstractDoctrineExtensionTest.php | 7 +- .../xml/orm_attach_lazy_entity_listener.xml | 5 +- .../yml/orm_attach_lazy_entity_listener.yml | 7 +- .../yml/orm_entity_listener_resolver.yml | 6 +- ...> ContainerEntityListenerResolverTest.php} | 12 +- 9 files changed, 169 insertions(+), 128 deletions(-) create mode 100644 Mapping/ContainerEntityListenerResolver.php rename Tests/Mapping/{ContainerAwareEntityListenerResolverTest.php => ContainerEntityListenerResolverTest.php} (90%) diff --git a/DependencyInjection/Compiler/EntityListenerPass.php b/DependencyInjection/Compiler/EntityListenerPass.php index ab0cc00d2..80578a18a 100644 --- a/DependencyInjection/Compiler/EntityListenerPass.php +++ b/DependencyInjection/Compiler/EntityListenerPass.php @@ -3,6 +3,7 @@ namespace Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; @@ -19,6 +20,8 @@ public function process(ContainerBuilder $container) { $resolvers = $container->findTaggedServiceIds('doctrine.orm.entity_listener'); + $lazyServiceReferencesByResolver = []; + foreach ($resolvers as $id => $tagAttributes) { foreach ($tagAttributes as $attributes) { $name = isset($attributes['entity_manager']) ? $attributes['entity_manager'] : $container->getParameter('doctrine.default_entity_manager'); @@ -41,35 +44,44 @@ public function process(ContainerBuilder $container) $this->attachToListener($container, $name, $id, $attributes); } - if (isset($attributes['lazy']) && $attributes['lazy']) { + $interface = 'Doctrine\\Bundle\\DoctrineBundle\\Mapping\\EntityListenerServiceResolver'; + $class = $resolver->getClass(); + + if (substr($class, 0, 1) === '%') { + // resolve container parameter first + $class = $container->getParameterBag()->resolveValue($resolver->getClass()); + } + $resolverSupportsLazyListeners = is_a($class, $interface, true); + + $lazyByAttribute = isset($attributes['lazy']) && $attributes['lazy']; + if ($lazyByAttribute && ! $resolverSupportsLazyListeners) { + throw new InvalidArgumentException( + sprintf('Lazy-loaded entity listeners can only be resolved by a resolver implementing %s.', $interface) + ); + } + + if (! isset($attributes['lazy']) && $resolverSupportsLazyListeners || $lazyByAttribute) { $listener = $container->findDefinition($id); if ($listener->isAbstract()) { throw new InvalidArgumentException(sprintf('The service "%s" must not be abstract as this entity listener is lazy-loaded.', $id)); } - $interface = 'Doctrine\\Bundle\\DoctrineBundle\\Mapping\\EntityListenerServiceResolver'; - $class = $resolver->getClass(); - - if (substr($class, 0, 1) === '%') { - // resolve container parameter first - $class = $container->getParameterBag()->resolveValue($resolver->getClass()); - } + $resolver->addMethodCall('registerService', [$listener->getClass(), $id]); - if (! is_a($class, $interface, true)) { - throw new InvalidArgumentException( - sprintf('Lazy-loaded entity listeners can only be resolved by a resolver implementing %s.', $interface) - ); + if (! isset($lazyServiceReferencesByResolver[$resolverId])) { + $lazyServiceReferencesByResolver[$resolverId] = []; } - - $listener->setPublic(true); - - $resolver->addMethodCall('registerService', [$listener->getClass(), $id]); + $lazyServiceReferencesByResolver[$resolverId][$id] = new Reference($id); } else { $resolver->addMethodCall('register', [new Reference($id)]); } } } + + foreach ($lazyServiceReferencesByResolver as $resolverId => $listenerReferences) { + $container->findDefinition($resolverId)->replaceArgument(0, ServiceLocatorTagPass::register($container, $listenerReferences)); + } } private function attachToListener(ContainerBuilder $container, $name, $id, array $attributes) diff --git a/Mapping/ContainerAwareEntityListenerResolver.php b/Mapping/ContainerAwareEntityListenerResolver.php index 4345015b7..f56f90e04 100644 --- a/Mapping/ContainerAwareEntityListenerResolver.php +++ b/Mapping/ContainerAwareEntityListenerResolver.php @@ -2,107 +2,16 @@ namespace Doctrine\Bundle\DoctrineBundle\Mapping; -use InvalidArgumentException; -use RuntimeException; use Symfony\Component\DependencyInjection\ContainerInterface; -class ContainerAwareEntityListenerResolver implements EntityListenerServiceResolver +/** + * @deprecated since 1.11 and will be removed in 2.0. Use ContainerEntityListenerResolver instead. + */ +class ContainerAwareEntityListenerResolver extends ContainerEntityListenerResolver { - /** @var ContainerInterface */ - private $container; - - /** @var object[] Map to store entity listener instances. */ - private $instances = []; - - /** @var string[] Map to store registered service ids */ - private $serviceIds = []; - public function __construct(ContainerInterface $container) { - $this->container = $container; - } - - /** - * {@inheritdoc} - */ - public function clear($className = null) - { - if ($className === null) { - $this->instances = []; - - return; - } - - $className = $this->normalizeClassName($className); - - if (! isset($this->instances[$className])) { - return; - } - - unset($this->instances[$className]); - } - - /** - * {@inheritdoc} - */ - public function register($object) - { - if (! is_object($object)) { - throw new InvalidArgumentException(sprintf('An object was expected, but got "%s".', gettype($object))); - } - - $className = $this->normalizeClassName(get_class($object)); - - $this->instances[$className] = $object; - } - - /** - * {@inheritdoc} - */ - public function registerService($className, $serviceId) - { - $this->serviceIds[$this->normalizeClassName($className)] = $serviceId; - } - - /** - * {@inheritdoc} - */ - public function resolve($className) - { - $className = $this->normalizeClassName($className); - - if (! isset($this->instances[$className])) { - if (isset($this->serviceIds[$className])) { - $this->instances[$className] = $this->resolveService($this->serviceIds[$className]); - } else { - $this->instances[$className] = new $className(); - } - } - - return $this->instances[$className]; - } - - /** - * @param string $serviceId - * - * @return object - */ - private function resolveService($serviceId) - { - if (! $this->container->has($serviceId)) { - throw new RuntimeException(sprintf('There is no service named "%s"', $serviceId)); - } - - return $this->container->get($serviceId); - } - - /** - * @param string $className - * - * @return string - */ - private function normalizeClassName($className) - { - return trim($className, '\\'); + @trigger_error(sprintf('The class "%s" is deprecated since 1.11 and will be removed in 2.0 Use "%s" instead.', self::class, 'Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver'), E_USER_DEPRECATED); + parent::__construct($container); } } diff --git a/Mapping/ContainerEntityListenerResolver.php b/Mapping/ContainerEntityListenerResolver.php new file mode 100644 index 000000000..2dcc8b28a --- /dev/null +++ b/Mapping/ContainerEntityListenerResolver.php @@ -0,0 +1,111 @@ +container = $container; + } + + /** + * {@inheritdoc} + */ + public function clear($className = null) + { + if ($className === null) { + $this->instances = []; + + return; + } + + $className = $this->normalizeClassName($className); + + if (! isset($this->instances[$className])) { + return; + } + + unset($this->instances[$className]); + } + + /** + * {@inheritdoc} + */ + public function register($object) + { + if (! is_object($object)) { + throw new InvalidArgumentException(sprintf('An object was expected, but got "%s".', gettype($object))); + } + + $className = $this->normalizeClassName(get_class($object)); + + $this->instances[$className] = $object; + } + + /** + * {@inheritdoc} + */ + public function registerService($className, $serviceId) + { + $this->serviceIds[$this->normalizeClassName($className)] = $serviceId; + } + + /** + * {@inheritdoc} + */ + public function resolve($className) + { + $className = $this->normalizeClassName($className); + + if (! isset($this->instances[$className])) { + if (isset($this->serviceIds[$className])) { + $this->instances[$className] = $this->resolveService($this->serviceIds[$className]); + } else { + $this->instances[$className] = new $className(); + } + } + + return $this->instances[$className]; + } + + /** + * @param string $serviceId + * + * @return object + */ + private function resolveService($serviceId) + { + if (! $this->container->has($serviceId)) { + throw new RuntimeException(sprintf('There is no service named "%s"', $serviceId)); + } + + return $this->container->get($serviceId); + } + + /** + * @param string $className + * + * @return string + */ + private function normalizeClassName($className) + { + return trim($className, '\\'); + } +} diff --git a/Resources/config/orm.xml b/Resources/config/orm.xml index 0fe60c165..ab9dd5bbe 100644 --- a/Resources/config/orm.xml +++ b/Resources/config/orm.xml @@ -62,7 +62,7 @@ Doctrine\ORM\Mapping\AnsiQuoteStrategy - Doctrine\Bundle\DoctrineBundle\Mapping\ContainerAwareEntityListenerResolver + Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver Doctrine\ORM\Cache\DefaultCacheFactory diff --git a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php index 6f8793df4..c6bf000c6 100644 --- a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php +++ b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php @@ -831,7 +831,7 @@ public function testEntityListenerResolver() $this->assertDICDefinitionMethodCallOnce($definition, 'setEntityListenerResolver', [new Reference('doctrine.orm.em2_entity_listener_resolver')]); $listener = $container->getDefinition('doctrine.orm.em1_entity_listener_resolver'); - $this->assertDICDefinitionMethodCallOnce($listener, 'register', [new Reference('entity_listener1')]); + $this->assertDICDefinitionMethodCallOnce($listener, 'registerService', ['EntityListener', 'entity_listener1']); $listener = $container->getDefinition('entity_listener_resolver'); $this->assertDICDefinitionMethodCallOnce($listener, 'register', [new Reference('entity_listener2')]); @@ -849,10 +849,10 @@ public function testAttachEntityListenerTag() $this->compileContainer($container); $listener = $container->getDefinition('doctrine.orm.em1_entity_listener_resolver'); - $this->assertDICDefinitionMethodCallOnce($listener, 'register', [new Reference('entity_listener1')]); + $this->assertDICDefinitionMethodCallOnce($listener, 'registerService', ['EntityListener1', 'entity_listener1']); $listener = $container->getDefinition('doctrine.orm.em2_entity_listener_resolver'); - $this->assertDICDefinitionMethodCallOnce($listener, 'register', [new Reference('entity_listener2')]); + $this->assertDICDefinitionMethodCallOnce($listener, 'registerService', ['EntityListener2', 'entity_listener2']); $attachListener = $container->getDefinition('doctrine.orm.em1_listeners.attach_entity_listeners'); $this->assertDICDefinitionMethodCallOnce($attachListener, 'addEntityListener', ['My/Entity1', 'EntityListener1', 'postLoad']); @@ -894,6 +894,7 @@ public function testAttachLazyEntityListener() $resolver1 = $container->getDefinition('doctrine.orm.em1_entity_listener_resolver'); $this->assertDICDefinitionMethodCallOnce($resolver1, 'registerService', ['EntityListener1', 'entity_listener1']); + $this->assertDICDefinitionMethodCallOnce($resolver1, 'register', [new Reference('entity_listener3')]); $resolver2 = $container->findDefinition('custom_entity_listener_resolver'); $this->assertDICDefinitionMethodCallOnce($resolver2, 'registerService', ['EntityListener2', 'entity_listener2']); diff --git a/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml b/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml index 65358d08f..60a4bf1b1 100644 --- a/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml +++ b/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml @@ -7,7 +7,7 @@ http://symfony.com/schema/dic/doctrine http://symfony.com/schema/dic/doctrine/doctrine-1.0.xsd"> - + @@ -17,6 +17,9 @@ + + + diff --git a/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml b/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml index 530186187..ecc08c055 100644 --- a/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml +++ b/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml @@ -1,6 +1,6 @@ services: custom_entity_listener_resolver: - class: Doctrine\Bundle\DoctrineBundle\Mapping\ContainerAwareEntityListenerResolver + class: Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver arguments: - '@service_container' @@ -14,6 +14,11 @@ services: tags: - { name: doctrine.orm.entity_listener, entity_manager: em2, lazy: true } + entity_listener3: + class: EntityListener3 + tags: + - { name: doctrine.orm.entity_listener, lazy: false } + doctrine: dbal: default_connection: default diff --git a/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_resolver.yml b/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_resolver.yml index 2d93aeeae..bab9df4a2 100644 --- a/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_resolver.yml +++ b/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_resolver.yml @@ -1,14 +1,14 @@ services: entity_listener_resolver: - class: \Doctrine\ORM\Mapping\DefaultEntityListenerResolver + class: Doctrine\ORM\Mapping\DefaultEntityListenerResolver entity_listener1: - class: \EntityListener + class: EntityListener tags: - { name: doctrine.orm.entity_listener } entity_listener2: - class: \EntityListener + class: EntityListener tags: - { name: doctrine.orm.entity_listener, entity_manager: em2 } diff --git a/Tests/Mapping/ContainerAwareEntityListenerResolverTest.php b/Tests/Mapping/ContainerEntityListenerResolverTest.php similarity index 90% rename from Tests/Mapping/ContainerAwareEntityListenerResolverTest.php rename to Tests/Mapping/ContainerEntityListenerResolverTest.php index 696c77419..f6fa7bb7c 100644 --- a/Tests/Mapping/ContainerAwareEntityListenerResolverTest.php +++ b/Tests/Mapping/ContainerEntityListenerResolverTest.php @@ -2,14 +2,14 @@ namespace Doctrine\Bundle\DoctrineBundle\Tests\Mapping; -use Doctrine\Bundle\DoctrineBundle\Mapping\ContainerAwareEntityListenerResolver; +use Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver; use PHPUnit\Framework\TestCase; use PHPUnit_Framework_MockObject_MockObject; -use Symfony\Component\DependencyInjection\ContainerInterface; +use Psr\Container\ContainerInterface; -class ContainerAwareEntityListenerResolverTest extends TestCase +class ContainerEntityListenerResolverTest extends TestCase { - /** @var ContainerAwareEntityListenerResolver */ + /** @var ContainerEntityListenerResolver */ private $resolver; /** @var ContainerInterface|PHPUnit_Framework_MockObject_MockObject */ @@ -19,8 +19,8 @@ protected function setUp() { parent::setUp(); - $this->container = $this->getMockForAbstractClass('\Symfony\Component\DependencyInjection\ContainerInterface'); - $this->resolver = new ContainerAwareEntityListenerResolver($this->container); + $this->container = $this->createMock(ContainerInterface::class); + $this->resolver = new ContainerEntityListenerResolver($this->container); } public function testResolveClass() From 01ad487648a94b3c5c823dcf511ea410fe66ace1 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Sun, 7 Apr 2019 14:35:13 +0200 Subject: [PATCH 2/8] Code review changes --- DependencyInjection/Compiler/EntityListenerPass.php | 8 ++++---- Mapping/ContainerAwareEntityListenerResolver.php | 2 +- Mapping/ContainerEntityListenerResolver.php | 4 ---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/DependencyInjection/Compiler/EntityListenerPass.php b/DependencyInjection/Compiler/EntityListenerPass.php index 80578a18a..632e2ee4b 100644 --- a/DependencyInjection/Compiler/EntityListenerPass.php +++ b/DependencyInjection/Compiler/EntityListenerPass.php @@ -2,6 +2,7 @@ namespace Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler; +use Doctrine\Bundle\DoctrineBundle\Mapping\EntityListenerServiceResolver; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -44,19 +45,18 @@ public function process(ContainerBuilder $container) $this->attachToListener($container, $name, $id, $attributes); } - $interface = 'Doctrine\\Bundle\\DoctrineBundle\\Mapping\\EntityListenerServiceResolver'; - $class = $resolver->getClass(); + $class = $resolver->getClass(); if (substr($class, 0, 1) === '%') { // resolve container parameter first $class = $container->getParameterBag()->resolveValue($resolver->getClass()); } - $resolverSupportsLazyListeners = is_a($class, $interface, true); + $resolverSupportsLazyListeners = is_a($class, EntityListenerServiceResolver::class, true); $lazyByAttribute = isset($attributes['lazy']) && $attributes['lazy']; if ($lazyByAttribute && ! $resolverSupportsLazyListeners) { throw new InvalidArgumentException( - sprintf('Lazy-loaded entity listeners can only be resolved by a resolver implementing %s.', $interface) + sprintf('Lazy-loaded entity listeners can only be resolved by a resolver implementing %s.', EntityListenerServiceResolver::class) ); } diff --git a/Mapping/ContainerAwareEntityListenerResolver.php b/Mapping/ContainerAwareEntityListenerResolver.php index f56f90e04..77ed6cae1 100644 --- a/Mapping/ContainerAwareEntityListenerResolver.php +++ b/Mapping/ContainerAwareEntityListenerResolver.php @@ -11,7 +11,7 @@ class ContainerAwareEntityListenerResolver extends ContainerEntityListenerResolv { public function __construct(ContainerInterface $container) { - @trigger_error(sprintf('The class "%s" is deprecated since 1.11 and will be removed in 2.0 Use "%s" instead.', self::class, 'Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver'), E_USER_DEPRECATED); + @trigger_error(sprintf('The class "%s" is deprecated since 1.11 and will be removed in 2.0. Use "%s" instead.', self::class, 'Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver'), E_USER_DEPRECATED); parent::__construct($container); } } diff --git a/Mapping/ContainerEntityListenerResolver.php b/Mapping/ContainerEntityListenerResolver.php index 2dcc8b28a..1eb26f0ee 100644 --- a/Mapping/ContainerEntityListenerResolver.php +++ b/Mapping/ContainerEntityListenerResolver.php @@ -38,10 +38,6 @@ public function clear($className = null) $className = $this->normalizeClassName($className); - if (! isset($this->instances[$className])) { - return; - } - unset($this->instances[$className]); } From 8f53a8bfd9f81c3e3bfc7e4c3b1ba5d6b6236ff6 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Mon, 8 Apr 2019 12:15:31 +0200 Subject: [PATCH 3/8] review changes --- Mapping/ContainerAwareEntityListenerResolver.php | 7 ------- Mapping/ContainerEntityListenerResolver.php | 12 ++---------- 2 files changed, 2 insertions(+), 17 deletions(-) diff --git a/Mapping/ContainerAwareEntityListenerResolver.php b/Mapping/ContainerAwareEntityListenerResolver.php index 77ed6cae1..635708e2c 100644 --- a/Mapping/ContainerAwareEntityListenerResolver.php +++ b/Mapping/ContainerAwareEntityListenerResolver.php @@ -2,16 +2,9 @@ namespace Doctrine\Bundle\DoctrineBundle\Mapping; -use Symfony\Component\DependencyInjection\ContainerInterface; - /** * @deprecated since 1.11 and will be removed in 2.0. Use ContainerEntityListenerResolver instead. */ class ContainerAwareEntityListenerResolver extends ContainerEntityListenerResolver { - public function __construct(ContainerInterface $container) - { - @trigger_error(sprintf('The class "%s" is deprecated since 1.11 and will be removed in 2.0. Use "%s" instead.', self::class, 'Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver'), E_USER_DEPRECATED); - parent::__construct($container); - } } diff --git a/Mapping/ContainerEntityListenerResolver.php b/Mapping/ContainerEntityListenerResolver.php index 1eb26f0ee..e43ff39a5 100644 --- a/Mapping/ContainerEntityListenerResolver.php +++ b/Mapping/ContainerEntityListenerResolver.php @@ -8,7 +8,6 @@ class ContainerEntityListenerResolver implements EntityListenerServiceResolver { - /** @var ContainerInterface */ private $container; /** @var object[] Map to store entity listener instances. */ @@ -82,11 +81,9 @@ public function resolve($className) } /** - * @param string $serviceId - * * @return object */ - private function resolveService($serviceId) + private function resolveService(string $serviceId) { if (! $this->container->has($serviceId)) { throw new RuntimeException(sprintf('There is no service named "%s"', $serviceId)); @@ -95,12 +92,7 @@ private function resolveService($serviceId) return $this->container->get($serviceId); } - /** - * @param string $className - * - * @return string - */ - private function normalizeClassName($className) + private function normalizeClassName(string $className): string { return trim($className, '\\'); } From 3b894aff95657599fbaa8a652c113b256d214bec Mon Sep 17 00:00:00 2001 From: David Maicher Date: Mon, 8 Apr 2019 12:17:16 +0200 Subject: [PATCH 4/8] review changes --- DependencyInjection/Compiler/EntityListenerPass.php | 7 ++++--- Mapping/ContainerAwareEntityListenerResolver.php | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/DependencyInjection/Compiler/EntityListenerPass.php b/DependencyInjection/Compiler/EntityListenerPass.php index 632e2ee4b..f15fce762 100644 --- a/DependencyInjection/Compiler/EntityListenerPass.php +++ b/DependencyInjection/Compiler/EntityListenerPass.php @@ -55,9 +55,10 @@ public function process(ContainerBuilder $container) $lazyByAttribute = isset($attributes['lazy']) && $attributes['lazy']; if ($lazyByAttribute && ! $resolverSupportsLazyListeners) { - throw new InvalidArgumentException( - sprintf('Lazy-loaded entity listeners can only be resolved by a resolver implementing %s.', EntityListenerServiceResolver::class) - ); + throw new InvalidArgumentException(sprintf( + 'Lazy-loaded entity listeners can only be resolved by a resolver implementing %s.', + EntityListenerServiceResolver::class + )); } if (! isset($attributes['lazy']) && $resolverSupportsLazyListeners || $lazyByAttribute) { diff --git a/Mapping/ContainerAwareEntityListenerResolver.php b/Mapping/ContainerAwareEntityListenerResolver.php index 635708e2c..89f8320d7 100644 --- a/Mapping/ContainerAwareEntityListenerResolver.php +++ b/Mapping/ContainerAwareEntityListenerResolver.php @@ -3,7 +3,7 @@ namespace Doctrine\Bundle\DoctrineBundle\Mapping; /** - * @deprecated since 1.11 and will be removed in 2.0. Use ContainerEntityListenerResolver instead. + * @deprecated since DoctrineBundle 1.11 and will be removed in 2.0. Use ContainerEntityListenerResolver instead. */ class ContainerAwareEntityListenerResolver extends ContainerEntityListenerResolver { From 05d277d1c2f0abd3037f3c305b91d9372365a906 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Mon, 8 Apr 2019 12:23:30 +0200 Subject: [PATCH 5/8] fix phpcs --- Mapping/ContainerEntityListenerResolver.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Mapping/ContainerEntityListenerResolver.php b/Mapping/ContainerEntityListenerResolver.php index e43ff39a5..02eee1b62 100644 --- a/Mapping/ContainerEntityListenerResolver.php +++ b/Mapping/ContainerEntityListenerResolver.php @@ -8,6 +8,7 @@ class ContainerEntityListenerResolver implements EntityListenerServiceResolver { + /** @var ContainerInterface */ private $container; /** @var object[] Map to store entity listener instances. */ @@ -92,7 +93,7 @@ private function resolveService(string $serviceId) return $this->container->get($serviceId); } - private function normalizeClassName(string $className): string + private function normalizeClassName(string $className) : string { return trim($className, '\\'); } From 2c526bc4666d5156cf135e66d18dd349574fc001 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Mon, 8 Apr 2019 20:19:06 +0200 Subject: [PATCH 6/8] add more tests --- .../AbstractDoctrineExtensionTest.php | 15 +++++++++++++-- .../xml/orm_attach_lazy_entity_listener.xml | 3 +++ .../yml/orm_attach_lazy_entity_listener.yml | 5 +++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php index c6bf000c6..d61901120 100644 --- a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php +++ b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php @@ -16,6 +16,7 @@ use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag; use Symfony\Component\DependencyInjection\Reference; +use Symfony\Component\DependencyInjection\ServiceLocator; abstract class AbstractDoctrineExtensionTest extends TestCase { @@ -893,8 +894,16 @@ public function testAttachLazyEntityListener() $this->compileContainer($container); $resolver1 = $container->getDefinition('doctrine.orm.em1_entity_listener_resolver'); - $this->assertDICDefinitionMethodCallOnce($resolver1, 'registerService', ['EntityListener1', 'entity_listener1']); - $this->assertDICDefinitionMethodCallOnce($resolver1, 'register', [new Reference('entity_listener3')]); + $this->assertDICDefinitionMethodCallAt(0, $resolver1, 'registerService', ['EntityListener1', 'entity_listener1']); + $this->assertDICDefinitionMethodCallAt(1, $resolver1, 'register', [new Reference('entity_listener3')]); + $this->assertDICDefinitionMethodCallAt(2, $resolver1, 'registerService', ['EntityListener4', 'entity_listener4']); + + $serviceLocatorReference = $resolver1->getArgument(0); + $this->assertInstanceOf(Reference::class, $serviceLocatorReference); + $serviceLocatorDefinition = $container->getDefinition((string) $serviceLocatorReference); + $this->assertSame(ServiceLocator::class, $serviceLocatorDefinition->getClass()); + $serviceLocatorMap = $serviceLocatorDefinition->getArgument(0); + $this->assertSame(['entity_listener1', 'entity_listener4'], array_keys($serviceLocatorMap)); $resolver2 = $container->findDefinition('custom_entity_listener_resolver'); $this->assertDICDefinitionMethodCallOnce($resolver2, 'registerService', ['EntityListener2', 'entity_listener2']); @@ -1008,6 +1017,8 @@ private function assertDICDefinitionMethodCallAt($pos, Definition $definition, $ { $calls = $definition->getMethodCalls(); if (! isset($calls[$pos][0])) { + $this->fail(sprintf('Method call at position %s not found!', $pos)); + return; } diff --git a/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml b/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml index 60a4bf1b1..1da2f822d 100644 --- a/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml +++ b/Tests/DependencyInjection/Fixtures/config/xml/orm_attach_lazy_entity_listener.xml @@ -20,6 +20,9 @@ + + + diff --git a/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml b/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml index ecc08c055..f312827e9 100644 --- a/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml +++ b/Tests/DependencyInjection/Fixtures/config/yml/orm_attach_lazy_entity_listener.yml @@ -19,6 +19,11 @@ services: tags: - { name: doctrine.orm.entity_listener, lazy: false } + entity_listener4: + class: EntityListener4 + tags: + - { name: doctrine.orm.entity_listener } + doctrine: dbal: default_connection: default From 2d99dde0cfcc7465aa7d7179c48744c2331ce294 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 9 Apr 2019 17:16:28 +0200 Subject: [PATCH 7/8] avoid BC break when using custom resolver classes --- .../Compiler/EntityListenerPass.php | 36 +++++++++----- .../AbstractDoctrineExtensionTest.php | 18 +++++++ .../CustomEntityListenerServiceResolver.php | 48 +++++++++++++++++++ .../orm_entity_listener_custom_resolver.xml | 26 ++++++++++ .../orm_entity_listener_custom_resolver.yml | 23 +++++++++ 5 files changed, 140 insertions(+), 11 deletions(-) create mode 100644 Tests/DependencyInjection/Fixtures/CustomEntityListenerServiceResolver.php create mode 100644 Tests/DependencyInjection/Fixtures/config/xml/orm_entity_listener_custom_resolver.xml create mode 100644 Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_custom_resolver.yml diff --git a/DependencyInjection/Compiler/EntityListenerPass.php b/DependencyInjection/Compiler/EntityListenerPass.php index f15fce762..f1eff8fe5 100644 --- a/DependencyInjection/Compiler/EntityListenerPass.php +++ b/DependencyInjection/Compiler/EntityListenerPass.php @@ -2,10 +2,12 @@ namespace Doctrine\Bundle\DoctrineBundle\DependencyInjection\Compiler; +use Doctrine\Bundle\DoctrineBundle\Mapping\ContainerEntityListenerResolver; use Doctrine\Bundle\DoctrineBundle\Mapping\EntityListenerServiceResolver; use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; use Symfony\Component\DependencyInjection\Reference; @@ -45,13 +47,8 @@ public function process(ContainerBuilder $container) $this->attachToListener($container, $name, $id, $attributes); } - $class = $resolver->getClass(); - - if (substr($class, 0, 1) === '%') { - // resolve container parameter first - $class = $container->getParameterBag()->resolveValue($resolver->getClass()); - } - $resolverSupportsLazyListeners = is_a($class, EntityListenerServiceResolver::class, true); + $resolverClass = $this->getResolverClass($resolver, $container); + $resolverSupportsLazyListeners = is_a($resolverClass, EntityListenerServiceResolver::class, true); $lazyByAttribute = isset($attributes['lazy']) && $attributes['lazy']; if ($lazyByAttribute && ! $resolverSupportsLazyListeners) { @@ -70,10 +67,15 @@ public function process(ContainerBuilder $container) $resolver->addMethodCall('registerService', [$listener->getClass(), $id]); - if (! isset($lazyServiceReferencesByResolver[$resolverId])) { - $lazyServiceReferencesByResolver[$resolverId] = []; + // if the resolver uses the default class we will use a service locator for all listeners + if ($resolverClass === ContainerEntityListenerResolver::class) { + if (! isset($lazyServiceReferencesByResolver[$resolverId])) { + $lazyServiceReferencesByResolver[$resolverId] = []; + } + $lazyServiceReferencesByResolver[$resolverId][$id] = new Reference($id); + } else { + $listener->setPublic(true); } - $lazyServiceReferencesByResolver[$resolverId][$id] = new Reference($id); } else { $resolver->addMethodCall('register', [new Reference($id)]); } @@ -81,7 +83,7 @@ public function process(ContainerBuilder $container) } foreach ($lazyServiceReferencesByResolver as $resolverId => $listenerReferences) { - $container->findDefinition($resolverId)->replaceArgument(0, ServiceLocatorTagPass::register($container, $listenerReferences)); + $container->findDefinition($resolverId)->setArgument(0, ServiceLocatorTagPass::register($container, $listenerReferences)); } } @@ -107,4 +109,16 @@ private function attachToListener(ContainerBuilder $container, $name, $id, array $container->findDefinition($listenerId)->addMethodCall('addEntityListener', $args); } + + private function getResolverClass(Definition $resolver, ContainerBuilder $container) : string + { + $resolverClass = $resolver->getClass(); + + if (substr($resolverClass, 0, 1) === '%') { + // resolve container parameter first + $resolverClass = $container->getParameterBag()->resolveValue($resolver->getClass()); + } + + return $resolverClass; + } } diff --git a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php index d61901120..a26437388 100644 --- a/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php +++ b/Tests/DependencyInjection/AbstractDoctrineExtensionTest.php @@ -909,6 +909,24 @@ public function testAttachLazyEntityListener() $this->assertDICDefinitionMethodCallOnce($resolver2, 'registerService', ['EntityListener2', 'entity_listener2']); } + public function testAttachLazyEntityListenerForCustomResolver() + { + $container = $this->getContainer([]); + $loader = new DoctrineExtension(); + $container->registerExtension($loader); + $container->addCompilerPass(new EntityListenerPass()); + + $this->loadFromFile($container, 'orm_entity_listener_custom_resolver'); + + $this->compileContainer($container); + + $resolver = $container->getDefinition('custom_entity_listener_resolver'); + $this->assertTrue($resolver->isPublic()); + $this->assertEmpty($resolver->getArguments(), 'We must not change the arguments for custom services.'); + $this->assertDICDefinitionMethodCallOnce($resolver, 'registerService', ['EntityListener', 'entity_listener']); + $this->assertTrue($container->getDefinition('entity_listener')->isPublic()); + } + /** * @expectedException \InvalidArgumentException * @expectedExceptionMessage EntityListenerServiceResolver diff --git a/Tests/DependencyInjection/Fixtures/CustomEntityListenerServiceResolver.php b/Tests/DependencyInjection/Fixtures/CustomEntityListenerServiceResolver.php new file mode 100644 index 000000000..76a43a279 --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/CustomEntityListenerServiceResolver.php @@ -0,0 +1,48 @@ +resolver = $resolver; + } + + /** + * {@inheritdoc} + */ + public function clear($className = null) + { + $this->resolver->clear($className); + } + + /** + * {@inheritdoc} + */ + public function resolve($className) + { + return $this->resolver->resolve($className); + } + + /** + * {@inheritdoc} + */ + public function register($object) + { + $this->resolver->register($object); + } + + /** + * {@inheritdoc} + */ + public function registerService($className, $serviceId) + { + $this->resolver->registerService($className, $serviceId); + } +} diff --git a/Tests/DependencyInjection/Fixtures/config/xml/orm_entity_listener_custom_resolver.xml b/Tests/DependencyInjection/Fixtures/config/xml/orm_entity_listener_custom_resolver.xml new file mode 100644 index 000000000..69734f4e4 --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/config/xml/orm_entity_listener_custom_resolver.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + diff --git a/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_custom_resolver.yml b/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_custom_resolver.yml new file mode 100644 index 000000000..32c705feb --- /dev/null +++ b/Tests/DependencyInjection/Fixtures/config/yml/orm_entity_listener_custom_resolver.yml @@ -0,0 +1,23 @@ +services: + custom_entity_listener_resolver: + class: Doctrine\Bundle\DoctrineBundle\Tests\DependencyInjection\Fixtures\CustomEntityListenerServiceResolver + public: false + + entity_listener: + class: EntityListener + public: false + tags: + - { name: doctrine.orm.entity_listener } + +doctrine: + dbal: + default_connection: default + connections: + default: + dbname: db + + orm: + default_entity_manager: em1 + entity_managers: + em1: + entity_listener_resolver: custom_entity_listener_resolver From 2926cd41c9f84d2f59ff55f8aa6a13c0bacd5b51 Mon Sep 17 00:00:00 2001 From: David Maicher Date: Tue, 9 Apr 2019 17:19:40 +0200 Subject: [PATCH 8/8] make class final --- Mapping/ContainerEntityListenerResolver.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/Mapping/ContainerEntityListenerResolver.php b/Mapping/ContainerEntityListenerResolver.php index 02eee1b62..9645838d8 100644 --- a/Mapping/ContainerEntityListenerResolver.php +++ b/Mapping/ContainerEntityListenerResolver.php @@ -6,6 +6,9 @@ use Psr\Container\ContainerInterface; use RuntimeException; +/** + * @final + */ class ContainerEntityListenerResolver implements EntityListenerServiceResolver { /** @var ContainerInterface */