Skip to content

Commit d4e60e9

Browse files
Jean-Berunicolas-grekas
Jean-Beru
authored and
nicolas-grekas
committed
[DependencyInjection][EventDispatcher] Avoid instantiating not called listeners when tracing
1 parent 0758e59 commit d4e60e9

File tree

9 files changed

+98
-20
lines changed

9 files changed

+98
-20
lines changed

‎src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1739,7 +1739,18 @@ private function dumpValue(mixed $value, bool $interpolate = true): string
17391739

17401740
$code = sprintf('return %s;', $code);
17411741

1742-
return sprintf("function ()%s {\n %s\n }", $returnedType, $code);
1742+
$attribute = '';
1743+
if ($value) {
1744+
$attribute = 'name: '.$this->dumpValue((string) $value, $interpolate);
1745+
1746+
if ($this->container->hasDefinition($value) && ($class = $this->container->findDefinition($value)->getClass()) && $class !== (string) $value) {
1747+
$attribute .= ', class: '.$this->dumpValue($class, $interpolate);
1748+
}
1749+
1750+
$attribute = sprintf('#[\Closure(%s)] ', $attribute);
1751+
}
1752+
1753+
return sprintf("%sfunction ()%s {\n %s\n }", $attribute, $returnedType, $code);
17431754
}
17441755

