diff --git a/src/Symfony/Component/DependencyInjection/ChildDefinition.php b/src/Symfony/Component/DependencyInjection/ChildDefinition.php index bafcd3b7b87bf..1607240a42188 100644 --- a/src/Symfony/Component/DependencyInjection/ChildDefinition.php +++ b/src/Symfony/Component/DependencyInjection/ChildDefinition.php @@ -23,7 +23,6 @@ class ChildDefinition extends Definition { private $parent; private $inheritTags = false; - private $changes = array(); /** * @param string $parent The id of Definition instance to decorate @@ -45,16 +44,6 @@ public function getParent() return $this->parent; } - /** - * Returns all changes tracked for the Definition object. - * - * @return array An array of changes for this Definition - */ - public function getChanges() - { - return $this->changes; - } - /** * Sets whether tags should be inherited from the parent or not. * @@ -79,116 +68,6 @@ public function getInheritTags() return $this->inheritTags; } - /** - * {@inheritdoc} - */ - public function setClass($class) - { - $this->changes['class'] = true; - - return parent::setClass($class); - } - - /** - * {@inheritdoc} - */ - public function setFactory($callable) - { - $this->changes['factory'] = true; - - return parent::setFactory($callable); - } - - /** - * {@inheritdoc} - */ - public function setConfigurator($callable) - { - $this->changes['configurator'] = true; - - return parent::setConfigurator($callable); - } - - /** - * {@inheritdoc} - */ - public function setFile($file) - { - $this->changes['file'] = true; - - return parent::setFile($file); - } - - /** - * {@inheritdoc} - */ - public function setShared($boolean) - { - $this->changes['shared'] = true; - - return parent::setShared($boolean); - } - - /** - * {@inheritdoc} - */ - public function setPublic($boolean) - { - $this->changes['public'] = true; - - return parent::setPublic($boolean); - } - - /** - * {@inheritdoc} - */ - public function setLazy($boolean) - { - $this->changes['lazy'] = true; - - return parent::setLazy($boolean); - } - - /** - * {@inheritdoc} - */ - public function setAbstract($boolean) - { - $this->changes['abstract'] = true; - - return parent::setAbstract($boolean); - } - - /** - * {@inheritdoc} - */ - public function setDecoratedService($id, $renamedId = null, $priority = 0) - { - $this->changes['decorated_service'] = true; - - return parent::setDecoratedService($id, $renamedId, $priority); - } - - /** - * {@inheritdoc} - */ - public function setDeprecated($boolean = true, $template = null) - { - $this->changes['deprecated'] = true; - - return parent::setDeprecated($boolean, $template); - } - - /** - * {@inheritdoc} - */ - public function setAutowired($autowired) - { - $this->changes['autowired'] = true; - - return parent::setAutowired($autowired); - } - /** * Gets an argument to pass to the service constructor/factory method. * diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index 47402b8ad8a90..91dc2ddf856ee 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -42,13 +42,13 @@ public function __construct() $this->beforeOptimizationPasses = array( 100 => array( $resolveClassPass = new ResolveClassPass(), - new ResolveDefinitionInheritancePass(), ), ); $this->optimizationPasses = array(array( new ExtensionCompilerPass(), new ResolveDefinitionTemplatesPass(), + new ResolveDefinitionInheritancePass(), new ServiceLocatorTagPass(), new DecoratorServicePass(), new ResolveParameterPlaceHoldersPass(), diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php index f06b19e33b44f..8b11081c53738 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php @@ -11,7 +11,6 @@ namespace Symfony\Component\DependencyInjection\Compiler; -use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Definition; /** @@ -27,7 +26,7 @@ protected function processValue($value, $isRoot = false) return parent::processValue($value, $isRoot); } - $class = $value instanceof ChildDefinition ? $this->resolveDefinition($value) : $value->getClass(); + $class = $value->getClass(); if (!$class || false !== strpos($class, '%') || !$instanceof = $value->getInstanceofConditionals()) { return parent::processValue($value, $isRoot); @@ -39,55 +38,56 @@ protected function processValue($value, $isRoot = false) continue; } if ($interface === $class || is_subclass_of($class, $interface)) { - $this->mergeDefinition($value, $definition); + $this->mergeInstanceofDefinition($value, $definition); } } return parent::processValue($value, $isRoot); } - /** - * Populates the class and tags from parent definitions. - */ - private function resolveDefinition(ChildDefinition $definition) + private function mergeInstanceofDefinition(Definition $def, Definition $instanceofDefinition) { - if (!$this->container->has($parent = $definition->getParent())) { - return; + $configured = $def->getChanges(); + $changes = $instanceofDefinition->getChanges(); + if (!isset($configured['shared']) && isset($changes['shared'])) { + $def->setShared($instanceofDefinition->isShared()); } - - $parentDef = $this->container->findDefinition($parent); - $class = $parentDef instanceof ChildDefinition ? $this->resolveDefinition($parentDef) : $parentDef->getClass(); - $class = $definition->getClass() ?: $class; - - // append parent tags when inheriting is enabled - if ($definition->getInheritTags()) { - $definition->setInheritTags(false); - - foreach ($parentDef->getTags() as $k => $v) { - foreach ($v as $v) { - $definition->addTag($k, $v); - } - } + if (!isset($configured['configurator']) && isset($changes['configurator'])) { + $def->setConfigurator($instanceofDefinition->getConfigurator()); } - - return $class; - } - - private function mergeDefinition(Definition $def, ChildDefinition $definition) - { - $changes = $definition->getChanges(); - if (isset($changes['shared'])) { - $def->setShared($definition->isShared()); + if (!isset($configured['public']) && isset($changes['public'])) { + $def->setPublic($instanceofDefinition->isPublic()); } - if (isset($changes['abstract'])) { - $def->setAbstract($definition->isAbstract()); + if (!isset($configured['lazy']) && isset($changes['lazy'])) { + $def->setLazy($instanceofDefinition->isLazy()); + } + if (!isset($configured['autowired']) && isset($changes['autowired'])) { + $def->setAutowired($instanceofDefinition->isAutowired()); + } + // merge properties + $properties = $def->getProperties(); + foreach ($instanceofDefinition->getProperties() as $k => $v) { + // don't override properties set explicitly on the service + if (!array_key_exists($k, $properties)) { + $def->setProperty($k, $v); + } + } + // merge method calls + if ($instanceofCalls = $instanceofDefinition->getMethodCalls()) { + $currentCallMethods = array_map(function ($call) { + return strtolower($call[0]); + }, $def->getMethodCalls()); + + $uniqueInstanceofCalls = array_filter($instanceofCalls, function ($instanceofCall) use ($currentCallMethods) { + // don't add an instanceof call if it was overridden on the service + return !in_array(strtolower($instanceofCall[0]), $currentCallMethods); + }); + + $def->setMethodCalls(array_merge($uniqueInstanceofCalls, $def->getMethodCalls())); } - - ResolveDefinitionTemplatesPass::mergeDefinition($def, $definition); - // prepend instanceof tags $tailTags = $def->getTags(); - if ($headTags = $definition->getTags()) { + if ($headTags = $instanceofDefinition->getTags()) { $def->setTags($headTags); foreach ($tailTags as $k => $v) { foreach ($v as $v) { diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php index 7d9f7da4b8452..62aad728774d6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php @@ -95,33 +95,27 @@ private function doResolveDefinition(ChildDefinition $definition) if ($parentDef->isDeprecated()) { $def->setDeprecated(true, $parentDef->getDeprecationMessage('%service_id%')); } - $def->setFactory($parentDef->getFactory()); - $def->setConfigurator($parentDef->getConfigurator()); - $def->setFile($parentDef->getFile()); - $def->setPublic($parentDef->isPublic()); - $def->setLazy($parentDef->isLazy()); - $def->setAutowired($parentDef->isAutowired()); - self::mergeDefinition($def, $definition); - - // merge autowiring types - foreach ($definition->getAutowiringTypes(false) as $autowiringType) { - $def->addAutowiringType($autowiringType); + $parentChanges = $parentDef->getChanges(); + if (isset($parentChanges['factory'])) { + $def->setFactory($parentDef->getFactory()); + } + if (isset($parentChanges['configurator'])) { + $def->setConfigurator($parentDef->getConfigurator()); + } + if (isset($parentChanges['file'])) { + $def->setFile($parentDef->getFile()); + } + if (isset($parentChanges['public'])) { + $def->setPublic($parentDef->isPublic()); + } + if (isset($parentChanges['lazy'])) { + $def->setLazy($parentDef->isLazy()); + } + if (isset($parentChanges['autowired'])) { + $def->setAutowired($parentDef->isAutowired()); } - // these attributes are always taken from the child - $def->setAbstract($definition->isAbstract()); - $def->setShared($definition->isShared()); - $def->setTags($definition->getTags()); - - return $def; - } - - /** - * @internal - */ - public static function mergeDefinition(Definition $def, ChildDefinition $definition) - { // overwrite with values specified in the decorator $changes = $definition->getChanges(); if (isset($changes['class'])) { @@ -182,5 +176,31 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit if ($calls = $definition->getMethodCalls()) { $def->setMethodCalls(array_merge($def->getMethodCalls(), $calls)); } + + // merge autowiring types + foreach ($definition->getAutowiringTypes(false) as $autowiringType) { + $def->addAutowiringType($autowiringType); + } + + // these attributes are always taken from the child + if (isset($changes['abstract'])) { + $def->setAbstract($definition->isAbstract()); + } + if (isset($changes['shared'])) { + $def->setShared($definition->isShared()); + } + $def->setTags($definition->getTags()); + $def->setInstanceofConditionals($definition->getInstanceofConditionals()); + + // append parent tags when inheriting is enabled + if ($definition->getInheritTags()) { + foreach ($parentDef->getTags() as $k => $v) { + foreach ($v as $v) { + $def->addTag($k, $v); + } + } + } + + return $def; } } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveParameterPlaceHoldersPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveParameterPlaceHoldersPass.php index d1d6786145bad..2979d742ee865 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveParameterPlaceHoldersPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveParameterPlaceHoldersPass.php @@ -58,10 +58,13 @@ protected function processValue($value, $isRoot = false) return $this->bag->resolveValue($value); } if ($value instanceof Definition) { + // don't record these as new changes to the Definition + $value->setTrackChanges(false); $value->setClass($this->bag->resolveValue($value->getClass())); $value->setFile($this->bag->resolveValue($value->getFile())); $value->setProperties($this->bag->resolveValue($value->getProperties())); $value->setMethodCalls($this->bag->resolveValue($value->getMethodCalls())); + $value->setTrackChanges(true); } return parent::processValue($value, $isRoot); diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php index 94671b80eb42f..d110119e9affb 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php @@ -40,10 +40,13 @@ public function process(ContainerBuilder $container) continue; } + // don't record these as new changes to the Definition + $definition->setTrackChanges(false); $definition->setArguments($this->processArguments($definition->getArguments())); $definition->setMethodCalls($this->processArguments($definition->getMethodCalls())); $definition->setProperties($this->processArguments($definition->getProperties())); $definition->setFactory($this->processFactory($definition->getFactory())); + $definition->setTrackChanges(true); } foreach ($container->getAliases() as $id => $alias) { diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index e3c79c511aa7a..0069587be818d 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -39,6 +39,8 @@ class Definition private $decoratedService; private $autowired = false; private $autowiringTypes = array(); + private $changes = array(); + private $trackChanges = true; protected $arguments; @@ -48,7 +50,9 @@ class Definition */ public function __construct($class = null, array $arguments = array()) { - $this->class = $class; + if (null !== $class) { + $this->setClass($class); + } $this->arguments = $arguments; } @@ -65,6 +69,7 @@ public function setFactory($factory) $factory = explode('::', $factory, 2); } + $this->recordChange('factory'); $this->factory = $factory; return $this; @@ -97,6 +102,8 @@ public function setDecoratedService($id, $renamedId = null, $priority = 0) throw new InvalidArgumentException(sprintf('The decorated service inner name for "%s" must be different than the service name itself.', $id)); } + $this->recordChange('decorated_service'); + if (null === $id) { $this->decoratedService = null; } else { @@ -125,6 +132,7 @@ public function getDecoratedService() */ public function setClass($class) { + $this->recordChange('class'); $this->class = $class; return $this; @@ -331,7 +339,7 @@ public function getMethodCalls() /** * Sets the definition templates to conditionally apply on the current definition, keyed by parent interface/class. * - * @param $instanceof ChildDefinition[] + * @param $instanceof Definition[] */ public function setInstanceofConditionals(array $instanceof) { @@ -343,7 +351,7 @@ public function setInstanceofConditionals(array $instanceof) /** * Gets the definition templates to conditionally apply on the current definition, keyed by parent interface/class. * - * @return ChildDefinition[] + * @return Definition[] */ public function getInstanceofConditionals() { @@ -448,6 +456,7 @@ public function clearTags() */ public function setFile($file) { + $this->recordChange('file'); $this->file = $file; return $this; @@ -472,6 +481,7 @@ public function getFile() */ public function setShared($shared) { + $this->recordChange('shared'); $this->shared = (bool) $shared; return $this; @@ -496,6 +506,7 @@ public function isShared() */ public function setPublic($boolean) { + $this->recordChange('public'); $this->public = (bool) $boolean; return $this; @@ -520,6 +531,7 @@ public function isPublic() */ public function setLazy($lazy) { + $this->recordChange('lazy'); $this->lazy = (bool) $lazy; return $this; @@ -545,6 +557,7 @@ public function isLazy() */ public function setSynthetic($boolean) { + $this->recordChange('synthetic'); $this->synthetic = (bool) $boolean; return $this; @@ -571,6 +584,7 @@ public function isSynthetic() */ public function setAbstract($boolean) { + $this->recordChange('abstract'); $this->abstract = (bool) $boolean; return $this; @@ -612,6 +626,7 @@ public function setDeprecated($status = true, $template = null) $this->deprecationTemplate = $template; } + $this->recordChange('deprecated'); $this->deprecated = (bool) $status; return $this; @@ -653,6 +668,7 @@ public function setConfigurator($configurator) $configurator = explode('::', $configurator, 2); } + $this->recordChange('configurator'); $this->configurator = $configurator; return $this; @@ -709,6 +725,7 @@ public function isAutowired() */ public function setAutowired($autowired) { + $this->recordChange('autowired'); $this->autowired = (bool) $autowired; return $this; @@ -781,4 +798,35 @@ public function hasAutowiringType($type) return isset($this->autowiringTypes[$type]); } + + /** + * Returns all changes tracked for the Definition object. + * + * @return array An array of changes for this Definition + */ + public function getChanges() + { + return $this->changes; + } + + /** + * Turn internal change tracking on or off. + * + * @param bool $trackChanges + * + * @return $this + */ + public function setTrackChanges($trackChanges) + { + $this->trackChanges = $trackChanges; + + return $this; + } + + private function recordChange($type) + { + if ($this->trackChanges) { + $this->changes[$type] = true; + } + } } diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php index 243bced03ddf4..4a9f5e9009a14 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php @@ -11,7 +11,6 @@ namespace Symfony\Component\DependencyInjection\Loader; -use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\ContainerBuilder; use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; @@ -74,9 +73,6 @@ public function registerClasses(Definition $prototype, $namespace, $resource) protected function setDefinition($id, Definition $definition) { if ($this->isLoadingInstanceof) { - if (!$definition instanceof ChildDefinition) { - throw new InvalidArgumentException(sprintf('Invalid type definition "%s": ChildDefinition expected, "%s" given.', $id, get_class($definition))); - } $this->instanceof[$id] = $definition; } else { $this->container->setDefinition($id, $definition->setInstanceofConditionals($this->instanceof)); diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index 798b6b1adcde4..31a967e281a99 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -210,7 +210,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults = } if ($this->isLoadingInstanceof) { - $definition = new ChildDefinition(''); + $definition = new Definition(); } elseif ($parent = $service->getAttribute('parent')) { $definition = new ChildDefinition($parent); @@ -227,7 +227,10 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults = if ($publicAttr = $service->getAttribute('public')) { $definition->setPublic(XmlUtils::phpize($publicAttr)); } elseif (isset($defaults['public'])) { - $definition->setPublic($defaults['public']); + $definition + ->setTrackChanges(false) + ->setPublic($defaults['public']) + ->setTrackChanges(true); } foreach (array('class', 'shared', 'synthetic', 'lazy', 'abstract') as $key) { @@ -240,7 +243,10 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults = if ($value = $service->getAttribute('autowire')) { $definition->setAutowired(XmlUtils::phpize($value)); } elseif (isset($defaults['autowire'])) { - $definition->setAutowired($defaults['autowire']); + $definition + ->setTrackChanges(false) + ->setAutowired($defaults['autowire']) + ->setTrackChanges(true); } if ($files = $this->getChildren($service, 'file')) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 7c7550e6c982b..2bd05dfc708df 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -81,10 +81,6 @@ class YamlFileLoader extends FileLoader 'shared' => 'shared', 'lazy' => 'lazy', 'public' => 'public', - 'abstract' => 'abstract', - 'deprecated' => 'deprecated', - 'factory' => 'factory', - 'arguments' => 'arguments', 'properties' => 'properties', 'configurator' => 'configurator', 'calls' => 'calls', @@ -355,7 +351,7 @@ private function parseDefinition($id, $service, $file, array $defaults) } if ($this->isLoadingInstanceof) { - $definition = new ChildDefinition(''); + $definition = new Definition(); } elseif (isset($service['parent'])) { $definition = new ChildDefinition($service['parent']); @@ -384,9 +380,13 @@ private function parseDefinition($id, $service, $file, array $defaults) $definition->setLazy($service['lazy']); } - $public = isset($service['public']) ? $service['public'] : (isset($defaults['public']) ? $defaults['public'] : null); - if (null !== $public) { - $definition->setPublic($public); + if (isset($service['public'])) { + $definition->setPublic($service['public']); + } elseif (isset($defaults['public'])) { + $definition + ->setTrackChanges(false) + ->setPublic($defaults['public']) + ->setTrackChanges(true); } if (isset($service['abstract'])) { @@ -484,9 +484,22 @@ private function parseDefinition($id, $service, $file, array $defaults) $definition->setDecoratedService($service['decorates'], $renameId, $priority); } - $autowire = isset($service['autowire']) ? $service['autowire'] : (isset($defaults['autowire']) ? $defaults['autowire'] : null); + $autowire = null; + $autowireDefaultsUsed = false; + if (isset($service['autowire'])) { + $autowire = $service['autowire']; + } elseif (isset($defaults['autowire'])) { + $autowire = $defaults['autowire']; + $autowireDefaultsUsed = true; + } + if (null !== $autowire) { - $definition->setAutowired($autowire); + if ($autowireDefaultsUsed) { + $definition->setTrackChanges(false); + } + $definition->setAutowired($autowire) + ->setTrackChanges(true) + ; } if (isset($service['autowiring_types'])) { diff --git a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd index ebbcf00ef487a..79a9ad4637cf6 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd +++ b/src/Symfony/Component/DependencyInjection/Loader/schema/dic/services/services-1.0.xsd @@ -136,10 +136,7 @@ - - - @@ -148,7 +145,6 @@ - diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php index db33d4b4a14dc..ed68be7c84f7c 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/IntegrationTest.php @@ -13,6 +13,8 @@ use PHPUnit\Framework\TestCase; use Symfony\Component\DependencyInjection\Alias; +use Symfony\Component\DependencyInjection\ChildDefinition; +use Symfony\Component\DependencyInjection\Definition; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -114,4 +116,58 @@ public function testProcessInlinesWhenThereAreMultipleReferencesButFromTheSameDe $this->assertFalse($container->hasDefinition('b')); $this->assertFalse($container->hasDefinition('c'), 'Service C was not inlined.'); } + + /** + * Tests that instanceof config applies through parent-child service definitions. + * + * This test involves multiple compiler passes. + */ + public function testConfigurationOverridePriority() + { + $container = new ContainerBuilder(); + $container->setResourceTracking(false); + + $parentDef = $container->register('parent', self::class); + $container->setAlias('parent_alias', 'parent'); + $def = new ChildDefinition('parent_alias'); + $container->setDefinition('child', $def); + + $parentDef + // overrides instanceof below + ->setConfigurator('parent_configurator') + ->addTag('foo', array('foo_tag_attr' => 'bar')) + ->addTag('bar'); + + $def + ->setInheritTags(true) + // overrides instanceof below + ->setAutowired(true) + ->setInstanceofConditionals(array( + parent::class => (new Definition()) + ->setLazy(true) + // both autowired and configurator are overridden + ->setAutowired(false) + ->setConfigurator('instanceof_configurator') + ->addTag('foo') + ->addTag('baz'), + )); + + $container->compile(); + + // instanceof sets this + $childDef = $container->getDefinition('child'); + $this->assertTrue($childDef->isLazy()); + $this->assertTrue($childDef->isAutowired()); + $this->assertEquals('parent_configurator', $childDef->getConfigurator()); + $this->assertSame( + array( + // foo tag is merged together, instanceof first + 'foo' => array(array(), array('foo_tag_attr' => 'bar')), + 'baz' => array(array()), + 'bar' => array(array()), + ), + $container->getDefinition('child')->getTags() + ); + $this->assertFalse($container->hasDefinition('c'), 'Service C was not inlined.'); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionInheritancePassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionInheritancePassTest.php index f521f96002fd7..0c1df4c20d845 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionInheritancePassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionInheritancePassTest.php @@ -12,68 +12,74 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler; use PHPUnit\Framework\TestCase; -use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Compiler\ResolveDefinitionInheritancePass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; class ResolveDefinitionInheritancePassTest extends TestCase { public function testProcess() { $container = new ContainerBuilder(); - $def = $container->register('parent', self::class)->setArguments(array('moo', 'b'))->setProperty('foo', 'moo'); + $def = $container->register('parent', self::class) + ->setProperty('foo', 'moo') + ->setProperty('nullProp', null); $def->setInstanceofConditionals(array( - parent::class => (new ChildDefinition('')) - ->replaceArgument(0, 'a') + parent::class => (new Definition()) + ->setShared(false) + ->setLazy(true) + ->setPublic(false) + ->setConfigurator('instanceof_configurator') + ->addMethodCall('foo_call') + ->addTag('foo_tag') + ->setAutowired(true) ->setProperty('foo', 'bar') - ->setClass('bar'), + ->setProperty('otherProp', 'baz') + ->setProperty('nullProp', 'will_be_overridden'), )); $this->process($container); $this->assertEmpty($def->getInstanceofConditionals()); $this->assertSame($def, $container->getDefinition('parent')); - $this->assertEquals('bar', $def->getClass()); - $this->assertEquals(array('a', 'b'), $def->getArguments()); - $this->assertEquals(array('foo' => 'bar'), $def->getProperties()); + $this->assertFalse($def->isShared()); + $this->assertTrue($def->isLazy()); + $this->assertFalse($def->isPublic()); + $this->assertEquals('instanceof_configurator', $def->getConfigurator()); + $this->assertEquals(array(array('foo_call', array())), $def->getMethodCalls()); + $this->assertEquals(array('foo_tag' => array(array())), $def->getTags()); + $this->assertTrue($def->isAutowired()); + // foo property is not replaced, but otherProp is added + $this->assertEquals(array('foo' => 'moo', 'otherProp' => 'baz', 'nullProp' => null), $def->getProperties()); } - public function testProcessAppendsMethodCallsAlways() + public function testProcessMergesMethodCallsAlways() { $container = new ContainerBuilder(); $def = $container ->register('parent', self::class) - ->addMethodCall('foo', array('bar')); + ->addMethodCall('foo', array('bar')) + ->addMethodCall('setBaz', array('sunshine_baz')); $def->setInstanceofConditionals(array( - parent::class => (new ChildDefinition('')) - ->addMethodCall('bar', array('foo')), + parent::class => (new Definition()) + ->addMethodCall('bar', array('foo')) + // lowercased - should still be overridden + ->addMethodCall('setbaz', array('rainbow_baz')), )); $this->process($container); $this->assertEquals(array( - array('foo', array('bar')), + // instanceof call is first array('bar', array('foo')), + array('foo', array('bar')), + // because it has the same name, the definition overrides instanceof + array('setBaz', array('sunshine_baz')), ), $container->getDefinition('parent')->getMethodCalls()); } - public function testProcessDoesReplaceAbstract() - { - $container = new ContainerBuilder(); - - $def = $container->register('parent', 'stdClass'); - - $def->setInstanceofConditionals(array( - 'stdClass' => (new ChildDefinition(''))->setAbstract(true), - )); - - $this->process($container); - - $this->assertTrue($def->isAbstract()); - } - public function testProcessDoesReplaceShared() { $container = new ContainerBuilder(); @@ -81,7 +87,7 @@ public function testProcessDoesReplaceShared() $def = $container->register('parent', 'stdClass'); $def->setInstanceofConditionals(array( - 'stdClass' => (new ChildDefinition(''))->setShared(false), + 'stdClass' => (new Definition())->setShared(false), )); $this->process($container); @@ -95,17 +101,17 @@ public function testProcessHandlesMultipleInheritance() $def = $container ->register('parent', self::class) - ->setArguments(array('foo', 'bar', 'c')) + ->setProperties(array('foo' => 'fooval', 'bar' => 'barval', 'baz' => 'bazval')) ; $def->setInstanceofConditionals(array( - parent::class => (new ChildDefinition(''))->replaceArgument(1, 'b'), - self::class => (new ChildDefinition(''))->replaceArgument(0, 'a'), + parent::class => (new Definition())->setProperty('bar', 'barval_changed'), + self::class => (new Definition())->setProperty('foo', 'fooval_changed'), )); $this->process($container); - $this->assertEquals(array('a', 'b', 'c'), $def->getArguments()); + $this->assertEquals(array('foo' => 'fooval', 'bar' => 'barval', 'baz' => 'bazval'), $def->getProperties()); } public function testSetLazyOnServiceHasParent() @@ -115,7 +121,7 @@ public function testSetLazyOnServiceHasParent() $def = $container->register('parent', 'stdClass'); $def->setInstanceofConditionals(array( - 'stdClass' => (new ChildDefinition(''))->setLazy(true), + 'stdClass' => (new Definition())->setLazy(true), )); $this->process($container); @@ -127,42 +133,75 @@ public function testProcessInheritTags() { $container = new ContainerBuilder(); - $container->register('parent', self::class)->addTag('parent'); - - $def = $container->setDefinition('child', new ChildDefinition('parent')) - ->addTag('child') - ->setInheritTags(true) + $def = $container->register('parent', self::class) + ->addTag('tag_foo') + ->addTag('tag_bar', array('priority' => 100)) ; $def->setInstanceofConditionals(array( - parent::class => (new ChildDefinition(''))->addTag('foo'), + parent::class => (new Definition()) + ->addTag('tag_bar', array('priority' => 500)) + ->addTag('tag_baz'), )); $this->process($container); $t = array(array()); - $this->assertSame(array('foo' => $t, 'child' => $t, 'parent' => $t), $def->getTags()); + $this->assertSame( + array( + // all tags are kept, and merged together + 'tag_bar' => array(array('priority' => 500), array('priority' => 100)), + // instanceof tags are added first + 'tag_baz' => $t, + 'tag_foo' => $t, + ), + $def->getTags() + ); } - public function testProcessResolvesAliasesAndTags() + /** + * Tests that service configuration cascades in the correct order. + * + * For configuration: defaults > instanceof > service. In + * other words, service-specific configuration is the strongest, + * then instanceof and finally defaults. + */ + public function testConfigurationOverridePriority() { $container = new ContainerBuilder(); - $container->register('parent', self::class); - $container->setAlias('parent_alias', 'parent'); - $def = $container->setDefinition('child', new ChildDefinition('parent_alias')); + $def = $container->register('parent', self::class); + // mimic how _defaults/defaults is loaded in YAML/XML + $def + ->setTrackChanges(false) + ->setPublic(false) + ->setAutowired(true) + ->setTrackChanges(true); + $def->setInstanceofConditionals(array( - parent::class => (new ChildDefinition(''))->addTag('foo'), + parent::class => (new Definition()) + // overrides autowired on _defaults + ->setAutowired(false) + ->setConfigurator('foo_configurator'), )); + $def + // overrides public on _defaults + ->setPublic(true) + // overrides configurator on instanceof + ->setConfigurator('bar_configurator') + ; + $this->process($container); - $this->assertSame(array('foo' => array(array())), $def->getTags()); - $this->assertSame($def, $container->getDefinition('child')); - $this->assertEmpty($def->getClass()); + // service-level wins over instanceof + $this->assertTrue($def->isPublic()); + $this->assertEquals('bar_configurator', $def->getConfigurator()); + // instanceof-level wins over defaults + $this->assertFalse($def->isAutowired()); } - protected function process(ContainerBuilder $container) + private function process(ContainerBuilder $container) { $pass = new ResolveDefinitionInheritancePass(); $pass->process($container); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php index 306804b610f12..e7b473713c637 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveDefinitionTemplatesPassTest.php @@ -15,6 +15,7 @@ use Symfony\Component\DependencyInjection\ChildDefinition; use Symfony\Component\DependencyInjection\Compiler\ResolveDefinitionTemplatesPass; use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Definition; class ResolveDefinitionTemplatesPassTest extends TestCase { @@ -364,6 +365,42 @@ public function testProcessResolvesAliases() $this->assertSame('ParentClass', $def->getClass()); } + public function testProcessInstanceofConditionals() + { + $container = new ContainerBuilder(); + + $container + ->register('parent') + ->setInstanceofConditionals(array('Foo' => (new Definition())->setLazy(true))) + ; + + $conditionals = array('stdClass' => (new Definition())->setAutowired(true), 'Bar' => (new Definition())->setShared(false)); + $container + ->setDefinition('child', new ChildDefinition('parent')) + ->setInstanceofConditionals($conditionals) + ; + + $this->process($container); + + $childDef = $container->getDefinition('child'); + // instanceof taken directly from child, parent ignored + $this->assertSame($conditionals, $childDef->getInstanceofConditionals()); + } + + public function testDefinitionOnlyShowsActualChanges() + { + $container = new ContainerBuilder(); + + $container->register('parent', 'ParentClass'); + + $container->setDefinition('child', new ChildDefinition('parent')); + + $this->process($container); + + $childDef = $container->getDefinition('child'); + $this->assertEquals(array('class' => true), $childDef->getChanges()); + } + protected function process(ContainerBuilder $container) { $pass = new ResolveDefinitionTemplatesPass(); diff --git a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php index 1acc7a3333631..d208c4dec1156 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/DefinitionTest.php @@ -20,20 +20,23 @@ public function testConstructor() { $def = new Definition('stdClass'); $this->assertEquals('stdClass', $def->getClass(), '__construct() takes the class name as its first argument'); + $this->assertSame(array('class' => true), $def->getChanges()); $def = new Definition('stdClass', array('foo')); $this->assertEquals(array('foo'), $def->getArguments(), '__construct() takes an optional array of arguments as its second argument'); + $this->assertSame(array('class' => true), $def->getChanges()); } public function testSetGetFactory() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertSame($def, $def->setFactory('foo'), '->setFactory() implements a fluent interface'); $this->assertEquals('foo', $def->getFactory(), '->getFactory() returns the factory'); $def->setFactory('Foo::bar'); $this->assertEquals(array('Foo', 'bar'), $def->getFactory(), '->setFactory() converts string static method call to the array'); + $this->assertSame(array('factory' => true), $def->getChanges()); } public function testSetGetClass() @@ -45,12 +48,13 @@ public function testSetGetClass() public function testSetGetDecoratedService() { - $def = new Definition('stdClass'); + $def = new Definition(); $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()); + $this->assertSame(array('decorated_service' => true), $def->getChanges()); $def = new Definition('stdClass'); $this->assertNull($def->getDecoratedService()); @@ -111,58 +115,65 @@ public function testExceptionOnEmptyMethodCall() public function testSetGetFile() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertSame($def, $def->setFile('foo'), '->setFile() implements a fluent interface'); $this->assertEquals('foo', $def->getFile(), '->getFile() returns the file to include'); + $this->assertSame(array('file' => true), $def->getChanges()); } public function testSetIsShared() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertTrue($def->isShared(), '->isShared() returns true by default'); $this->assertSame($def, $def->setShared(false), '->setShared() implements a fluent interface'); $this->assertFalse($def->isShared(), '->isShared() returns false if the instance must not be shared'); + $this->assertSame(array('shared' => true), $def->getChanges()); } public function testSetIsPublic() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertTrue($def->isPublic(), '->isPublic() returns true by default'); $this->assertSame($def, $def->setPublic(false), '->setPublic() implements a fluent interface'); $this->assertFalse($def->isPublic(), '->isPublic() returns false if the instance must not be public.'); + $this->assertSame(array('public' => true), $def->getChanges()); } public function testSetIsSynthetic() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertFalse($def->isSynthetic(), '->isSynthetic() returns false by default'); $this->assertSame($def, $def->setSynthetic(true), '->setSynthetic() implements a fluent interface'); $this->assertTrue($def->isSynthetic(), '->isSynthetic() returns true if the service is synthetic.'); + $this->assertSame(array('synthetic' => true), $def->getChanges()); } public function testSetIsLazy() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertFalse($def->isLazy(), '->isLazy() returns false by default'); $this->assertSame($def, $def->setLazy(true), '->setLazy() implements a fluent interface'); $this->assertTrue($def->isLazy(), '->isLazy() returns true if the service is lazy.'); + $this->assertSame(array('lazy' => true), $def->getChanges()); } public function testSetIsAbstract() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertFalse($def->isAbstract(), '->isAbstract() returns false by default'); $this->assertSame($def, $def->setAbstract(true), '->setAbstract() implements a fluent interface'); $this->assertTrue($def->isAbstract(), '->isAbstract() returns true if the instance must not be public.'); + $this->assertSame(array('abstract' => true), $def->getChanges()); } public function testSetIsDeprecated() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertFalse($def->isDeprecated(), '->isDeprecated() returns false by default'); $this->assertSame($def, $def->setDeprecated(true), '->setDeprecated() implements a fluent interface'); $this->assertTrue($def->isDeprecated(), '->isDeprecated() returns true if the instance should not be used anymore.'); $this->assertSame('The "deprecated_service" service is deprecated. You should stop using it, as it will soon be removed.', $def->getDeprecationMessage('deprecated_service'), '->getDeprecationMessage() should return a formatted message template'); + $this->assertSame(array('deprecated' => true), $def->getChanges()); } /** @@ -187,9 +198,10 @@ public function invalidDeprecationMessageProvider() public function testSetGetConfigurator() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertSame($def, $def->setConfigurator('foo'), '->setConfigurator() implements a fluent interface'); $this->assertEquals('foo', $def->getConfigurator(), '->getConfigurator() returns the configurator'); + $this->assertSame(array('configurator' => true), $def->getChanges()); } public function testClearTags() @@ -305,11 +317,12 @@ public function testSetProperty() public function testAutowired() { - $def = new Definition('stdClass'); + $def = new Definition(); $this->assertFalse($def->isAutowired()); $def->setAutowired(true); $this->assertTrue($def->isAutowired()); + $this->assertSame(array('autowired' => true), $def->getChanges()); $def->setAutowired(false); $this->assertFalse($def->isAutowired()); @@ -330,4 +343,63 @@ public function testTypes() $this->assertSame($def, $def->removeAutowiringType('Foo')); $this->assertEquals(array('Bar'), $def->getAutowiringTypes()); } + + public function testChangesNoChanges() + { + $def = new Definition(); + + $this->assertSame(array(), $def->getChanges()); + } + + public function testGetChangesWithChanges() + { + $def = new Definition('stdClass', array('fooarg')); + + $def->setAbstract(true); + $def->setAutowired(true); + $def->setConfigurator('configuration_func'); + $def->setDecoratedService(null); + $def->setDeprecated(true); + $def->setFactory('factory_func'); + $def->setFile('foo.php'); + $def->setLazy(true); + $def->setPublic(true); + $def->setShared(true); + $def->setSynthetic(true); + // changes aren't tracked for these, class or arguments + $def->setInstanceofConditionals(array()); + $def->addTag('foo_tag'); + $def->addMethodCall('methodCall'); + $def->setProperty('fooprop', true); + + $this->assertSame(array( + 'class' => true, + 'abstract' => true, + 'autowired' => true, + 'configurator' => true, + 'decorated_service' => true, + 'deprecated' => true, + 'factory' => true, + 'file' => true, + 'lazy' => true, + 'public' => true, + 'shared' => true, + 'synthetic' => true, + ), $def->getChanges()); + } + + public function testGetChangesWithTrackChanges() + { + $def = new Definition(); + $def->setPublic(true); + + // make an untracked change! + $def->setTrackChanges(false); + $def->setSynthetic(true); + $def->setTrackChanges(true); + + $def->setAbstract(true); + + $this->assertSame(array('public' => true, 'abstract' => true), $def->getChanges()); + } } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/anonymous_services_in_instanceof.yml b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/anonymous_services_in_instanceof.yml index a45a73b993349..8022d03e23cc0 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/anonymous_services_in_instanceof.yml +++ b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/anonymous_services_in_instanceof.yml @@ -5,7 +5,8 @@ services: autowire: true DummyInterface: - arguments: [ !service { class: Anonymous } ] + properties: + fooprop: !service { class: Anonymous } # Ensure next conditionals are not considered as services Bar: diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php index 35d5f60de5dcc..8b628b098798a 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php @@ -648,6 +648,8 @@ public function testDefaults() $this->assertFalse($container->getDefinition('with_defaults')->isPublic()); $this->assertSame(array('foo' => array(array())), $container->getDefinition('with_defaults')->getTags()); $this->assertTrue($container->getDefinition('with_defaults')->isAutowired()); + $this->assertArrayNotHasKey('public', $container->getDefinition('with_defaults')->getChanges()); + $this->assertArrayNotHasKey('autowire', $container->getDefinition('with_defaults')->getChanges()); $this->assertArrayNotHasKey('public', $container->getDefinition('no_defaults_child')->getChanges()); $this->assertArrayNotHasKey('autowire', $container->getDefinition('no_defaults_child')->getChanges()); diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php index 8e4732f99e95e..ccc70a5f419e4 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php @@ -24,7 +24,6 @@ use Symfony\Component\Config\Resource\DirectoryResource; use Symfony\Component\Config\Resource\FileResource; use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass; -use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype; use Symfony\Component\DependencyInjection\Tests\Fixtures\NamedArgumentsDummy; use Symfony\Component\ExpressionLanguage\Expression; @@ -398,6 +397,8 @@ public function testDefaults() $this->assertFalse($container->getDefinition('with_defaults')->isPublic()); $this->assertSame(array('foo' => array(array())), $container->getDefinition('with_defaults')->getTags()); $this->assertTrue($container->getDefinition('with_defaults')->isAutowired()); + $this->assertArrayNotHasKey('public', $container->getDefinition('with_defaults')->getChanges()); + $this->assertArrayNotHasKey('autowire', $container->getDefinition('with_defaults')->getChanges()); $this->assertFalse($container->getAlias('with_defaults_aliased')->isPublic()); $this->assertFalse($container->getAlias('with_defaults_aliased_short')->isPublic()); @@ -532,12 +533,12 @@ public function testAnonymousServicesInInstanceof() $this->assertCount(3, $instanceof); $this->assertArrayHasKey('DummyInterface', $instanceof); - $args = $instanceof['DummyInterface']->getArguments(); - $this->assertCount(1, $args); - $this->assertInstanceOf(Reference::class, $args[0]); - $this->assertTrue($container->has((string) $args[0])); + $properties = $instanceof['DummyInterface']->getProperties(); + $this->assertCount(1, $properties); + $this->assertInstanceOf(Reference::class, $properties['fooprop']); + $this->assertTrue($container->has((string) $properties['fooprop'])); - $anonymous = $container->getDefinition((string) $args[0]); + $anonymous = $container->getDefinition((string) $properties['fooprop']); $this->assertEquals('Anonymous', $anonymous->getClass()); $this->assertFalse($anonymous->isPublic()); $this->assertEmpty($anonymous->getInstanceofConditionals());