Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit ce83b7b

Browse filesBrowse 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
Copy full SHA for ce83b7b

File tree

Expand file treeCollapse file tree

2 files changed

+36
-24
lines changed
Filter options
Expand file treeCollapse file tree

2 files changed

+36
-24
lines changed

‎Debug/TraceableEventDispatcher.php

Copy file name to clipboardExpand all lines: Debug/TraceableEventDispatcher.php
+27-15Lines 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

Copy file name to clipboardExpand all lines: Tests/Debug/TraceableEventDispatcherTest.php
+9-9Lines 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)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.