From 803dd58002b7bb80d90a1cc09ee3112c05e58c5e Mon Sep 17 00:00:00 2001 From: Johannes Schmitt Date: Thu, 27 Jan 2011 00:14:31 +0100 Subject: [PATCH] add definition inheritance support --- .../Compiler/AnalyzeServiceReferencesPass.php | 4 + .../Compiler/CheckDefinitionValidityPass.php | 11 +- .../Compiler/CheckReferenceScopePass.php | 127 --------------- .../Compiler/CheckReferenceValidityPass.php | 145 ++++++++++++++++++ .../Compiler/PassConfig.php | 14 +- .../RemoveAbstractDefinitionsPass.php | 17 ++ .../Compiler/RepeatedPass.php | 5 + .../ResolveDefinitionTemplatesPass.php | 109 +++++++++++++ .../ResolveInterfaceInjectorsPass.php | 2 +- .../Compiler/ResolveInvalidReferencesPass.php | 3 +- .../ResolveReferencesToAliasesPass.php | 4 + .../DependencyInjection/Definition.php | 42 +++++ .../DefinitionDecorator.php | 98 ++++++++++++ .../DependencyInjection/Dumper/PhpDumper.php | 8 +- .../Loader/XmlFileLoader.php | 10 +- .../Loader/YamlFileLoader.php | 12 +- .../schema/dic/services/services-1.0.xsd | 3 + .../DependencyInjection/SimpleXMLElement.php | 6 + ...php => CheckReferenceValidityPassTest.php} | 19 ++- .../ResolveDefinitionTemplatesPassTest.php | 143 +++++++++++++++++ .../DefinitionDecoratorTest.php | 62 ++++++++ .../DependencyInjection/DefinitionTest.php | 12 ++ .../Fixtures/php/services9.php | 46 +++--- .../Fixtures/php/services_interfaces-2.php | 16 +- 24 files changed, 744 insertions(+), 174 deletions(-) delete mode 100644 src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceScopePass.php create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceValidityPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/RemoveAbstractDefinitionsPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php create mode 100644 src/Symfony/Component/DependencyInjection/DefinitionDecorator.php rename tests/Symfony/Tests/Component/DependencyInjection/Compiler/{CheckReferenceScopePassTest.php => CheckReferenceValidityPassTest.php} (81%) create mode 100644 tests/Symfony/Tests/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPassTest.php create mode 100644 tests/Symfony/Tests/Component/DependencyInjection/DefinitionDecoratorTest.php diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php index 2f56f50364b9..4d3a387a8295 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/AnalyzeServiceReferencesPass.php @@ -50,6 +50,10 @@ public function process(ContainerBuilder $container) $this->graph->clear(); foreach ($container->getDefinitions() as $id => $definition) { + if ($definition->isSynthetic() || $definition->isAbstract()) { + continue; + } + $this->currentId = $id; $this->currentDefinition = $definition; $this->processArguments($definition->getArguments()); diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php index f2bfad29c24e..4b93f9ac0f3c 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckDefinitionValidityPass.php @@ -12,7 +12,7 @@ * Later passes can rely on the following, and specifically do not need to * perform these checks themself: * - * - non synthetic services always have a class set + * - non synthetic, non abstract services always have a class set * - synthetic services are always public * - synthetic services are always of non-prototype scope * @@ -39,8 +39,8 @@ public function process(ContainerBuilder $container) )); } - // non-synthetic service has class - if (!$definition->isSynthetic() && !$definition->getClass()) { + // non-synthetic, non-abstract service has class + if (!$definition->isAbstract() && !$definition->isSynthetic() && !$definition->getClass()) { if ($definition->getFactoryService()) { throw new \RuntimeException(sprintf( 'Please add the class to service "%s" even if it is constructed ' @@ -52,8 +52,9 @@ public function process(ContainerBuilder $container) throw new \RuntimeException(sprintf( 'The definition for "%s" has no class. If you intend to inject ' - .'this service dynamically at runtime, please mark it as synthetic=true, ' - .'otherwise specify a class to get rid of this error.', + .'this service dynamically at runtime, please mark it as synthetic=true. ' + .'If this is an abstract definition solely used by child definitions, ' + .'please add abstract=true, otherwise specify a class to get rid of this error.', $id )); } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceScopePass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceScopePass.php deleted file mode 100644 index 086f752cb461..000000000000 --- a/src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceScopePass.php +++ /dev/null @@ -1,127 +0,0 @@ - - */ -class CheckReferenceScopePass implements CompilerPassInterface -{ - protected $container; - protected $currentId; - protected $currentScope; - protected $currentScopeAncestors; - protected $currentScopeChildren; - - public function process(ContainerBuilder $container) - { - $this->container = $container; - - $children = $this->container->getScopeChildren(); - $ancestors = array(); - - $scopes = $this->container->getScopes(); - foreach ($scopes as $name => $parent) { - $ancestors[$name] = array($parent); - - while (isset($scopes[$parent])) { - $ancestors[$name][] = $parent = $scopes[$parent]; - } - } - - foreach ($container->getDefinitions() as $id => $definition) { - if ($definition->isSynthetic()) { - continue; - } - - $this->currentId = $id; - $this->currentScope = $scope = $definition->getScope(); - - if (ContainerInterface::SCOPE_PROTOTYPE === $scope) { - continue; - } - - if (ContainerInterface::SCOPE_CONTAINER === $scope) { - $this->currentScopeChildren = array_keys($scopes); - $this->currentScopeAncestors = array(); - } else { - $this->currentScopeChildren = $children[$scope]; - $this->currentScopeAncestors = $ancestors[$scope]; - } - - $this->validateReferences($definition->getArguments()); - $this->validateReferences($definition->getMethodCalls()); - } - } - - protected function validateReferences(array $arguments) - { - foreach ($arguments as $argument) { - if (is_array($argument)) { - $this->validateReferences($argument); - } elseif ($argument instanceof Reference) { - if (!$argument->isStrict()) { - continue; - } - - if (null === $definition = $this->getDefinition($id = (string) $argument)) { - continue; - } - - if ($this->currentScope === $scope = $definition->getScope()) { - continue; - } - - if (in_array($scope, $this->currentScopeChildren, true)) { - throw new \RuntimeException(sprintf( - 'Scope Widening Injection detected: The definition "%s" references the service "%s" which belongs to a narrower scope. ' - .'Generally, it is safer to either move "%s" to scope "%s" or alternatively rely on the provider pattern by injecting the container itself, and requesting the service "%s" each time it is needed. ' - .'In rare, special cases however that might not be necessary, then you can set the reference to strict=false to get rid of this error.', - $this->currentId, - $id, - $this->currentId, - $scope, - $id - )); - } - - if (!in_array($scope, $this->currentScopeAncestors, true)) { - throw new \RuntimeException(sprintf( - 'Cross-Scope Injection detected: The definition "%s" references the service "%s" which belongs to another scope hierarchy. ' - .'This service might not be available consistently. Generally, it is safer to either move the definition "%s" to scope "%s", or ' - .'declare "%s" as a child scope of "%s". If you can be sure that the other scope is always active, you can set the reference to strict=false to get rid of this error.', - $this->currentId, - $id, - $this->currentId, - $scope, - $this->currentScope, - $scope - )); - } - } - } - } - - protected function getDefinition($id) - { - if (!$this->container->hasDefinition($id)) { - return null; - } - - return $this->container->getDefinition($id); - } -} diff --git a/src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceValidityPass.php b/src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceValidityPass.php new file mode 100644 index 000000000000..c8606ff1861a --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/CheckReferenceValidityPass.php @@ -0,0 +1,145 @@ + + */ +class CheckReferenceValidityPass implements CompilerPassInterface +{ + protected $container; + protected $currentId; + protected $currentDefinition; + protected $currentScope; + protected $currentScopeAncestors; + protected $currentScopeChildren; + + public function process(ContainerBuilder $container) + { + $this->container = $container; + + $children = $this->container->getScopeChildren(); + $ancestors = array(); + + $scopes = $this->container->getScopes(); + foreach ($scopes as $name => $parent) { + $ancestors[$name] = array($parent); + + while (isset($scopes[$parent])) { + $ancestors[$name][] = $parent = $scopes[$parent]; + } + } + + foreach ($container->getDefinitions() as $id => $definition) { + if ($definition->isSynthetic() || $definition->isAbstract()) { + continue; + } + + $this->currentId = $id; + $this->currentDefinition = $definition; + $this->currentScope = $scope = $definition->getScope(); + + if (ContainerInterface::SCOPE_CONTAINER === $scope) { + $this->currentScopeChildren = array_keys($scopes); + $this->currentScopeAncestors = array(); + } else if (ContainerInterface::SCOPE_PROTOTYPE !== $scope) { + $this->currentScopeChildren = $children[$scope]; + $this->currentScopeAncestors = $ancestors[$scope]; + } + + $this->validateReferences($definition->getArguments()); + $this->validateReferences($definition->getMethodCalls()); + } + } + + protected function validateReferences(array $arguments) + { + foreach ($arguments as $argument) { + if (is_array($argument)) { + $this->validateReferences($argument); + } elseif ($argument instanceof Reference) { + $targetDefinition = $this->getDefinition((string) $argument); + + if (null !== $targetDefinition && $targetDefinition->isAbstract()) { + throw new \RuntimeException(sprintf( + 'The definition "%s" has a reference to an abstract definition "%s". ' + .'Abstract definitions cannot be the target of references.', + $this->currentId, + $argument + )); + } + + $this->validateScope($argument, $targetDefinition); + } + } + } + + protected function validateScope(Reference $reference, Definition $definition = null) + { + if (ContainerInterface::SCOPE_PROTOTYPE === $this->currentScope) { + return; + } + + if (!$reference->isStrict()) { + return; + } + + if (null === $definition) { + return; + } + + if ($this->currentScope === $scope = $definition->getScope()) { + return; + } + + $id = (string) $reference; + + if (in_array($scope, $this->currentScopeChildren, true)) { + throw new \RuntimeException(sprintf( + 'Scope Widening Injection detected: The definition "%s" references the service "%s" which belongs to a narrower scope. ' + .'Generally, it is safer to either move "%s" to scope "%s" or alternatively rely on the provider pattern by injecting the container itself, and requesting the service "%s" each time it is needed. ' + .'In rare, special cases however that might not be necessary, then you can set the reference to strict=false to get rid of this error.', + $this->currentId, + $id, + $this->currentId, + $scope, + $id + )); + } + + if (!in_array($scope, $this->currentScopeAncestors, true)) { + throw new \RuntimeException(sprintf( + 'Cross-Scope Injection detected: The definition "%s" references the service "%s" which belongs to another scope hierarchy. ' + .'This service might not be available consistently. Generally, it is safer to either move the definition "%s" to scope "%s", or ' + .'declare "%s" as a child scope of "%s". If you can be sure that the other scope is always active, you can set the reference to strict=false to get rid of this error.', + $this->currentId, + $id, + $this->currentId, + $scope, + $this->currentScope, + $scope + )); + } + } + + protected function getDefinition($id) + { + if (!$this->container->hasDefinition($id)) { + return null; + } + + return $this->container->getDefinition($id); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index d613e498f3e8..527acd009444 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -42,6 +42,7 @@ public function __construct() $this->beforeRemovingPasses = array(); $this->optimizationPasses = array( + new ResolveDefinitionTemplatesPass(), new ResolveParameterPlaceHoldersPass(), new CheckDefinitionValidityPass(), new ResolveReferencesToAliasesPass(), @@ -49,11 +50,12 @@ public function __construct() new ResolveInvalidReferencesPass(), new AnalyzeServiceReferencesPass(true), new CheckCircularReferencesPass(), - new CheckReferenceScopePass(), + new CheckReferenceValidityPass(), ); $this->removingPasses = array( new RemovePrivateAliasesPass(), + new RemoveAbstractDefinitionsPass(), new ReplaceAliasByActualDefinitionPass(), new RepeatedPass(array( new AnalyzeServiceReferencesPass(), @@ -87,6 +89,11 @@ public function addPass(CompilerPassInterface $pass, $type = self::TYPE_BEFORE_O $passes[] = $pass; } + public function getAfterRemovingPasses() + { + return $this->afterRemovingPasses; + } + public function getBeforeOptimizationPasses() { return $this->beforeOptimizationPasses; @@ -117,6 +124,11 @@ public function setMergePass(CompilerPassInterface $pass) $this->mergePass = $pass; } + public function setAfterRemovingPasses(array $passes) + { + $this->afterRemovingPasses = $passes; + } + public function setBeforeOptimizationPasses(array $passes) { $this->beforeOptimizationPasses = $passes; diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RemoveAbstractDefinitionsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RemoveAbstractDefinitionsPass.php new file mode 100644 index 000000000000..34d9e51248da --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/RemoveAbstractDefinitionsPass.php @@ -0,0 +1,17 @@ +getDefinitions() as $id => $definition) { + if ($definition->isAbstract()) { + $container->remove($id); + } + } + } +} \ No newline at end of file diff --git a/src/Symfony/Component/DependencyInjection/Compiler/RepeatedPass.php b/src/Symfony/Component/DependencyInjection/Compiler/RepeatedPass.php index 467f5c12ba81..911b8576f390 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/RepeatedPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/RepeatedPass.php @@ -57,4 +57,9 @@ public function setRepeat() { $this->repeat = true; } + + public function getPasses() + { + return $this->passes; + } } \ No newline at end of file diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php new file mode 100644 index 000000000000..a0cc8213f541 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php @@ -0,0 +1,109 @@ + + */ +class ResolveDefinitionTemplatesPass implements CompilerPassInterface +{ + protected $container; + + public function process(ContainerBuilder $container) + { + $this->container = $container; + foreach (array_keys($container->getDefinitions()) as $id) { + // yes, we are specifically fetching the definition from the + // container to ensure we are not operating on stale data + $definition = $container->getDefinition($id); + if (!$definition instanceof DefinitionDecorator) { + continue; + } + + $this->resolveDefinition($id, $definition); + } + } + + protected function resolveDefinition($id, DefinitionDecorator $definition) + { + if (!$this->container->hasDefinition($parent = $definition->getParent())) { + throw new \RuntimeException(sprintf('The parent definition "%s" defined for definition "%s" does not exist.', $parent, $id)); + } + + $parentDef = $this->container->getDefinition($parent); + if ($parentDef instanceof DefinitionDecorator) { + $parentDef = $this->resolveDefinition($parent, $parentDef); + } + + $def = new Definition(); + + // merge in parent definition + // purposely ignored attributes: scope, abstract, tags + $def->setClass($parentDef->getClass()); + $def->setArguments($parentDef->getArguments()); + $def->setMethodCalls($parentDef->getMethodCalls()); + $def->setFactoryService($parentDef->getFactoryService()); + $def->setFactoryMethod($parentDef->getFactoryMethod()); + $def->setConfigurator($parentDef->getConfigurator()); + $def->setFile($parentDef->getFile()); + $def->setPublic($parentDef->isPublic()); + + // overwrite with values specified in the decorator + $changes = $definition->getChanges(); + if (isset($changes['class'])) { + $def->setClass($definition->getClass()); + } + if (isset($changes['factory_method'])) { + $def->setFactoryMethod($definition->getFactoryMethod()); + } + if (isset($changes['factory_service'])) { + $def->setFactoryService($definition->getFactoryService()); + } + if (isset($changes['configurator'])) { + $def->setConfigurator($definition->getConfigurator()); + } + if (isset($changes['file'])) { + $def->setFile($definition->getFile()); + } + if (isset($changes['public'])) { + $def->setPublic($definition->isPublic()); + } + + // merge arguments + foreach ($definition->getArguments() as $k => $v) { + if (is_numeric($k)) { + $def->addArgument($v); + continue; + } + + if (0 !== strpos($k, 'index_')) { + throw new \RuntimeException(sprintf('Invalid argument key "%s" found.', $k)); + } + + $index = (integer) substr($k, strlen('index_')); + $def->setArgument($index, $v); + } + + // append method calls + if (count($calls = $definition->getMethodCalls()) > 0) { + $def->setMethodCalls(array_merge($def->getMethodCalls(), $calls)); + } + + // these attributes are always taken from the child + $def->setAbstract($definition->isAbstract()); + $def->setScope($definition->getScope()); + $def->setTags($definition->getTags()); + + // set new definition on container + $this->container->setDefinition($id, $def); + + return $def; + } +} \ No newline at end of file diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInterfaceInjectorsPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInterfaceInjectorsPass.php index 3552a6306c0f..1373665cd039 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInterfaceInjectorsPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInterfaceInjectorsPass.php @@ -26,7 +26,7 @@ class ResolveInterfaceInjectorsPass implements CompilerPassInterface public function process(ContainerBuilder $container) { foreach ($container->getDefinitions() as $definition) { - if ($definition->isSynthetic()) { + if (!$definition->getClass()) { continue; } diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php index 9c65cd9a0baa..9dca3dc87ea9 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveInvalidReferencesPass.php @@ -40,7 +40,7 @@ public function process(ContainerBuilder $container) { $this->container = $container; foreach ($container->getDefinitions() as $definition) { - if ($definition->isSynthetic()) { + if ($definition->isSynthetic() || $definition->isAbstract()) { continue; } @@ -75,6 +75,7 @@ protected function processArguments(array $arguments, $inMethodCall = false) $invalidBehavior = $argument->getInvalidBehavior(); $exists = $this->container->has($id); + // resolve invalid behavior if ($exists && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $invalidBehavior) { $arguments[$k] = new Reference($id); } else if (!$exists && ContainerInterface::NULL_ON_INVALID_REFERENCE === $invalidBehavior) { diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php index c1d99d1968e0..06050a1bc7a1 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveReferencesToAliasesPass.php @@ -30,6 +30,10 @@ public function process(ContainerBuilder $container) foreach ($container->getDefinitions() as $id => $definition) { + if ($definition->isSynthetic() || $definition->isAbstract()) { + continue; + } + $definition->setArguments($this->processArguments($definition->getArguments())); $definition->setMethodCalls($this->processArguments($definition->getMethodCalls())); } diff --git a/src/Symfony/Component/DependencyInjection/Definition.php b/src/Symfony/Component/DependencyInjection/Definition.php index a24adbc6777e..7ee19496531a 100644 --- a/src/Symfony/Component/DependencyInjection/Definition.php +++ b/src/Symfony/Component/DependencyInjection/Definition.php @@ -29,6 +29,7 @@ class Definition protected $tags; protected $public; protected $synthetic; + protected $abstract; /** * Constructor. @@ -45,6 +46,7 @@ public function __construct($class = null, array $arguments = array()) $this->tags = array(); $this->public = true; $this->synthetic = false; + $this->abstract = false; } /** @@ -255,6 +257,20 @@ public function getMethodCalls() return $this->calls; } + /** + * Sets tags for this definition + * + * @param array $tags + * + * @return Definition the current instance + */ + public function setTags(array $tags) + { + $this->tags = $tags; + + return $this; + } + /** * Returns all tags. * @@ -417,6 +433,32 @@ public function isSynthetic() return $this->synthetic; } + /** + * Whether this definition is abstract, that means it merely serves as a + * template for other definitions. + * + * @param Boolean $boolean + * + * @return Definition the current instance + */ + public function setAbstract($boolean) + { + $this->abstract = (Boolean) $boolean; + + return $this; + } + + /** + * Whether this definition is abstract, that means it merely serves as a + * template for other definitions. + * + * @return Boolean + */ + public function isAbstract() + { + return $this->abstract; + } + /** * Sets a configurator to call after the service is fully initialized. * diff --git a/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php new file mode 100644 index 000000000000..cde424c50bf1 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/DefinitionDecorator.php @@ -0,0 +1,98 @@ + + */ +class DefinitionDecorator extends Definition +{ + protected $parent; + protected $changes; + + public function __construct($parent) + { + parent::__construct(); + + $this->parent = $parent; + $this->changes = array(); + } + + public function getParent() + { + return $this->parent; + } + + public function getChanges() + { + return $this->changes; + } + + public function setClass($class) + { + $this->changes['class'] = true; + + return parent::setClass($class); + } + + public function setFactoryService($service) + { + $this->changes['factory_service'] = true; + + return parent::setFactoryService($service); + } + + public function setFactoryMethod($method) + { + $this->changes['factory_method'] = true; + + return parent::setFactoryMethod($method); + } + + public function setConfigurator($callable) + { + $this->changes['configurator'] = true; + + return parent::setConfigurator($callable); + } + + public function setFile($file) + { + $this->changes['file'] = true; + + return parent::setFile($file); + } + + public function setPublic($boolean) + { + $this->changes['public'] = true; + + return parent::setPublic($boolean); + } + + /** + * You should always use this method when overwriting existing arguments + * of the parent definition. + * + * If you directly call setArguments() keep in mind that you must follow + * certain conventions when you want to overwrite the arguments of the + * parent definition, otherwise your arguments will only be appended. + * + * @param integer $index + * @param mixed $value + * + * @return DefinitionDecorator the current instance + */ + public function setArgument($index, $value) + { + if (!is_int($index)) { + throw new \InvalidArgumentException('$index must be an integer.'); + } + + $this->arguments['index_'.$index] = $value; + + return $this; + } +} \ No newline at end of file diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index e052fe449b76..5cb3a4e48884 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -494,7 +494,9 @@ protected function get{$name}Service() protected function addServices() { $publicServices = $privateServices = $aliasServices = ''; - foreach ($this->container->getDefinitions() as $id => $definition) { + $definitions = $this->container->getDefinitions(); + ksort($definitions); + foreach ($definitions as $id => $definition) { if ($definition->isPublic()) { $publicServices .= $this->addService($id, $definition); } else { @@ -502,7 +504,9 @@ protected function addServices() } } - foreach ($this->container->getAliases() as $alias => $id) { + $aliases = $this->container->getAliases(); + ksort($aliases); + foreach ($aliases as $alias => $id) { $aliasServices .= $this->addServiceAlias($alias, $id); } diff --git a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php index e03423e0c826..ec0a10625ee2 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php @@ -11,6 +11,8 @@ namespace Symfony\Component\DependencyInjection\Loader; +use Symfony\Component\DependencyInjection\DefinitionDecorator; + use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\InterfaceInjector; @@ -135,9 +137,13 @@ protected function parseDefinition($id, $service, $file) return; } - $definition = new Definition(); + if (isset($service['parent'])) { + $definition = new DefinitionDecorator($service['parent']); + } else { + $definition = new Definition(); + } - foreach (array('class', 'scope', 'public', 'factory-method', 'factory-service', 'synthetic') as $key) { + foreach (array('class', 'scope', 'public', 'factory-method', 'factory-service', 'synthetic', 'abstract') as $key) { if (isset($service[$key])) { $method = 'set'.str_replace('-', '', $key); $definition->$method((string) $service->getAttributeAsPhp($key)); diff --git a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php index 39534375a412..65def0455f83 100644 --- a/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php +++ b/src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php @@ -11,6 +11,8 @@ namespace Symfony\Component\DependencyInjection\Loader; +use Symfony\Component\DependencyInjection\DefinitionDecorator; + use Symfony\Component\DependencyInjection\Alias; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\InterfaceInjector; @@ -137,7 +139,11 @@ protected function parseDefinition($id, $service, $file) return; } - $definition = new Definition(); + if (isset($service['parent'])) { + $definition = new DefinitionDecorator($service['parent']); + } else { + $definition = new Definition(); + } if (isset($service['class'])) { $definition->setClass($service['class']); @@ -155,6 +161,10 @@ protected function parseDefinition($id, $service, $file) $definition->setPublic($service['public']); } + if (isset($service['abstract'])) { + $definition->setAbstract($service['abstract']); + } + if (isset($service['factory_method'])) { $definition->setFactoryMethod($service['factory_method']); } 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 3ee5e98228b1..a6ccf726ee44 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 @@ -104,9 +104,11 @@ + + @@ -140,6 +142,7 @@ + diff --git a/src/Symfony/Component/DependencyInjection/SimpleXMLElement.php b/src/Symfony/Component/DependencyInjection/SimpleXMLElement.php index 98fb282fc4ac..fb7670bc564d 100644 --- a/src/Symfony/Component/DependencyInjection/SimpleXMLElement.php +++ b/src/Symfony/Component/DependencyInjection/SimpleXMLElement.php @@ -34,6 +34,12 @@ public function getArgumentsAsPhp($name) $key = strtolower($key); } + // this is used by DefinitionDecorator to overwrite a specific + // argument of the parent definition + if (isset($arg['index'])) { + $key = 'index_'.$arg['index']; + } + switch ($arg['type']) { case 'service': $invalidBehavior = ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE; diff --git a/tests/Symfony/Tests/Component/DependencyInjection/Compiler/CheckReferenceScopePassTest.php b/tests/Symfony/Tests/Component/DependencyInjection/Compiler/CheckReferenceValidityPassTest.php similarity index 81% rename from tests/Symfony/Tests/Component/DependencyInjection/Compiler/CheckReferenceScopePassTest.php rename to tests/Symfony/Tests/Component/DependencyInjection/Compiler/CheckReferenceValidityPassTest.php index c9373fc3ad17..e1e595459b77 100644 --- a/tests/Symfony/Tests/Component/DependencyInjection/Compiler/CheckReferenceScopePassTest.php +++ b/tests/Symfony/Tests/Component/DependencyInjection/Compiler/CheckReferenceValidityPassTest.php @@ -2,12 +2,12 @@ namespace Symfony\Tests\Component\DependencyInjection\Compiler; -use Symfony\Component\DependencyInjection\Compiler\CheckReferenceScopePass; +use Symfony\Component\DependencyInjection\Compiler\CheckReferenceValidityPass; use Symfony\Component\DependencyInjection\ContainerInterface; use Symfony\Component\DependencyInjection\Reference; use Symfony\Component\DependencyInjection\ContainerBuilder; -class CheckReferenceScopePassTest extends \PHPUnit_Framework_TestCase +class CheckReferenceValidityPassTest extends \PHPUnit_Framework_TestCase { public function testProcessIgnoresScopeWideningIfNonStrictReference() { @@ -57,6 +57,19 @@ public function testProcessDetectsCrossScopeHierarchyReference() $this->process($container); } + /** + * @expectedException \RuntimeException + */ + public function testProcessDetectsReferenceToAbstractDefinition() + { + $container = new ContainerBuilder(); + + $container->register('a')->setAbstract(true); + $container->register('b')->addArgument(new Reference('a')); + + $this->process($container); + } + public function testProcess() { $container = new ContainerBuilder(); @@ -68,7 +81,7 @@ public function testProcess() protected function process(ContainerBuilder $container) { - $pass = new CheckReferenceScopePass(); + $pass = new CheckReferenceValidityPass(); $pass->process($container); } } \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPassTest.php b/tests/Symfony/Tests/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPassTest.php new file mode 100644 index 000000000000..ef5f6e0d28b4 --- /dev/null +++ b/tests/Symfony/Tests/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPassTest.php @@ -0,0 +1,143 @@ +register('parent', 'foo')->setArguments(array('moo', 'b')); + $container->setDefinition('child', new DefinitionDecorator('parent')) + ->setArgument(0, 'a') + ->setClass('bar') + ; + + $this->process($container); + + $def = $container->getDefinition('child'); + $this->assertNotInstanceOf('Symfony\Component\DependencyInjection\DefinitionDecorator', $def); + $this->assertEquals('bar', $def->getClass()); + $this->assertEquals(array('a', 'b'), $def->getArguments()); + } + + public function testProcessAppendsMethodCallsAlways() + { + $container = new ContainerBuilder(); + + $container + ->register('parent') + ->addMethodCall('foo', array('bar')) + ; + + $container + ->setDefinition('child', new DefinitionDecorator('parent')) + ->addMethodCall('bar', array('foo')) + ; + + $this->process($container); + + $def = $container->getDefinition('child'); + $this->assertEquals(array( + array('foo', array('bar')), + array('bar', array('foo')), + ), $def->getMethodCalls()); + } + + public function testProcessDoesNotCopyAbstract() + { + $container = new ContainerBuilder(); + + $container + ->register('parent') + ->setAbstract(true) + ; + + $container + ->setDefinition('child', new DefinitionDecorator('parent')) + ; + + $this->process($container); + + $def = $container->getDefinition('child'); + $this->assertFalse($def->isAbstract()); + } + + public function testProcessDoesNotCopyScope() + { + $container = new ContainerBuilder(); + + $container + ->register('parent') + ->setScope('foo') + ; + + $container + ->setDefinition('child', new DefinitionDecorator('parent')) + ; + + $this->process($container); + + $def = $container->getDefinition('child'); + $this->assertEquals(ContainerInterface::SCOPE_CONTAINER, $def->getScope()); + } + + public function testProcessDoesNotCopyTags() + { + $container = new ContainerBuilder(); + + $container + ->register('parent') + ->addTag('foo') + ; + + $container + ->setDefinition('child', new DefinitionDecorator('parent')) + ; + + $this->process($container); + + $def = $container->getDefinition('child'); + $this->assertEquals(array(), $def->getTags()); + } + + public function testProcessHandlesMultipleInheritance() + { + $container = new ContainerBuilder(); + + $container + ->register('parent', 'foo') + ->setArguments(array('foo', 'bar', 'c')) + ; + + $container + ->setDefinition('child2', new DefinitionDecorator('child1')) + ->setArgument(1, 'b') + ; + + $container + ->setDefinition('child1', new DefinitionDecorator('parent')) + ->setArgument(0, 'a') + ; + + $this->process($container); + + $def = $container->getDefinition('child2'); + $this->assertEquals(array('a', 'b', 'c'), $def->getArguments()); + $this->assertEquals('foo', $def->getClass()); + } + + protected function process(ContainerBuilder $container) + { + $pass = new ResolveDefinitionTemplatesPass(); + $pass->process($container); + } +} \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/DependencyInjection/DefinitionDecoratorTest.php b/tests/Symfony/Tests/Component/DependencyInjection/DefinitionDecoratorTest.php new file mode 100644 index 000000000000..ff80fba5d83b --- /dev/null +++ b/tests/Symfony/Tests/Component/DependencyInjection/DefinitionDecoratorTest.php @@ -0,0 +1,62 @@ +assertEquals('foo', $def->getParent()); + $this->assertEquals(array(), $def->getChanges()); + } + + /** + * @dataProvider getPropertyTests + */ + public function testSetProperty($property, $changeKey) + { + $def = new DefinitionDecorator('foo'); + + $getter = 'get'.ucfirst($property); + $setter = 'set'.ucfirst($property); + + $this->assertNull($def->$getter()); + $this->assertSame($def, $def->$setter('foo')); + $this->assertEquals('foo', $def->$getter()); + $this->assertEquals(array($changeKey => true), $def->getChanges()); + } + + public function getPropertyTests() + { + return array( + array('class', 'class'), + array('factoryService', 'factory_service'), + array('factoryMethod', 'factory_method'), + array('configurator', 'configurator'), + array('file', 'file'), + ); + } + + public function testSetPublic() + { + $def = new DefinitionDecorator('foo'); + + $this->assertTrue($def->isPublic()); + $this->assertSame($def, $def->setPublic(false)); + $this->assertFalse($def->isPublic()); + $this->assertEquals(array('public' => true), $def->getChanges()); + } + + public function testSetArgument() + { + $def = new DefinitionDecorator('foo'); + + $this->assertEquals(array(), $def->getArguments()); + $this->assertSame($def, $def->setArgument(0, 'foo')); + $this->assertEquals(array('index_0' => 'foo'), $def->getArguments()); + } +} \ No newline at end of file diff --git a/tests/Symfony/Tests/Component/DependencyInjection/DefinitionTest.php b/tests/Symfony/Tests/Component/DependencyInjection/DefinitionTest.php index c7c6b79c0054..41ded4f06bc3 100644 --- a/tests/Symfony/Tests/Component/DependencyInjection/DefinitionTest.php +++ b/tests/Symfony/Tests/Component/DependencyInjection/DefinitionTest.php @@ -137,6 +137,18 @@ public function testSetIsSynthetic() $this->assertTrue($def->isSynthetic(), '->isSynthetic() returns true if the instance must not be public.'); } + /** + * @covers Symfony\Component\DependencyInjection\Definition::setAbstract + * @covers Symfony\Component\DependencyInjection\Definition::isAbstract + */ + public function testSetIsAbstract() + { + $def = new Definition('stdClass'); + $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.'); + } + /** * @covers Symfony\Component\DependencyInjection\Definition::setConfigurator * @covers Symfony\Component\DependencyInjection\Definition::getConfigurator diff --git a/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services9.php b/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services9.php index 7d3e5f09d3bb..0d1607fa2d8b 100644 --- a/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services9.php +++ b/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services9.php @@ -23,34 +23,47 @@ public function __construct() } /** - * Gets the 'foo' service. + * Gets the 'bar' service. + * + * This service is shared. + * This method always returns the same instance of the service. * * @return FooClass A FooClass instance. */ - protected function getFooService() + protected function getBarService() { - $instance = call_user_func(array('FooClass', 'getInstance'), 'foo', $this->get('foo.baz'), array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo'), 'bar' => $this->getParameter('foo')), true, $this); + $this->services['bar'] = $instance = new \FooClass('foo', $this->get('foo.baz'), $this->getParameter('foo_bar')); - $instance->setBar($this->get('bar')); - $instance->initialize(); - sc_configure($instance); + $this->get('foo.baz')->configure($instance); return $instance; } /** - * Gets the 'bar' service. + * Gets the 'factory_service' service. * * This service is shared. * This method always returns the same instance of the service. * + * @return Object An instance returned by foo.baz::getInstance(). + */ + protected function getFactoryServiceService() + { + return $this->services['factory_service'] = $this->get('foo.baz')->getInstance(); + } + + /** + * Gets the 'foo' service. + * * @return FooClass A FooClass instance. */ - protected function getBarService() + protected function getFooService() { - $this->services['bar'] = $instance = new \FooClass('foo', $this->get('foo.baz'), $this->getParameter('foo_bar')); + $instance = call_user_func(array('FooClass', 'getInstance'), 'foo', $this->get('foo.baz'), array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo'), 'bar' => $this->getParameter('foo')), true, $this); - $this->get('foo.baz')->configure($instance); + $instance->setBar($this->get('bar')); + $instance->initialize(); + sc_configure($instance); return $instance; } @@ -112,19 +125,6 @@ protected function getMethodCall1Service() return $instance; } - /** - * Gets the 'factory_service' service. - * - * This service is shared. - * This method always returns the same instance of the service. - * - * @return Object An instance returned by foo.baz::getInstance(). - */ - protected function getFactoryServiceService() - { - return $this->services['factory_service'] = $this->get('foo.baz')->getInstance(); - } - /** * Gets the alias_for_foo service alias. * diff --git a/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services_interfaces-2.php b/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services_interfaces-2.php index 243e6c4a4a0d..3104010daee8 100755 --- a/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services_interfaces-2.php +++ b/tests/Symfony/Tests/Component/DependencyInjection/Fixtures/php/services_interfaces-2.php @@ -23,31 +23,31 @@ public function __construct() } /** - * Gets the 'barfactory' service. + * Gets the 'bar' service. * * This service is shared. * This method always returns the same instance of the service. * - * @return BarClassFactory A BarClassFactory instance. + * @return Object An instance returned by barFactory::createBarClass(). */ - protected function getBarfactoryService() + protected function getBarService() { - return $this->services['barfactory'] = new \BarClassFactory(); + return $this->services['bar'] = $this->get('barFactory')->createBarClass(); $this->applyInterfaceInjectors($instance); } /** - * Gets the 'bar' service. + * Gets the 'barfactory' service. * * This service is shared. * This method always returns the same instance of the service. * - * @return Object An instance returned by barFactory::createBarClass(). + * @return BarClassFactory A BarClassFactory instance. */ - protected function getBarService() + protected function getBarfactoryService() { - return $this->services['bar'] = $this->get('barFactory')->createBarClass(); + return $this->services['barfactory'] = new \BarClassFactory(); $this->applyInterfaceInjectors($instance); }