8000 bug #22001 [Doctrine Bridge] fix priority for doctrine event listener… · symfony/symfony@ac109f1 · GitHub
[go: up one dir, main page]
Skip to content

Commit ac109f1

Browse files
fabpotfabpot
committed
bug #22001 [Doctrine Bridge] fix priority for doctrine event listeners (dmaicher)
This PR was merged into the 2.7 branch. Discussion ---------- [Doctrine Bridge] fix priority for doctrine event listeners | Q | A | ------------- | --- | Branch? | 2.7 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #21977 | License | MIT | Doc PR | - This fixes handling the priorities for doctrine event listeners. As found out by @chapterjason in #21977 the priority was incorrectly handled as soon as a listener had more than one tag (so listening to multiple events). With this changes all tagged listeners are globally sorted by priority (using the same stable sort approach as in the later available `PriorityTaggedServiceTrait`) and then added one by one to the event manager. I also updated the tests a bit as it was not covering all cases. We also have to extend the docs for it I think as it does not mention the `priority` and `lazy` option at all? http://symfony.com/doc/current/doctrine/event_listeners_subscribers.html Commits ------- 9d9d4ef [Doctrine Bridge] fix priority for doctrine event listeners
2 parents dd8e50b + 9d9d4ef commit ac109f1

File tree

2 files changed

+210
-114
lines changed

2 files changed

+210
-114
lines changed

‎src/Symfony/Bridge/Doctrine/DependencyInjection/CompilerPass/RegisterEventListenersAndSubscribersPass.php

Lines changed: 89 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -11,27 +11,30 @@
1111

1212
namespace Symfony\Bridge\Doctrine\DependencyInjection\CompilerPass;
1313

14+
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1415
use Symfony\Component\DependencyInjection\ContainerBuilder;
16+
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
17+
use Symfony\Component\DependencyInjection\Exception\RuntimeException;
1518
use Symfony\Component\DependencyInjection\Reference;
16-
use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
1719

