Skip to content

[DoctrineBridge] Make subscriber and listeners prioritizable #39978

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 5, 2021
Merged

[DoctrineBridge] Make subscriber and listeners prioritizable #39978

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 37 additions & 12 deletions src/Symfony/Bridge/Doctrine/ContainerAwareEventManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

use Doctrine\Common\EventArgs;
use Doctrine\Common\EventManager;
use Doctrine\Common\EventSubscriber;
use Psr\Container\ContainerInterface;

/**
Expand All @@ -34,6 +35,9 @@ class ContainerAwareEventManager extends EventManager
private $methods = [];
private $container;

/**
* @param list<string|EventSubscriber|array{string[], string|object}> $subscriberIds List of subscribers, subscriber ids, or [events, listener] tuples
*/
public function __construct(ContainerInterface $container, array $subscriberIds = [])
{
$this->container = $container;
Expand Down Expand Up @@ -113,6 +117,10 @@ public function hasListeners($event)
*/
public function addEventListener($events, $listener)
{
if (!$this->initializedSubscribers) {
$this->initializeSubscribers();
}

$hash = $this->getHash($listener);

foreach ((array) $events as $event) {
Expand All @@ -135,6 +143,10 @@ public function addEventListener($events, $listener)
*/
public function removeEventListener($events, $listener)
{
if (!$this->initializedSubscribers) {
$this->initializeSubscribers();
}

$hash = $this->getHash($listener);

foreach ((array) $events as $event) {
Expand All @@ -149,6 +161,24 @@ public function removeEventListener($events, $listener)
}
}

public function addEventSubscriber(EventSubscriber $subscriber): void
{
if (!$this->initializedSubscribers) {
$this->initializeSubscribers();
}

parent::addEventSubscriber($subscriber);
}

public function removeEventSubscriber(EventSubscriber $subscriber): void
{
if (!$this->initializedSubscribers) {
$this->initializeSubscribers();
}

parent::removeEventSubscriber($subscriber);
}

private function initializeListeners(string $eventName)
{
$this->initialized[$eventName] = true;
Expand All @@ -164,20 +194,15 @@ private function initializeListeners(string $eventName)
private function initializeSubscribers()
{
$this->initializedSubscribers = true;

$eventListeners = $this->listeners;
// reset eventListener to respect priority: EventSubscribers have a higher priority
$this->listeners = [];
foreach ($this->subscribers as $id => $subscriber) {
if (\is_string($subscriber)) {
parent::addEventSubscriber($this->subscribers[$id] = $this->container->get($subscriber));
foreach ($this->subscribers as $subscriber) {
if (\is_array($subscriber)) {
$this->addEventListener(...$subscriber);
continue;
}
}
foreach ($eventListeners as $event => $listeners) {
if (!isset($this->listeners[$event])) {
$this->listeners[$event] = [];
if (\is_string($subscriber)) {
$subscriber = $this->container->get($subscriber);
}
$this->listeners[$event] += $listeners;
parent::addEventSubscriber($subscriber);
}
$this->subscribers = [];
}
Expand Down
67 changes: 29 additions & 38 deletions ...ge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,9 +57,7 @@ public function process(ContainerBuilder $container)
}

$this->connections = $container->getParameter($this->connections);
$listenerRefs = [];
$this->addTaggedSubscribers($container, $listenerRefs);
$this->addTaggedListeners($container, $listenerRefs);
$listenerRefs = $this->addTaggedServices($container);

// replace service container argument of event managers with smaller service locator
// so services can even remain private
Expand All @@ -69,15 +67,20 @@ public function process(ContainerBuilder $container)
}
}

private function addTaggedSubscribers(ContainerBuilder $container, array &$listenerRefs)
private function addTaggedServices(ContainerBuilder $container): array
{
$listenerTag = $this->tagPrefix.'.event_listener';
$subscriberTag = $this->tagPrefix.'.event_subscriber';
$taggedSubscribers = $this->findAndSortTags($subscriberTag, $container);
$listenerRefs = [];
$taggedServices = $this->findAndSortTags([$subscriberTag, $listenerTag], $container);

$managerDefs = [];
foreach ($taggedSubscribers as $taggedSubscriber) {
[$id, $tag] = $taggedSubscriber;
foreach ($taggedServices as $taggedSubscriber) {
[$tagName, $id, $tag] = $taggedSubscriber;
$connections = isset($tag['connection']) ? [$tag['connection']] : array_keys($this->connections);
if ($listenerTag === $tagName && !isset($tag['event'])) {
throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id));
}
foreach ($connections as $con) {
if (!isset($this->connections[$con])) {
throw new RuntimeException(sprintf('The Doctrine connection "%s" referenced in service "%s" does not exist. Available connections names: "%s".', $con, $id, implode('", "', array_keys($this->connections))));
Expand All @@ -95,39 +98,25 @@ private function addTaggedSubscribers(ContainerBuilder $container, array &$liste
}

if (ContainerAwareEventManager::class === $managerClass) {
$listenerRefs[$con][$id] = new Reference($id);
$refs = $managerDef->getArguments()[1] ?? [];
$refs[] = $id;
$listenerRefs[$con][$id] = new Reference($id);
if ($subscriberTag === $tagName) {
$refs[] = $id;
} else {
$refs[] = [[$tag['event']], $id];
}
$managerDef->setArgument(1, $refs);
} else {
$managerDef->addMethodCall('addEventSubscriber', [new Reference($id)]);
if ($subscriberTag === $tagName) {
$managerDef->addMethodCall('addEventSubscriber', [new Reference($id)]);
} else {
$managerDef->addMethodCall('addEventListener', [[$tag['event']], new Reference($id)]);
}
}
}
}
}

private function addTaggedListeners(ContainerBuilder $container, array &$listenerRefs)
{
$listenerTag = $this->tagPrefix.'.event_listener';
$taggedListeners = $this->findAndSortTags($listenerTag, $container);

foreach ($taggedListeners as $taggedListener) {
[$id, $tag] = $taggedListener;
if (!isset($tag['event'])) {
throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id));
}

$connections = isset($tag['connection']) ? [$tag['connection']] : array_keys($this->connections);
foreach ($connections as $con) {
if (!isset($this->connections[$con])) {
throw new RuntimeException(sprintf('The Doctrine connection "%s" referenced in service "%s" does not exist. Available connections names: "%s".', $con, $id, implode('", "', array_keys($this->connections))));
}
$listenerRefs[$con][$id] = new Reference($id);

// we add one call per event per service so we have the correct order
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', [[$tag['event']], $id]);
}
}
return $listenerRefs;
}

private function getEventManagerDef(ContainerBuilder $container, string $name)
Expand All @@ -149,14 +138,16 @@ private function getEventManagerDef(ContainerBuilder $container, string $name)
* @see https://bugs.php.net/53710
* @see https://bugs.php.net/60926
*/
private function findAndSortTags(string $tagName, ContainerBuilder $container): array
private function findAndSortTags(array $tagNames, ContainerBuilder $container): array
{
$sortedTags = [];

foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $tags) {
foreach ($tags as $attributes) {
$priority = $attributes['priority'] ?? 0;
$sortedTags[$priority][] = [$serviceId, $attributes];
foreach ($tagNames as $tagName) {
foreach ($container->findTaggedServiceIds($tagName, true) as $serviceId => $tags) {
foreach ($tags as $attributes) {
$priority = $attributes['priority'] ?? 0;
$sortedTags[$priority][] = [$tagName, $serviceId, $attributes];
}
}
}

Expand Down
23 changes: 19 additions & 4 deletions src/Symfony/Bridge/Doctrine/Tests/ContainerAwareEventManagerTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,24 @@ protected function setUp(): void
$this->evm = new ContainerAwareEventManager($this->container);
}

public function testDispatchEventRespectOrder()
{
$this->evm = new ContainerAwareEventManager($this->container, ['sub1', [['foo'], 'list1'], 'sub2']);

$this->container->set('list1', $listener1 = new MyListener());
$this->container->set('sub1', $subscriber1 = new MySubscriber(['foo']));
$this->container->set('sub2', $subscriber2 = new MySubscriber(['foo']));

$this->assertSame([$subscriber1, $listener1, $subscriber2], array_values($this->evm->getListeners('foo')));
}

public function testDispatchEvent()
{
$this->evm = new ContainerAwareEventManager($this->container, ['lazy4']);

$this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo']));
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);

$this->container->set('lazy1', $listener1 = new MyListener());
$this->evm->addEventListener('foo', 'lazy1');
$this->evm->addEventListener('foo', $listener2 = new MyListener());
Expand All @@ -40,10 +54,8 @@ public function testDispatchEvent()
$this->container->set('lazy3', $listener5 = new MyListener());
$this->evm->addEventListener('foo', $listener5 = new MyListener());
$this->evm->addEventListener('bar', $listener5);
$this->container->set('lazy4', $subscriber1 = new MySubscriber(['foo']));
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));

$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);

