Skip to content

Commit

Permalink
feature #22356 [DI] Rework config hierarchy: defaults > instanceof > …
Browse files Browse the repository at this point in the history 
…service config (weaverryan, nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Rework config hierarchy: defaults > instanceof > service config

| Q             | A
| ------------- | ---
| Branch?       | 3.3
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Replaces #22294.
The main change is that ChildDefinition does not have "instanceof" applied anymore. All the complexity of the pass came from the merging logic required to deal with them. But in fact, we'd better not have any such logic and just not apply "instanceof" to ChildDefinition (but have them inherit from their parents with the usual logic).

Commits
-------

6d6116b Adding an integration test for the hirarchy of defaults, instanceof, child, parent definitions
ab86457 [DI] Rework config hierarchy: defaults > instanceof > service config
cbaee55 [DI] Track changes at the "Definition" level
  • Loading branch information
@fabpot
fabpot committed Apr 11, 2017
2 parents 44008e8 + 6d6116b commit 9950b90
Show file tree
Hide file tree
Showing 22 changed files with 575 additions and 442 deletions.
125 changes: 8 additions & 117 deletions src/Symfony/Component/DependencyInjection/ChildDefinition.php
View file
Expand Up @@ -23,15 +23,12 @@ class ChildDefinition extends Definition
{
private $parent;
private $inheritTags = false;
private $changes = array();

/**
* @param string $parent The id of Definition instance to decorate
*/
public function __construct($parent)
{
parent::__construct();

$this->parent = $parent;
}

Expand All @@ -46,13 +43,17 @@ public function getParent()
}

/**
* Returns all changes tracked for the Definition object.
* Sets the Definition being decorated.
*
* @param string $parent
*
* @return array An array of changes for this Definition
* @return $this
*/
public function getChanges()
public function setParent($parent)
{
return $this->changes;
$this->parent = $parent;

return $this;
}

/**
Expand All @@ -79,116 +80,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.
*
Expand Down
9 changes: 5 additions & 4 deletions src/Symfony/Component/DependencyInjection/Compiler/AbstractRecursivePass.php
View file
Expand Up @@ -65,11 +65,12 @@ protected function processValue($value, $isRoot = false)
$value->setProperties($this->processValue($value->getProperties()));
$value->setMethodCalls($this->processValue($value->getMethodCalls()));

if ($v = $value->getFactory()) {
$value->setFactory($this->processValue($v));
$changes = $value->getChanges();
if (isset($changes['factory'])) {
$value->setFactory($this->processValue($value->getFactory()));
}
if ($v = $value->getConfigurator()) {
$value->setConfigurator($this->processValue($v));
if (isset($changes['configurator'])) {
$value->setConfigurator($this->processValue($value->getConfigurator()));
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php
View file
Expand Up @@ -42,7 +42,8 @@ public function __construct()
$this->beforeOptimizationPasses = array(
100 => array(
$resolveClassPass = new ResolveClassPass(),
new ResolveDefinitionInheritancePass(),
new ResolveInstanceofConditionalsPass(),
new ResolveTagsInheritancePass(),
),
);

Expand Down
99 changes: 0 additions & 99 deletions src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionInheritancePass.php
View file

This file was deleted.

37 changes: 16 additions & 21 deletions src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php
View file
Expand Up @@ -84,7 +84,7 @@ private function doResolveDefinition(ChildDefinition $definition)
$def = new Definition();

// merge in parent definition
// purposely ignored attributes: abstract, tags
// purposely ignored attributes: abstract, shared, tags
$def->setClass($parentDef->getClass());
$def->setArguments($parentDef->getArguments());
$def->setMethodCalls($parentDef->getMethodCalls());
Expand All @@ -101,27 +101,8 @@ private function doResolveDefinition(ChildDefinition $definition)
$def->setPublic($parentDef->isPublic());
$def->setLazy($parentDef->isLazy());
$def->setAutowired($parentDef->isAutowired());
$def->setChanges($parentDef->getChanges());

self::mergeDefinition($def, $definition);

// merge autowiring types
foreach ($definition->getAutowiringTypes(false) as $autowiringType) {
$def->addAutowiringType($autowiringType);
}

// 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'])) {
Expand All @@ -148,6 +129,9 @@ public static function mergeDefinition(Definition $def, ChildDefinition $definit
if (isset($changes['autowired'])) {
$def->setAutowired($definition->isAutowired());
}
if (isset($changes['shared'])) {
$def->setShared($definition->isShared());
}
if (isset($changes['decorated_service'])) {
$decoratedService = $definition->getDecoratedService();
if (null === $decoratedService) {
Expand Down Expand Up @@ -177,5 +161,16 @@ 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
$def->setAbstract($definition->isAbstract());
$def->setTags($definition->getTags());

return $def;
}
}

0 comments on commit 9950b90

Please sign in to comment.