1820
/**
1921
* Registers event listeners and subscribers to the available doctrine connections.
2022
*
2123
* @author Jeremy Mikola <jmikola@gmail.com>
2224
* @author Alexander <iam.asm89@gmail.com>
25+
* @author David Maicher <mail@dmaicher.de>
2326
*/
2427
class RegisterEventListenersAndSubscribersPass implements CompilerPassInterface
2528
{
29+
/**
30+
* @var string|string[]
31+
*/
2632
private $connections;
27-
private $container;
2833
private $eventManagers;
2934
private $managerTemplate;
3035
private $tagPrefix;
3136

3237
/**
33-
* Constructor.
34-
*
3538
* @param string $connections Parameter ID for connections
3639
* @param string $managerTemplate sprintf() template for generating the event
3740
* manager's service ID for a connection name
@@ -53,105 +56,112 @@ public function process(ContainerBuilder $container)
5356
return;
5457
}
5558

56-
$taggedSubscribers = $container->findTaggedServiceIds($this->tagPrefix.'.event_subscriber');
57-
$taggedListeners = $container->findTaggedServiceIds($this->tagPrefix.'.event_listener');
58-
59-
if (empty($taggedSubscribers) && empty($taggedListeners)) {
60-
return;
61-
}
62-
63-
$this->container = $container;
6459
$this->connections = $container->getParameter($this->connections);
65-
$sortFunc = function ($a, $b) {
66-
$a = isset($a['priority']) ? $a['priority'] : 0;
67-
$b = isset($b['priority']) ? $b['priority'] : 0;
68-
69-
return $a > $b ? -1 : 1;
70-
};
60+
$this->addTaggedSubscribers($container);
61+
$this->addTaggedListeners($container);
62+
}
7163

72-
if (!empty($taggedSubscribers)) {
73-
$subscribersPerCon = $this->groupByConnection($taggedSubscribers);
74-
foreach ($subscribersPerCon as $con => $subscribers) {
75-
$em = $this->getEventManager($con);
64+
private function addTaggedSubscribers(ContainerBuilder $container)
65+
{
66+
$subscriberTag = $this->tagPrefix.'.event_subscriber';
67+
$taggedSubscribers = $this->findAndSortTags($subscriberTag, $container);
7668

77-
uasort($subscribers, $sortFunc);
78-
foreach ($subscribers as $id => $instance) {
79-
if ($container->getDefinition($id)->isAbstract()) {
80-
throw new \InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event subscriber.', $id));
81-
}
69+
foreach ($taggedSubscribers as $taggedSubscriber) {
70+
$id = $taggedSubscriber[0];
71+
$taggedSubscriberDef = $container->getDefinition($id);
8272

83-
$em->addMethodCall('addEventSubscriber', array(new Reference($id)));
84-
}
73+
if ($taggedSubscriberDef->isAbstract()) {
74+
throw new InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event subscriber.', $id));
8575
}
86-
}
8776

88-
if (!empty($taggedListeners)) {
89-
$listenersPerCon = $this->groupByConnection($taggedListeners, true);
90-
foreach ($listenersPerCon as $con => $listeners) {
91-
$em = $this->getEventManager($con);
92-
93-
uasort($listeners, $sortFunc);
94-
foreach ($listeners as $id => $instance) {
95-
if ($container->getDefinition($id)->isAbstract()) {
96-
throw new \InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event listener.', $id));
97-
}
98-
99-
$em->addMethodCall('addEventListener', array(
100-
array_unique($instance['event']),
101-
isset($instance['lazy']) && $instance['lazy'] ? $id : new Reference($id),
102-
));
77+
$tag = $taggedSubscriber[1];
78+
$connections = isset($tag['connection']) ? array($tag['connection']) : array_keys($this->connections);
79+
foreach ($connections as $con) {
80+
if (!isset($this->connections[$con])) {
81+
throw new RuntimeException(sprintf('The Doctrine connection "%s" referenced in service "%s" does not exist. Available connections names: %s', $con, $taggedSubscriber, implode(', ', array_keys($this->connections))));
10382
}
83+
84+
$this->getEventManagerDef($container, $con)->addMethodCall('addEventSubscriber', array(new Reference($id)));
10485
}
10586
}
10687
}
10788

108-
private function groupByConnection(array $services, $isListener = false)
89+
private function addTaggedListeners(ContainerBuilder $container)
10990
{
110-
$grouped = array();
111-
foreach ($allCons = array_keys($this->connections) as $con) {
112-
$grouped[$con] = array();
113-
}
91+
$listenerTag = $this->tagPrefix.'.event_listener';
92+
$taggedListeners = $this->findAndSortTags($listenerTag, $container);
93+
94+
foreach ($taggedListeners as $taggedListener) {
95+
$id = $taggedListener[0];
96+
$taggedListenerDef = $container->getDefinition($taggedListener[0]);
97+
if ($taggedListenerDef->isAbstract()) {
98+
throw new InvalidArgumentException(sprintf('The abstract service "%s" cannot be tagged as a doctrine event listener.', $id));
99+
}
114100

115-
foreach ($services as $id => $instances) {
116-
foreach ($instances as $instance) {
117-
if ($isListener) {
118-
if (!isset($instance['event'])) {
119-
throw new \InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id));
120-
}
121-
$instance['event'] = array($instance['event']);
122-
123-
if (isset($instance['lazy']) && $instance['lazy']) {
124-
$this->container->getDefinition($id)->setPublic(true);
125-
}
101+
$tag = $taggedListener[1];
102+
if (!isset($tag['event'])) {
103+
throw new InvalidArgumentException(sprintf('Doctrine event listener "%s" must specify the "event" attribute.', $id));
104+
}
105+
106+
$connections = isset($tag['connection']) ? array($tag['connection']) : array_keys($this->connections);
107+
foreach ($connections as $con) {
108+
if (!isset($this->connections[$con])) {
109+
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))));
126110
}
127111

128-
$cons = isset($instance['connection']) ? array($instance['connection']) : $allCons;
129-
foreach ($cons as $con) {
130-
if (!isset($grouped[$con])) {
131-
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))));
132-
}
133-
134-
if ($isListener && isset($grouped[$con][$id])) {
135-
$grouped[$con][$id]['event'] = array_merge($grouped[$con][$id]['event'], $instance['event']);
136-
} else {
137-
$grouped[$con][$id] = $instance;
138-
}
112+
if ($lazy = isset($tag['lazy']) && $tag['lazy']) {
113+
$taggedListenerDef->setPublic(true);
139114
}
115+
116+
// we add one call per event per service so we have the correct order
117+
$this->getEventManagerDef($container, $con)->addMethodCall('addEventListener', array(
118+
$tag['event'],
119+
$lazy ? $id : new Reference($id),
120+
));
140121
}
141122
}
123+
}
142124

143-
return $grouped;
125+
private function getEventManagerDef(ContainerBuilder $container, $name)
126+
{
127+
if (!isset($this->eventManagers[$name])) {
128+
$this->eventManagers[$name] = $container->getDefinition(sprintf($this->managerTemplate, $name));
129+
}
130+
131+
return $this->eventManagers[$name];
144132
}
145133

146-
private function getEventManager($name)
134+
/**
135+
* Finds and orders all service tags with the given name by their priority.
136+
*
137+
* The order of additions must be respected for services having the same priority,
138+
* and knowing that the \SplPriorityQueue class does not respect the FIFO method,
139+
* we should not use this class.
140+
*
141+
* @see https://bugs.php.net/bug.php?id=53710
142+
* @see https://bugs.php.net/bug.php?id=60926
143+
*
144+
* @param string $tagName
145+
* @param ContainerBuilder $container
146+
*
147+
* @return array
148+
*/
149+
private function findAndSortTags($tagName, ContainerBuilder $container)
147150
{
148-
if (null === $this->eventManagers) {
149-
$this->eventManagers = array();
150-
foreach ($this->connections as $n => $id) {
151-
$this->eventManagers[$n] = $this->container->getDefinition(sprintf($this->managerTemplate, $n));
151+
$sortedTags = array();
152+
153+
foreach ($container->findTaggedServiceIds($tagName) as $serviceId => $tags) {
154+
foreach ($tags as $attributes) {
155+
$priority = isset($attributes['priority']) ? $attributes['priority'] : 0;
156+
$sortedTags[$priority][] = array($serviceId, $attributes);
152157
}
153158
}
154159

155-
return $this->eventManagers[$name];
160+
if ($sortedTags) {
161+
krsort($sortedTags);
162+
$sortedTags = call_user_func_array('array_merge', $sortedTags);
163+
}
164+
165+
return $sortedTags;
156166
}
157167
}

0 commit comments

Comments
 (0)
0