$this->evm->dispatchEvent('foo');
Expand Down Expand Up @@ -72,19 +84,22 @@ public function testAddEventListenerAndSubscriberAfterDispatchEvent()
{
$this->evm = new ContainerAwareEventManager($this->container, ['lazy7']);

$this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo']));
$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);

$this->container->set('lazy1', $listener1 = new MyListener());
$this->evm->addEventListener('foo', 'lazy1');
$this->assertSame(1, $subscriber1->calledSubscribedEventsCount);

$this->evm->addEventListener('foo', $listener2 = new MyListener());
$this->container->set('lazy2', $listener3 = new MyListener());
$this->evm->addEventListener('bar', 'lazy2');
$this->evm->addEventListener('bar', $listener4 = new MyListener());
$this->container->set('lazy3', $listener5 = new MyListener());
$this->evm->addEventListener('foo', $listener5 = new MyListener());
$this->evm->addEventListener('bar', $listener5);
$this->container->set('lazy7', $subscriber1 = new MySubscriber(['foo']));
$this->evm->addEventSubscriber($subscriber2 = new MySubscriber(['bar']));

$this->assertSame(0, $subscriber1->calledSubscribedEventsCount);
$this->assertSame(1, $subscriber2->calledSubscribedEventsCount);

$this->evm->dispatchEvent('foo');
Expand Down
Loading