Added a way to define the priority of service decoration

dosten · dosten · commit 96d5a95101a7 · 2015-07-31T13:03:15.000-03:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ CHANGELOG
  * deprecated the concept of scopes
  * added `Definition::setShared()` and `Definition::isShared()`
  * added ResettableContainerInterface to be able to reset the container to release memory on shutdown
+ * added a way to define the priority of service decoration
 
 2.7.0
 -----
diff --git a/Compiler/DecoratorServicePass.php b/Compiler/DecoratorServicePass.php
@@ -19,18 +19,28 @@
  *
  * @author Christophe Coevoet <stof@notk.org>
  * @author Fabien Potencier <fabien@symfony.com>
+ * @author Diego Saint Esteben <diego@saintesteben.me>
  */
 class DecoratorServicePass implements CompilerPassInterface
 {
     public function process(ContainerBuilder $container)
     {
+        $definitions = new \SplPriorityQueue();
+        $order = PHP_INT_MAX;
+
         foreach ($container->getDefinitions() as $id => $definition) {
             if (!$decorated = $definition->getDecoratedService()) {
                 continue;
             }
+            $definitions->insert(array($id, $definition), array($decorated[2], --$order));
+        }
+
+        foreach ($definitions as $arr) {
+            list($id, $definition) = $arr;
+            list($inner, $renamedId) = $definition->getDecoratedService();
+
             $definition->setDecoratedService(null);
 
-            list($inner, $renamedId) = $decorated;
             if (!$renamedId) {
                 $renamedId = $id.'.inner';
             }
diff --git a/Definition.php b/Definition.php
@@ -150,12 +150,13 @@ public function setFactoryMethod($factoryMethod)
      *
      * @param null|string $id        The decorated service id, use null to remove decoration
      * @param null|string $renamedId The new decorated service id
+     * @param int         $priority  The priority of decoration
      *
      * @return Definition The current instance
      *
      * @throws InvalidArgumentException In case the decorated service id and the new decorated service id are equals.
      */
-    public function setDecoratedService($id, $renamedId = null)
+    public function setDecoratedService($id, $renamedId = null, $priority = 0)
     {
         if ($renamedId && $id == $renamedId) {
             throw new \InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id));
@@ -164,7 +165,7 @@ public function setDecoratedService($id, $renamedId = null)
         if (null === $id) {
             $this->decoratedService = null;
         } else {
-            $this->decoratedService = array($id, $renamedId);
+            $this->decoratedService = array($id, $renamedId, (int) $priority);
         }
 
         return $this;
@@ -173,7 +174,7 @@ public function setDecoratedService($id, $renamedId = null)
     /**
      * Gets the service that decorates this service.
      *
-     * @return null|array An array composed of the decorated service id and the new id for it, null if no service is decorated
+     * @return null|array An array composed of the decorated service id, the new id for it and the priority of decoration, null if no service is decorated
      */
     public function getDecoratedService()
     {
diff --git a/DefinitionDecorator.php b/DefinitionDecorator.php
@@ -173,11 +173,11 @@ public function setLazy($boolean)
     /**
      * {@inheritdoc}
      */
-    public function setDecoratedService($id, $renamedId = null)
+    public function setDecoratedService($id, $renamedId = null, $priority = 0)
     {
         $this->changes['decorated_service'] = true;
 
-        return parent::setDecoratedService($id, $renamedId);
+        return parent::setDecoratedService($id, $renamedId, $priority);
     }
 
     /**
diff --git a/Dumper/XmlDumper.php b/Dumper/XmlDumper.php
@@ -145,11 +145,14 @@ private function addService($definition, $id, \DOMElement $parent)
             $service->setAttribute('lazy', 'true');
         }
         if (null !== $decorated = $definition->getDecoratedService()) {
-            list($decorated, $renamedId) = $decorated;
+            list($decorated, $renamedId, $priority) = $decorated;
             $service->setAttribute('decorates', $decorated);
             if (null !== $renamedId) {
                 $service->setAttribute('decoration-inner-name', $renamedId);
             }
+            if (0 !== $priority) {
+                $service->setAttribute('decoration-priority', $priority);
+            }
         }
 
         foreach ($definition->getTags() as $name => $tags) {
diff --git a/Dumper/YamlDumper.php b/Dumper/YamlDumper.php
@@ -137,11 +137,14 @@ private function addService($id, $definition)
         }
 
         if (null !== $decorated = $definition->getDecoratedService()) {
-            list($decorated, $renamedId) = $decorated;
+            list($decorated, $renamedId, $priority) = $decorated;
             $code .= sprintf("        decorates: %s\n", $decorated);
             if (null !== $renamedId) {
                 $code .= sprintf("        decoration_inner_name: %s\n", $renamedId);
             }
+            if (0 !== $priority) {
+                $code .= sprintf("        decoration_priority: %s\n", $priority);
+            }
         }
 
         if ($callable = $definition->getFactory()) {
diff --git a/Loader/XmlFileLoader.php b/Loader/XmlFileLoader.php
@@ -245,7 +245,8 @@ private function parseDefinition(\DOMElement $service, $file)
 
         if ($value = $service->getAttribute('decorates')) {
             $renameId = $service->hasAttribute('decoration-inner-name') ? $service->getAttribute('decoration-inner-name') : null;
-            $definition->setDecoratedService($value, $renameId);
+            $priority = $service->hasAttribute('decoration-priority') ? $service->getAttribute('decoration-priority') : 0;
+            $definition->setDecoratedService($value, $renameId, $priority);
         }
 
         return $definition;
diff --git a/Loader/YamlFileLoader.php b/Loader/YamlFileLoader.php
@@ -290,7 +290,8 @@ private function parseDefinition($id, $service, $file)
 
         if (isset($service['decorates'])) {
             $renameId = isset($service['decoration_inner_name']) ? $service['decoration_inner_name'] : null;
-            $definition->setDecoratedService($service['decorates'], $renameId);
+            $priority = isset($service['decoration_priority']) ? $service['decoration_priority'] : 0;
+            $definition->setDecoratedService($service['decorates'], $renameId, $priority);
         }
 
         $this->container->setDefinition($id, $definition);
diff --git a/Loader/schema/dic/services/services-1.0.xsd b/Loader/schema/dic/services/services-1.0.xsd
@@ -101,6 +101,7 @@
     <xsd:attribute name="parent" type="xsd:string" />
     <xsd:attribute name="decorates" type="xsd:string" />
     <xsd:attribute name="decoration-inner-name" type="xsd:string" />
+    <xsd:attribute name="decoration-priority" type="xsd:integer" />
   </xsd:complexType>
 
   <xsd:complexType name="tag">
diff --git a/Tests/Compiler/DecoratorServicePassTest.php b/Tests/Compiler/DecoratorServicePassTest.php
@@ -73,6 +73,48 @@ public function testProcessWithAlias()
         $this->assertNull($fooExtendedDefinition->getDecoratedService());
     }
 
+    public function testProcessWithPriority()
+    {
+        $container = new ContainerBuilder();
+        $fooDefinition = $container
+            ->register('foo')
+            ->setPublic(false)
+        ;
+        $barDefinition = $container
+            ->register('bar')
+            ->setPublic(true)
+            ->setDecoratedService('foo')
+        ;
+        $bazDefinition = $container
+            ->register('baz')
+            ->setPublic(true)
+            ->setDecoratedService('foo', null, 5)
+        ;
+        $quxDefinition = $container
+            ->register('qux')
+            ->setPublic(true)
+            ->setDecoratedService('foo', null, 3)
+        ;
+
+        $this->process($container);
+
+        $this->assertEquals('bar', $container->getAlias('foo'));
+        $this->assertFalse($container->getAlias('foo')->isPublic());
+
+        $this->assertSame($fooDefinition, $container->getDefinition('baz.inner'));
+        $this->assertFalse($container->getDefinition('baz.inner')->isPublic());
+
+        $this->assertEquals('qux', $container->getAlias('bar.inner'));
+        $this->assertFalse($container->getAlias('bar.inner')->isPublic());
+
+        $this->assertEquals('baz', $container->getAlias('qux.inner'));
+        $this->assertFalse($container->getAlias('qux.inner')->isPublic());
+
+        $this->assertNull($barDefinition->getDecoratedService());
+        $this->assertNull($bazDefinition->getDecoratedService());
+        $this->assertNull($quxDefinition->getDecoratedService());
+    }
+
     protected function process(ContainerBuilder $container)
     {
         $repeatedPass = new DecoratorServicePass();
diff --git a/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php b/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php
@@ -241,7 +241,7 @@ public function testSetDecoratedServiceOnServiceHasParent()
             ->setDecoratedService('foo', 'foo_inner')
         ;
 
-        $this->assertEquals(array('foo', 'foo_inner'), $container->getDefinition('child1')->getDecoratedService());
+        $this->assertEquals(array('foo', 'foo_inner', 0), $container->getDefinition('child1')->getDecoratedService());
     }
 
     protected function process(ContainerBuilder $container)
diff --git a/Tests/DefinitionTest.php b/Tests/DefinitionTest.php
@@ -56,16 +56,23 @@ public function testSetGetClass()
 
     public function testSetGetDecoratedService()
     {
+        $def = new Definition('stdClass');
+        $this->assertNull($def->getDecoratedService());
+        $def->setDecoratedService('foo', 'foo.renamed', 5);
+        $this->assertEquals(array('foo', 'foo.renamed', 5), $def->getDecoratedService());
+        $def->setDecoratedService(null);
+        $this->assertNull($def->getDecoratedService());
+
         $def = new Definition('stdClass');
         $this->assertNull($def->getDecoratedService());
         $def->setDecoratedService('foo', 'foo.renamed');
-        $this->assertEquals(array('foo', 'foo.renamed'), $def->getDecoratedService());
+        $this->assertEquals(array('foo', 'foo.renamed', 0), $def->getDecoratedService());
         $def->setDecoratedService(null);
         $this->assertNull($def->getDecoratedService());
 
         $def = new Definition('stdClass');
         $def->setDecoratedService('foo');
-        $this->assertEquals(array('foo', null), $def->getDecoratedService());
+        $this->assertEquals(array('foo', null, 0), $def->getDecoratedService());
         $def->setDecoratedService(null);
         $this->assertNull($def->getDecoratedService());
 
diff --git a/Tests/Fixtures/xml/services6.xml b/Tests/Fixtures/xml/services6.xml
@@ -47,6 +47,7 @@
     <service id="another_alias_for_foo" alias="foo" public="false" />
     <service id="decorator_service" decorates="decorated" />
     <service id="decorator_service_with_name" decorates="decorated" decoration-inner-name="decorated.pif-pouf"/>
+    <service id="decorator_service_with_name_and_priority" decorates="decorated" decoration-inner-name="decorated.pif-pouf" decoration-priority="5"/>
     <service id="new_factory1" class="FooBarClass">
       <factory function="factory" />
     </service>
diff --git a/Tests/Fixtures/yaml/services6.yml b/Tests/Fixtures/yaml/services6.yml
@@ -26,6 +26,10 @@ services:
     decorator_service_with_name:
         decorates: decorated
         decoration_inner_name: decorated.pif-pouf
+    decorator_service_with_name_and_priority:
+        decorates: decorated
+        decoration_inner_name: decorated.pif-pouf
+        decoration_priority: 5
     new_factory1: { class: FooBarClass, factory: factory}
     new_factory2: { class: FooBarClass, factory: [@baz, getClass]}
     new_factory3: { class: FooBarClass, factory: [BazClass, getInstance]}
diff --git a/Tests/Loader/XmlFileLoaderTest.php b/Tests/Loader/XmlFileLoaderTest.php
@@ -242,8 +242,9 @@ public function testLoadServices()
         $this->assertEquals('foo', (string) $aliases['another_alias_for_foo']);
         $this->assertFalse($aliases['another_alias_for_foo']->isPublic());
 
-        $this->assertEquals(array('decorated', null), $services['decorator_service']->getDecoratedService());
-        $this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService());
+        $this->assertEquals(array('decorated', null, 0), $services['decorator_service']->getDecoratedService());
+        $this->assertEquals(array('decorated', 'decorated.pif-pouf', 0), $services['decorator_service_with_name']->getDecoratedService());
+        $this->assertEquals(array('decorated', 'decorated.pif-pouf', 5), $services['decorator_service_with_name_and_priority']->getDecoratedService());
     }
 
     public function testParsesTags()
diff --git a/Tests/Loader/YamlFileLoaderTest.php b/Tests/Loader/YamlFileLoaderTest.php
@@ -171,8 +171,9 @@ public function testLoadServices()
         $this->assertEquals('foo', (string) $aliases['another_alias_for_foo']);
         $this->assertFalse($aliases['another_alias_for_foo']->isPublic());
 
-        $this->assertEquals(array('decorated', null), $services['decorator_service']->getDecoratedService());
-        $this->assertEquals(array('decorated', 'decorated.pif-pouf'), $services['decorator_service_with_name']->getDecoratedService());
+        $this->assertEquals(array('decorated', null, 0), $services['decorator_service']->getDecoratedService());
+        $this->assertEquals(array('decorated', 'decorated.pif-pouf', 0), $services['decorator_service_with_name']->getDecoratedService());
+        $this->assertEquals(array('decorated', 'decorated.pif-pouf', 5), $services['decorator_service_with_name_and_priority']->getDecoratedService());
     }
 
     public function testLoadFactoryShortSyntax()

Original file line number	Diff line number	Diff line change
`@@ -19,18 +19,28 @@`
`19`	`19`	`*`
`20`	`20`	`* @author Christophe Coevoet <stof@notk.org>`
`21`	`21`	`* @author Fabien Potencier <fabien@symfony.com>`
	`22`	`+ * @author Diego Saint Esteben <diego@saintesteben.me>`
`22`	`23`	`*/`
`23`	`24`	`class DecoratorServicePass implements CompilerPassInterface`
`24`	`25`	`{`
`25`	`26`	`public function process(ContainerBuilder $container)`
`26`	`27`	`{`
	`28`	`+ $definitions = new \SplPriorityQueue();`
	`29`	`+ $order = PHP_INT_MAX;`
	`30`	`+`
`27`	`31`	`foreach ($container->getDefinitions() as $id => $definition) {`
`28`	`32`	`if (!$decorated = $definition->getDecoratedService()) {`
`29`	`33`	`continue;`
`30`	`34`	`}`
	`35`	`+ $definitions->insert(array($id, $definition), array($decorated[2], --$order));`
	`36`	`+ }`
	`37`	`+`
	`38`	`+ foreach ($definitions as $arr) {`
	`39`	`+ list($id, $definition) = $arr;`
	`40`	`+ list($inner, $renamedId) = $definition->getDecoratedService();`
	`41`	`+`
`31`	`42`	`$definition->setDecoratedService(null);`
`32`	`43`
`33`		`- list($inner, $renamedId) = $decorated;`
`34`	`44`	`if (!$renamedId) {`
`35`	`45`	`$renamedId = $id.'.inner';`
`36`	`46`	`}`
Original file line number	Diff line number	Diff line change
`@@ -150,12 +150,13 @@ public function setFactoryMethod($factoryMethod)`
`150`	`150`	`*`
`151`	`151`	`* @param null\|string $id The decorated service id, use null to remove decoration`
`152`	`152`	`* @param null\|string $renamedId The new decorated service id`
	`153`	`+ * @param int $priority The priority of decoration`
`153`	`154`	`*`
`154`	`155`	`* @return Definition The current instance`
`155`	`156`	`*`
`156`	`157`	`* @throws InvalidArgumentException In case the decorated service id and the new decorated service id are equals.`
`157`	`158`	`*/`
`158`		`- public function setDecoratedService($id, $renamedId = null)`
	`159`	`+ public function setDecoratedService($id, $renamedId = null, $priority = 0)`
`159`	`160`	`{`
`160`	`161`	`if ($renamedId && $id == $renamedId) {`
`161`	`162`	`throw new \InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id));`
`@@ -164,7 +165,7 @@ public function setDecoratedService($id, $renamedId = null)`
`164`	`165`	`if (null === $id) {`
`165`	`166`	`$this->decoratedService = null;`
`166`	`167`	`} else {`
`167`		`- $this->decoratedService = array($id, $renamedId);`
	`168`	`+ $this->decoratedService = array($id, $renamedId, (int) $priority);`
`168`	`169`	`}`
`169`	`170`
`170`	`171`	`return $this;`
`@@ -173,7 +174,7 @@ public function setDecoratedService($id, $renamedId = null)`
`173`	`174`	`/**`
`174`	`175`	`* Gets the service that decorates this service.`
`175`	`176`	`*`
`176`		`- * @return null\|array An array composed of the decorated service id and the new id for it, null if no service is decorated`
	`177`	`+ * @return null\|array An array composed of the decorated service id, the new id for it and the priority of decoration, null if no service is decorated`
`177`	`178`	`*/`
`178`	`179`	`public function getDecoratedService()`
`179`	`180`	`{`
Original file line number	Diff line number	Diff line change
`@@ -173,11 +173,11 @@ public function setLazy($boolean)`
`173`	`173`	`/**`
`174`	`174`	`* {@inheritdoc}`
`175`	`175`	`*/`
`176`		`- public function setDecoratedService($id, $renamedId = null)`
	`176`	`+ public function setDecoratedService($id, $renamedId = null, $priority = 0)`
`177`	`177`	`{`
`178`	`178`	`$this->changes['decorated_service'] = true;`
`179`	`179`
`180`		`- return parent::setDecoratedService($id, $renamedId);`
	`180`	`+ return parent::setDecoratedService($id, $renamedId, $priority);`
`181`	`181`	`}`
`182`	`182`
`183`	`183`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -145,11 +145,14 @@ private function addService($definition, $id, \DOMElement $parent)`
`145`	`145`	`$service->setAttribute('lazy', 'true');`
`146`	`146`	`}`
`147`	`147`	`if (null !== $decorated = $definition->getDecoratedService()) {`
`148`		`- list($decorated, $renamedId) = $decorated;`
	`148`	`+ list($decorated, $renamedId, $priority) = $decorated;`
`149`	`149`	`$service->setAttribute('decorates', $decorated);`
`150`	`150`	`if (null !== $renamedId) {`
`151`	`151`	`$service->setAttribute('decoration-inner-name', $renamedId);`
`152`	`152`	`}`
	`153`	`+ if (0 !== $priority) {`
	`154`	`+ $service->setAttribute('decoration-priority', $priority);`
	`155`	`+ }`
`153`	`156`	`}`
`154`	`157`
`155`	`158`	`foreach ($definition->getTags() as $name => $tags) {`
Original file line number	Diff line number	Diff line change
`@@ -137,11 +137,14 @@ private function addService($id, $definition)`
`137`	`137`	`}`
`138`	`138`
`139`	`139`	`if (null !== $decorated = $definition->getDecoratedService()) {`
`140`		`- list($decorated, $renamedId) = $decorated;`
	`140`	`+ list($decorated, $renamedId, $priority) = $decorated;`
`141`	`141`	`$code .= sprintf(" decorates: %s\n", $decorated);`
`142`	`142`	`if (null !== $renamedId) {`
`143`	`143`	`$code .= sprintf(" decoration_inner_name: %s\n", $renamedId);`
`144`	`144`	`}`
	`145`	`+ if (0 !== $priority) {`
	`146`	`+ $code .= sprintf(" decoration_priority: %s\n", $priority);`
	`147`	`+ }`
`145`	`148`	`}`
`146`	`149`
`147`	`150`	`if ($callable = $definition->getFactory()) {`
Original file line number	Diff line number	Diff line change
`@@ -245,7 +245,8 @@ private function parseDefinition(\DOMElement $service, $file)`
`245`	`245`
`246`	`246`	`if ($value = $service->getAttribute('decorates')) {`
`247`	`247`	`$renameId = $service->hasAttribute('decoration-inner-name') ? $service->getAttribute('decoration-inner-name') : null;`
`248`		`- $definition->setDecoratedService($value, $renameId);`
	`248`	`+ $priority = $service->hasAttribute('decoration-priority') ? $service->getAttribute('decoration-priority') : 0;`
	`249`	`+ $definition->setDecoratedService($value, $renameId, $priority);`
`249`	`250`	`}`
`250`	`251`
`251`	`252`	`return $definition;`
Original file line number	Diff line number	Diff line change
`@@ -290,7 +290,8 @@ private function parseDefinition($id, $service, $file)`
`290`	`290`
`291`	`291`	`if (isset($service['decorates'])) {`
`292`	`292`	`$renameId = isset($service['decoration_inner_name']) ? $service['decoration_inner_name'] : null;`
`293`		`- $definition->setDecoratedService($service['decorates'], $renameId);`
	`293`	`+ $priority = isset($service['decoration_priority']) ? $service['decoration_priority'] : 0;`
	`294`	`+ $definition->setDecoratedService($service['decorates'], $renameId, $priority);`
`294`	`295`	`}`
`295`	`296`
`296`	`297`	`$this->container->setDefinition($id, $definition);`