Navigation Menu

Skip to content

Commit

Permalink
bug #20995 [DependencyInjection] Fix the priority order of compiler p…
Browse files Browse the repository at this point in the history
…ass 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](#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
  • Loading branch information
nicolas-grekas committed Jan 3, 2017
2 parents 1e39b99 + aac9a7e commit 142afa3
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 6 deletions.
Expand Up @@ -24,24 +24,34 @@ trait PriorityTaggedServiceTrait
/**
* Finds all services with the given tag name and order them by their priority.
*
* The order of additions must be respected for services having the same priority,
* and knowing that the \SplPriorityQueue class does not respect the FIFO method,
* we should not use this class.
*
* @see https://bugs.php.net/bug.php?id=53710
* @see https://bugs.php.net/bug.php?id=60926
*
* @param string $tagName
* @param ContainerBuilder $container
*
* @return Reference[]
*/
private function findAndSortTaggedServices($tagName, ContainerBuilder $container)
{
$services = $container->findTaggedServiceIds($tagName);

$queue = new \SplPriorityQueue();
$services = array();

foreach ($services as $serviceId => $tags) {
foreach ($container->findTaggedServiceIds($tagName) as $serviceId => $tags) {
foreach ($tags as $attributes) {
$priority = isset($attributes['priority']) ? $attributes['priority'] : 0;
$queue->insert(new Reference($serviceId), $priority);
$services[$priority][] = new Reference($serviceId);
}
}

return iterator_to_array($queue, false);
if ($services) {
krsort($services);
$services = call_user_func_array('array_merge', $services);
}

return $services;
}
}
Expand Up @@ -33,6 +33,12 @@ public function testThatCacheWarmersAreProcessedInPriorityOrder()
'my_service11' => array('my_custom_tag' => array('priority' => -1001)),
'my_service12' => array('my_custom_tag' => array('priority' => -1002)),
'my_service13' => array('my_custom_tag' => array('priority' => -1003)),
'my_service14' => array('my_custom_tag' => array('priority' => -1000)),
'my_service15' => array('my_custom_tag' => array('priority' => 1)),
'my_service16' => array('my_custom_tag' => array('priority' => -1)),
'my_service17' => array('my_custom_tag' => array('priority' => 200)),
'my_service18' => array('my_custom_tag' => array('priority' => 100)),
'my_service19' => array('my_custom_tag' => array()),
);

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

$expected = array(
new Reference('my_service2'),
new Reference('my_service17'),
new Reference('my_service1'),
new Reference('my_service18'),
new Reference('my_service8'),
new Reference('my_service15'),
new Reference('my_service4'),
new Reference('my_service19'),
new Reference('my_service5'),
new Reference('my_service16'),
new Reference('my_service9'),
new Reference('my_service7'),
new Reference('my_service6'),
new Reference('my_service3'),
new Reference('my_service10'),
new Reference('my_service14'),
new Reference('my_service11'),
new Reference('my_service12'),
new Reference('my_service13'),
Expand Down

0 comments on commit 142afa3

Please sign in to comment.