8000 bug #20995 [DependencyInjection] Fix the priority order of compiler p… · symfony/dependency-injection@bd626bc · GitHub
[go: up one dir, main page]

Skip to content

Commit bd626bc

Browse files
bug #20995 [DependencyInjection] Fix the priority order of compiler pass trait (francoispluchino)
This PR was merged into the 3.2 branch. Discussion ---------- [DependencyInjection] Fix the priority order of compiler pass trait | Q | A | ------------- | --- | Branch? | 3.2 | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #20332, #20993 | License | MIT | Doc PR | ~ A regression has appeared between the version `3.1` and `3.2` on the compilation passes using a priority option (see #20332). Indeed, the order is no longer preserved for service definitions with the same priority. This is caused by the `SplPriorityQueue` class that does not have a FIFO implementation (see this [comment](symfony/symfony#20332 (comment))). The PR #20993 fixes the problem but only for Forms, not for all compiler passes using the `PriorityTaggedServiceTrait` trait since `3.2`. Commits ------- aac9a7e Fix the priority order of compiler pass trait
2 parents 1205cad + cf307bb commit bd626bc

File tree

2 files changed

+28
-6
lines changed

2 files changed

+28
-6
lines changed

Compiler/PriorityTaggedServiceTrait.php

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -24,24 +24,34 @@ trait PriorityTaggedServiceTrait
2424
/**
2525
* Finds all services with the given tag name and order them by their priority.
2626
*
27+
* The order of additions must be respected for services having the same priority,
28+
* and knowing that the \SplPriorityQueue class does not respect the FIFO method,
29+
* we should not use this class.
30+
*
31+
* @see https://bugs.php.net/bug.php?id=53710
32+
* @see https://bugs.php.net/bug.php?id=60926
33+
*
2734
* @param string $tagName
2835
* @param ContainerBuilder $container
2936
*
3037
* @return Reference[]
3138
*/
3239
private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
3340
{
34-
$services = $container->findTaggedServiceIds($tagName);
35-
36-
$queue = new \SplPriorityQueue();
41+
$services = array();
3742

38-
foreach ($services as $serviceId => $tags) {
43+
foreach ($container->findTaggedServiceIds($tagName) as $serviceId => $tags) {
3944
foreach ($tags as $attributes) {
4045
$priority = isset($attributes['priority']) ? $attributes['priority'] : 0;
41-
$queue->insert(new Reference($serviceId), $priority);
46+
$services[$priority][] = new Reference($serviceId);
4247
}
4348
}
4449

45-
return iterator_to_array($queue, false);
50+
if ($services) {
51+
krsort($services);
52+
$services = call_user_func_array('array_merge', $services);
53+
}
54+
55+
return $services;
4656
}
4757
}

Tests/Compiler/PriorityTaggedServiceTraitTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@ public function testThatCacheWarmersAreProcessedInPriorityOrder()
3333
'my_service11' => array('my_custom_tag' => array('priority' => -1001)),
3434
'my_service12' => array('my_custom_tag' => array('priority' => -1002)),
3535
'my_service13' => array('my_custom_tag' => array('priority' => -1003)),
36+
'my_service14' => array('my_custom_tag' => array('priority' => -1000)),
37+
'my_service15' => array('my_custom_tag' => array('priority' => 1)),
38+
'my_service16' => array('my_custom_tag' => array('priority' => -1)),
39+
'my_service17' => array('my_custom_tag' => array('priority' => 200)),
40+
'my_service18' => array('my_custom_tag' => array('priority' => 100)),
41+
'my_service19' => array('my_custom_tag' => array()),
3642
);
3743

3844
$container = new ContainerBuilder();
@@ -47,15 +53,21 @@ public function testThatCacheWarmersAreProcessedInPriorityOrder()
4753

4854
$expected = array(
4955
new Reference('my_service2'),
56+
new Reference('my_service17'),
5057
new Reference('my_service1'),
58+
new Reference('my_service18'),
5159
new Reference('my_service8'),
60+
new Reference('my_service15'),
5261
new Reference('my_service4'),
62+
new Reference('my_service19'),
5363
new Reference('my_service5'),
64+
new Reference('my_service16'),
5465
new Reference('my_service9'),
5566
new Reference('my_service7'),
5667
new Reference('my_service6'),
5768
new Reference('my_service3'),
5869
new Reference('my_service10'),
70+
new Reference('my_service14'),
5971
new Reference('my_service11'),
6072
new Reference('my_service12'),
6173
new Reference('my_service13'),

0 commit comments

Comments
 (0)
0