8000 bug #29411 [EventDispatcher] Revers event tracing order (ro0NL) · symfony/event-dispatcher@ce83b7b · GitHub
[go: up one dir, main page]
Skip to content

Commit ce83b7b

Browse files
nicolas-grekasnicolas-grekas
committed
bug #29411 [EventDispatcher] Revers event tracing order (ro0NL)
This PR was merged into the 3.4 branch. Discussion ---------- [EventDispatcher] Revers event tracing order | Q | A | ------------- | --- | Branch? | 3.4 | Bug fix? | yes | New feature? | no | BC breaks? | no <!-- see https://symfony.com/bc --> | Deprecations? | no | Tests pass? | yes <!-- please add some, will be required by reviewers --> | Fixed tickets | #... <!-- #-prefixed issue number(s), if any --> | License | MIT | Doc PR | symfony/symfony-docs#... <!-- required for new features --> Split from #29312 for 3.4 This traces events in dispatch order. Before: ![image](https://user-images.githubusercontent.com/1047696/49330956-5f71e780-f596-11e8-8701-80458e715213.png) After: ![image](https://user-images.githubusercontent.com/1047696/49330963-79abc580-f596-11e8-8666-5535afd59b31.png) Here we see it also collects "terminate" events (both kernel & console for not-called listeners). In case of exception page the fix is even better: symfony/symfony#29312 (review) Which now correctly shows the kernel.exception event of the main request is dispatched _before_ the kernel.request event of the sub-request. Moreover, it de-duplicates events. So we actually see the sub-request events 🎉 _Havent looked at tests yet._ Commits ------- 2570d6f877 [EventDispatcher] Revers event tracing order
2 parents a22e2f0 + b68f7c1 commit ce83b7b

File tree

2 files changed

+36
-24
lines changed

2 files changed

+36
-24
lines changed

‎Debug/TraceableEventDispatcher.php

Lines changed: 27 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class TraceableEventDispatcher implements TraceableEventDispatcherInterface
2929
protected $logger;
3030
protected $stopwatch;
3131

32-
private $called;
32+
private $callStack;
3333
private $dispatcher;
3434
private $wrappedListeners;
3535

@@ -38,7 +38,6 @@ public function __construct(EventDispatcherInterface $dispatcher, Stopwatch $sto
3838
$this->dispatcher = $dispatcher;
3939
$this->stopwatch = $stopwatch;
4040
$this->logger = $logger;
41-
$this->called = array();
4241
$this->wrappedListeners = array();
4342
}
4443