17451756
if ($value instanceof IteratorArgument) {

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services10_as_files.txt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class getClosureService extends ProjectServiceContainer
2929
{
3030
$container->services['closure'] = $instance = new \stdClass();
3131

32-
$instance->closures = [0 => function () use ($container): ?\stdClass {
32+
$instance->closures = [0 => #[\Closure(name: 'foo', class: 'FooClass')] function () use ($container): ?\stdClass {
3333
return ($container->services['foo'] ?? null);
3434
}];
3535

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_closure_argument_compiled.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ protected function getFooService()
5555
*/
5656
protected function getServiceClosureService()
5757
{
58-
return $this->services['service_closure'] = new \Bar(function () {
58+
return $this->services['service_closure'] = new \Bar(#[\Closure(name: 'foo', class: 'Foo')] function () {
5959
return ($this->services['foo'] ?? ($this->services['foo'] = new \Foo()));
6060
});
6161
}

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_locator.php

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,9 +70,9 @@ protected function getBarServiceService()
7070
*/
7171
protected function getFooServiceService()
7272
{
73-
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\ServiceLocator(['bar' => function () {
73+
return $this->services['foo_service'] = new \Symfony\Component\DependencyInjection\ServiceLocator(['bar' => #[\Closure(name: 'bar_service', class: 'stdClass')] function () {
7474
return ($this->services['bar_service'] ?? $this->getBarServiceService());
75-
}, 'baz' => function (): \stdClass {
75+
}, 'baz' => #[\Closure(name: 'baz_service', class: 'stdClass')] function (): \stdClass {
7676
return ($this->privates['baz_service'] ?? ($this->privates['baz_service'] = new \stdClass()));
7777
}, 'nil' => function () {
7878
return NULL;
@@ -116,7 +116,7 @@ protected function getTranslator_Loader3Service()
116116
*/
117117
protected function getTranslator1Service()
118118
{
119-
return $this->services['translator_1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_1' => function () {
119+
return $this->services['translator_1'] = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_1' => #[\Closure(name: 'translator.loader_1', class: 'stdClass')] function () {
120120
return ($this->services['translator.loader_1'] ?? ($this->services['translator.loader_1'] = new \stdClass()));
121121
}]));
122122
}
@@ -128,7 +128,7 @@ protected function getTranslator1Service()
128128
*/
129129
protected function getTranslator2Service()
130130
{
131-
$this->services['translator_2'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_2' => function () {
131+
$this->services['translator_2'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_2' => #[\Closure(name: 'translator.loader_2', class: 'stdClass')] function () {
132132
return ($this->services['translator.loader_2'] ?? ($this->services['translator.loader_2'] = new \stdClass()));
133133
}]));
134134

@@ -144,7 +144,7 @@ protected function getTranslator2Service()
144144
*/
145145
protected function getTranslator3Service()
146146
{
147-
$this->services['translator_3'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_3' => function () {
147+
$this->services['translator_3'] = $instance = new \Symfony\Component\DependencyInjection\Tests\Fixtures\StubbedTranslator(new \Symfony\Component\DependencyInjection\ServiceLocator(['translator.loader_3' => #[\Closure(name: 'translator.loader_3', class: 'stdClass')] function () {
148148
return ($this->services['translator.loader_3'] ?? ($this->services['translator.loader_3'] = new \stdClass()));
149149
}]));
150150

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/php/services_uninitialized_ref.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,11 +58,11 @@ protected function getBarService()
5858
$instance->foo1 = ($this->services['foo1'] ?? null);
5959
$instance->foo2 = null;
6060
$instance->foo3 = ($this->privates['foo3'] ?? null);
61-
$instance->closures = [0 => function () {
61+
$instance->closures = [0 => #[\Closure(name: 'foo1', class: 'stdClass')] function () {
6262
return ($this->services['foo1'] ?? null);
63-
}, 1 => function () {
63+
}, 1 => #[\Closure(name: 'foo2')] function () {
6464
return null;
65-
}, 2 => function () {
65+
}, 2 => #[\Closure(name: 'foo3', class: 'stdClass')] function () {
6666
return ($this->privates['foo3'] ?? null);
6767
}];
6868
$instance->iter = new RewindableGenerator(function () {

‎src/Symfony/Component/EventDispatcher/Debug/TraceableEventDispatcher.php

Lines changed: 37 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
use Psr\EventDispatcher\StoppableEventInterface;
1515
use Psr\Log\LoggerInterface;
16+
use Symfony\Component\EventDispatcher\EventDispatcher;
1617
use Symfony\Component\EventDispatcher\EventDispatcherInterface;
1718
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
1819
use Symfony\Component\HttpFoundation\Request;
@@ -187,7 +188,7 @@ public function getCalledListeners(Request $request = null): array
187188
public function getNotCalledListeners(Request $request = null): array
188189
{
189190
try {
190-
$allListeners = $this->getListeners();
191+
$allListeners = $this->dispatcher instanceof EventDispatcher ? $this->getListenersWithPriority() : $this->getListenersWithoutPriority();
191192
} catch (\Exception $e) {
192193
$this->logger?->info('An exception was thrown while getting the uncalled listeners.', ['exception' => $e]);
193194

@@ -209,11 +210,12 @@ public function getNotCalledListeners(Request $request = null): array
209210
}
210211

211212
$notCalled = [];
213+
212214
foreach ($allListeners as $eventName => $listeners) {
213-
foreach ($listeners as $listener) {
215+
foreach ($listeners as [$listener, $priority]) {
214216
if (!\in_array($listener, $calledListeners, true)) {
215217
if (!$listener instanceof WrappedListener) {
216-
$listener = new WrappedListener($listener, null, $this->stopwatch, $this);
218+
$listener = new WrappedListener($listener, null, $this->stopwatch, $this, $priority);
217219
}
218220
$notCalled[] = $listener->getInfo($eventName);
219221
}
@@ -323,7 +325,7 @@ private function postProcess(string $eventName): void
323325
}
324326
}
325327

326-
private function sortNotCalledListeners(array $a, array $b)
328+
private function sortNotCalledListeners(array $a, array $b): int
327329
{
328330
if (0 !== $cmp = strcmp($a['event'], $b['event'])) {
329331
return $cmp;
@@ -347,4 +349,35 @@ private function sortNotCalledListeners(array $a, array $b)
347349

348350
return 1;
349351
}
352+
353+
private function getListenersWithPriority(): array
354+
{
355+
$result = [];
356+
357+
$allListeners = new \ReflectionProperty(EventDispatcher::class, 'listeners');
358+
$allListeners->setAccessible(true);
359+
360+
foreach ($allListeners->getValue($this->dispatcher) as $eventName => $listenersByPriority) {
361+
foreach ($listenersByPriority as $priority => $listeners) {
362+
foreach ($listeners as $listener) {
363+
$result[$eventName][] = [$listener, $priority];
364+
}
365+
}
366+
}
367+
368+
return $result;
369+
}
370+
371+
private function getListenersWithoutPriority(): array
372+
{
373+
$result = [];
374+
375+
foreach ($this->getListeners() as $eventName => $listeners) {
376+
foreach ($listeners as $listener) {
377+
$result[$eventName][] = [$listener, null];
378+
}
379+
}
380+
381+
return $result;
382+
}
350383
}

‎src/Symfony/Component/EventDispatcher/Debug/WrappedListener.php

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,23 @@ final class WrappedListener
2929
private Stopwatch $stopwatch;
3030
private ?EventDispatcherInterface $dispatcher;
3131
private string $pretty;
32+
private string $callableRef;
3233
private ClassStub|string $stub;
3334
private ?int $priority = null;
3435
private static bool $hasClassStub;
3536

36-
public function __construct(callable|array $listener, ?string $name, Stopwatch $stopwatch, EventDispatcherInterface $dispatcher = null)
37+
public function __construct(callable|array $listener, ?string $name, Stopwatch $stopwatch, EventDispatcherInterface $dispatcher = null, int $priority = null)
3738
{
3839
$this->listener = $listener;
3940
$this->optimizedListener = $listener instanceof \Closure ? $listener : (\is_callable($listener) ? \Closure::fromCallable($listener) : null);
4041
$this->stopwatch = $stopwatch;
4142
$this->dispatcher = $dispatcher;
43+
$this->priority = $priority;
4244

4345
if (\is_array($listener)) {
44-
$this->name = \is_object($listener[0]) ? get_debug_type($listener[0]) : $listener[0];
46+
[$this->name, $this->callableRef] = $this->parseListener($listener);
4547
$this->pretty = $this->name.'::'.$listener[1];
48+
$this->callableRef .= '::'.$listener[1];
4649
} elseif ($listener instanceof \Closure) {
4750
$r = new \ReflectionFunction($listener);
4851
if (str_contains($r->name, '{closure}')) {
@@ -58,6 +61,7 @@ public function __construct(callable|array $listener, ?string $name, Stopwatch $
5861
} else {
5962
$this->name = get_debug_type($listener);
6063
$this->pretty = $this->name.'::__invoke';
64+
$this->callableRef = \get_class($listener).'::__invoke';
6165
}
6266

6367
if (null !== $name) {
@@ -89,11 +93,11 @@ public function getPretty(): string
8993

9094
public function getInfo(string $eventName): array
9195
{
92-
$this->stub ??= self::$hasClassStub ? new ClassStub($this->pretty.'()', $this->listener) : $this->pretty.'()';
96+
$this->stub ??= self::$hasClassStub ? new ClassStub($this->pretty.'()', $this->callableRef ?? $this->listener) : $this->pretty.'()';
9397

9498
return [
9599
'event' => $eventName,
96-
'priority' => $this->priority ?? $this->dispatcher?->getListenerPriority($eventName, $this->listener),
100+
'priority' => $this->priority ??= $this->dispatcher?->getListenerPriority($eventName, $this->listener),
97101
'pretty' => $this->pretty,
98102
'stub' => $this->stub,
99103
];
@@ -104,7 +108,7 @@ public function __invoke(object $event, string $eventName, EventDispatcherInterf
104108
$dispatcher = $this->dispatcher ?: $dispatcher;
105109

106110
$this->called = true;
107-
$this->priority = $dispatcher->getListenerPriority($eventName, $this->listener);
111+
$this->priority ??= $dispatcher->getListenerPriority($eventName, $this->listener);
108112

109113
$e = $this->stopwatch->start($this->name, 'event_listener');
110114

@@ -118,4 +122,21 @@ public function __invoke(object $event, string $eventName, EventDispatcherInterf
118122
$this->stoppedPropagation = true;
119123
}
120124
}
125+
126+
private function parseListener(array $listener): array
127+
{
128+
if ($listener[0] instanceof \Closure) {
129+
foreach ((new \ReflectionFunction($listener[0]))->getAttributes(\Closure::class) as $attribute) {
130+
if ($name = $attribute->getArguments()['name'] ?? false) {
131+
return [$name, $attribute->getArguments()['class'] ?? $name];
132+
}
133+
}
134+
}
135+
136+
if (\is_object($listener[0])) {
137+
return [get_debug_type($listener[0]), \get_class($listener[0])];
138+
}
139+
140+
return [$listener[0], $listener[0]];
141+
}
121142
}

‎src/Symfony/Component/EventDispatcher/Tests/Debug/TraceableEventDispatcherTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,18 @@ public function testGetCalledListeners()
125125
$this->assertEquals([], $tdispatcher->getNotCalledListeners());
126126
}
127127

128+
public function testGetNotCalledClosureListeners()
129+
{
130+
$instantiationCount = 0;
131+
132+
$tdispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch());
133+
$tdispatcher->addListener('foo', [static function () use (&$instantiationCount) { ++$instantiationCount; }, 'onFoo']);
134+
135+
$tdispatcher->getNotCalledListeners();
136+
137+
$this->assertSame(0, $instantiationCount);
138+
}
139+
128140
public function testClearCalledListeners()
129141
{
130142
$tdispatcher = new TraceableEventDispatcher(new EventDispatcher(), new Stopwatch());

‎src/Symfony/Component/EventDispatcher/Tests/Debug/WrappedListenerTest.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ public function provideListenersToDescribe()
4040
[\Closure::fromCallable([new FooListener(), 'listen']), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen'],
4141
[\Closure::fromCallable(['Symfony\Component\EventDispatcher\Tests\Debug\FooListener', 'listenStatic']), 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listenStatic'],
4242
[\Closure::fromCallable(function () {}), 'closure'],
43+
[[#[\Closure(name: FooListener::class)] static fn () => new FooListener(), 'listen'], 'Symfony\Component\EventDispatcher\Tests\Debug\FooListener::listen'],
4344
];
4445
}
4546
}

0 commit comments

Comments
 (0)