@@ -123,6 +122,10 @@ public function hasListeners($eventName = null)
123122
*/
124123
public function dispatch($eventName, Event $event = null)
125124
{
125+
if (null === $this->callStack) {
126+
$this->callStack = new \SplObjectStorage();
127+
}
128+
126129
if (null === $event) {
127130
$event = new Event();
128131
}
@@ -158,11 +161,15 @@ public function dispatch($eventName, Event $event = null)
158161
*/
159162
public function getCalledListeners()
160163
{
164+
if (null === $this->callStack) {
165+
return array();
166+
}
167+
161168
$called = array();
162-
foreach ($this->called as $eventName => $listeners) {
163-
foreach ($listeners as $listener) {
164-
$called[$eventName.'.'.$listener->getPretty()] = $listener->getInfo($eventName);
165-
}
169+
foreach ($this->callStack as $listener) {
170+
list($eventName) = $this->callStack->getInfo();
171+
172+
$called[] = $listener->getInfo($eventName);
166173
}
167174

168175
return $called;
@@ -188,9 +195,9 @@ public function getNotCalledListeners()
188195
foreach ($allListeners as $eventName => $listeners) {
189196
foreach ($listeners as $listener) {
190197
$called = false;
191-
if (isset($this->called[$eventName])) {
192-
foreach ($this->called[$eventName] as $l) {
193-
if ($l->getWrappedListener() === $listener) {
198+
if (null !== $this->callStack) {
199+
foreach ($this->callStack as $calledListener) {
200+
if ($calledListener->getWrappedListener() === $listener) {
194201
$called = true;
195202

196203
break;
@@ -202,19 +209,19 @@ public function getNotCalledListeners()
202209
if (!$listener instanceof WrappedListener) {
203210
$listener = new WrappedListener($listener, null, $this->stopwatch, $this);
204211
}
205-
$notCalled[$eventName.'.'.$listener->getPretty()] = $listener->getInfo($eventName);
212+
$notCalled[] = $listener->getInfo($eventName);
206213
}
207214
}
208215
}
209216

210-
uasort($notCalled, array($this, 'sortListenersByPriority'));
217+
uasort($notCalled, array($this, 'sortNotCalledListeners'));
211218

212219
return $notCalled;
213220
}
214221

215222
public function reset()
216223
{
217-
$this->called = array();
224+
$this->callStack = array();
218225
}
219226

220227
/**
@@ -258,6 +265,7 @@ private function preProcess($eventName)
258265
$this->wrappedListeners[$eventName][] = $wrappedListener;
259266
$this->dispatcher->removeListener($eventName, $listener);
260267
$this->dispatcher->addListener($eventName, $wrappedListener, $priority);
268+
$this->callStack->attach($wrappedListener, array($eventName));
261269
}
262270
}
263271

@@ -286,8 +294,8 @@ private function postProcess($eventName)
286294
if (!isset($this->called[$eventName])) {
287295
$this->called[$eventName] = new \SplObjectStorage();
288296
}
289-
290-
$this->called[$eventName]->attach($listener);
297+
} else {
298+
$this->callStack->detach($listener);
291299
}
292300

293301
if (null !== $this->logger && $skipped) {
@@ -304,8 +312,12 @@ private function postProcess($eventName)
304312
}
305313
}
306314

307-
private function sortListenersByPriority($a, $b)
315+
private function sortNotCalledListeners(array $a, array $b)
308316
{
317+
if (0 !== $cmp = strcmp($a['event'], $b['event'])) {
318+
return $cmp;
319+
}
320+
309321
if (\is_int($a['priority']) && !\is_int($b['priority'])) {
310322
return 1;
311323
}

‎Tests/Debug/TraceableEventDispatcherTest.php

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -110,17 +110,17 @@ public function testGetCalledListeners()
110110
$tdispatcher->addListener('foo', function () {}, 5);
111111

112112
$listeners = $tdispatcher->getNotCalledListeners();
113-
$this->assertArrayHasKey('stub', $listeners['foo.closure']);
114-
unset($listeners['foo.closure']['stub']);
113+
$this->assertArrayHasKey('stub', $listeners[0]);
114+
unset($listeners[0]['stub']);
115115
$this->assertEquals(array(), $tdispatcher->getCalledListeners());
116-
$this->assertEquals(array('foo.closure' => array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
116+
$this->assertEquals(array(array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
117117

118118
$tdispatcher->dispatch('foo');
119119

120120
$listeners = $tdispatcher->getCalledListeners();
121-
$this->assertArrayHasKey('stub', $listeners['foo.closure']);
122-
unset($listeners['foo.closure']['stub']);
123-
$this->assertEquals(array('foo.closure' => array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
121+
$this->assertArrayHasKey('stub', $listeners[0]);
122+
unset($listeners[0]['stub']);
123+
$this->assertEquals(array(array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
124124
$this->assertEquals(array(), $tdispatcher->getNotCalledListeners());
125125
}
126126

@@ -133,10 +133,10 @@ public function testClearCalledListeners()
133133
$tdispatcher->reset();
134134

135135
$listeners = $tdispatcher->getNotCalledListeners();
136-
$this->assertArrayHasKey('stub', $listeners['foo.closure']);
137-
unset($listeners['foo.closure']['stub']);
136+
$this->assertArrayHasKey('stub', $listeners[0]);
137+
unset($listeners[0]['stub']);
138138
$this->assertEquals(array(), $tdispatcher->getCalledListeners());
139-
$this->assertEquals(array('foo.closure' => array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
139+
$this->assertEquals(array(array('event' => 'foo', 'pretty' => 'closure', 'priority' => 5)), $listeners);
140140
}
141141

142142
public function testGetCalledListenersNested()

0 commit comments

Comments
 (0)
0