-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
[DependencyInjection] ActionBundle integration: introduce _instanceof #21357
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0a48b9e
9b9dbea
14e85c6
4e0b1e5
ec9c5b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,8 +126,9 @@ private function parseDefinitions(\DOMDocument $xml, $file) | |
} | ||
|
||
$defaults = $this->getServiceDefaults($xml, $file); | ||
$instanceof = $xpath->query('//container:services/container:instanceof/container:service') ?: null; | ||
foreach ($services as $service) { | ||
if (null !== $definition = $this->parseDefinition($service, $file, $defaults)) { | ||
if (null !== $definition = $this->parseDefinition($service, $file, $defaults, $instanceof)) { | ||
$this->container->setDefinition((string) $service->getAttribute('id'), $definition); | ||
} | ||
} | ||
|
@@ -182,14 +183,16 @@ private function getServiceDefaults(\DOMDocument $xml, $file) | |
/** | ||
* Parses an individual Definition. | ||
* | ||
* @param \DOMElement $service | ||
* @param string $file | ||
* @param array $defaults | ||
* @param \DOMElement $service | ||
* @param string $file | ||
* @param array $defaults | ||
* @param \DOMNodeList|null $instanceof | ||
* | ||
* @return Definition|null | ||
*/ | ||
private function parseDefinition(\DOMElement $service, $file, array $defaults = array()) | ||
private function parseDefinition(\DOMElement $service, $file, array $defaults = array(), \DOMNodeList $instanceof = null) | ||
{ | ||
$id = (string) $service->getAttribute('id'); | ||
if ($alias = $service->getAttribute('alias')) { | ||
$this->validateAlias($service, $file); | ||
|
||
|
@@ -199,11 +202,48 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults = | |
} elseif (isset($defaults['public'])) { | ||
$public = $defaults['public']; | ||
} | ||
$this->container->setAlias((string) $service->getAttribute('id'), new Alias($alias, $public)); | ||
$this->container->setAlias($id, new Alias($alias, $public)); | ||
|
||
return; | ||
} | ||
|
||
$definition = $this->getDefinition($service, $file, $defaults); | ||
if (null === $instanceof) { | ||
return $definition; | ||
} | ||
|
||
$className = $definition->getClass() ?: $id; | ||
$parentId = $definition instanceof ChildDefinition ? $definition->getParent() : null; | ||
foreach ($instanceof as $s) { | ||
$type = (string) $s->getAttribute('id'); | ||
if (!is_a($className, $type, true)) { | ||
continue; | ||
} | ||
|
||
if ($parentId) { | ||
$s = clone $s; | ||
$s->setAttribute('parent', $parentId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't it throw an exception if there is already a parent? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is an issue here. This is altering the DOM node, which means it will impact the next services too (which will break things if they are not ChildDefinition themselves) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed. |
||
} | ||
$parentId = $this->generateInstanceofDefinitionId($id, $type, $file); | ||
$parentDefinition = $this->getDefinition($s, $file); | ||
$parentDefinition->setAbstract(true); | ||
if ($parentDefinition instanceof ChildDefinition) { | ||
$definition->setInheritTags(true); | ||
} | ||
$this->container->setDefinition($parentId, $parentDefinition); | ||
} | ||
|
||
if (null !== $parentId) { | ||
$service->setAttribute('parent', $parentId); | ||
$definition = $this->getDefinition($service, $file, $defaults); | ||
$definition->setInheritTags(true); | ||
} | ||
|
||
return $definition; | ||
} | ||
|
||
private function getDefinition(\DOMElement $service, $file, array $defaults = array()) | ||
{ | ||
if ($parent = $service->getAttribute('parent')) { | ||
$definition = new ChildDefinition($parent); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -159,8 +159,16 @@ private function parseDefinitions(array $content, $file) | |
} | ||
|
||
$defaults = $this->parseDefaults($content, $file); | ||
|
||
if ($this->isUnderscoredParamValid($content, '_instanceof', $file)) { | ||
$instanceof = $content['services']['_instanceof']; | ||
unset($content['services']['_instanceof']); | ||
} else { | ||
$instanceof = array(); | ||
} | ||
|
||
foreach ($content['services'] as $id => $service) { | ||
$this->parseDefinition($id, $service, $file, $defaults); | ||
$this->parseDefinition($id, $service, $file, $defaults, $instanceof); | ||
} | ||
} | ||
|
||
|
@@ -174,20 +182,13 @@ private function parseDefinitions(array $content, $file) | |
*/ | ||
private function parseDefaults(array &$content, $file) | ||
{ | ||
if (!isset($content['services']['_defaults'])) { | ||
return $content; | ||
} | ||
if (!is_array($defaults = $content['services']['_defaults'])) { | ||
throw new InvalidArgumentException(sprintf('Service defaults must be an array, "%s" given in "%s".', gettype($defaults), $file)); | ||
if (!$this->isUnderscoredParamValid($content, '_defaults', $file)) { | ||
return array(); | ||
} | ||
if (isset($defaults['alias']) || isset($defaults['class']) || isset($defaults['factory'])) { | ||
@trigger_error('Giving a service the "_defaults" name is deprecated since Symfony 3.3 and will be forbidden in 4.0. Rename your service.', E_USER_DEPRECATED); | ||
|
||
return $content; | ||
} | ||
|
||
$defaultKeys = array('public', 'tags', 'inherit_tags', 'autowire'); | ||
$defaults = $content['services']['_defaults']; | ||
unset($content['services']['_defaults']); | ||
$defaultKeys = array('public', 'tags', 'inherit_tags', 'autowire'); | ||
|
||
foreach ($defaults as $key => $default) { | ||
if (!in_array($key, $defaultKeys)) { | ||
|
@@ -226,17 +227,37 @@ private function parseDefaults(array &$content, $file) | |
return $defaults; | ||
} | ||
|
||
private function isUnderscoredParamValid($content, $name, $file) | ||
{ | ||
if (!isset($content['services'][$name])) { | ||
return false; | ||
} | ||
|
||
if (!is_array($underscoreParam = $content['services'][$name])) { | ||
throw new InvalidArgumentException(sprintf('Service "%s" key must be an array, "%s" given in "%s".', $name, gettype($underscoreParam), $file)); | ||
} | ||
|
||
if (isset($underscoreParam['alias']) || isset($underscoreParam['class']) || isset($underscoreParam['factory'])) { | ||
@trigger_error(sprintf('Giving a service the "%s" name is deprecated since Symfony 3.3 and will be forbidden in 4.0. Rename your service.', $name), E_USER_DEPRECATED); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we deprecate using All shortcuts we are adding in 3.3 will make it almost impossible to implement such detection in 3.4+ ( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 to deprecate all services starting by There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm mixed on this deprecation at the general service id level. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Btw, we have an inconsistency here: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will be fixed if we deprecate all services starting with a |
||
|
||
return false; | ||
} | ||
|
||
return true; | ||
} | ||
|
||
/** | ||
* Parses a definition. | ||
* | ||
* @param string $id | ||
* @param array|string $service | ||
* @param string $file | ||
* @param array $defaults | ||
* @param array $instanceof | ||
* | ||
* @throws InvalidArgumentException When tags are invalid | ||
*/ | ||
private function parseDefinition($id, $service, $file, array $defaults) | ||
private function parseDefinition($id, $service, $file, array $defaults, array $instanceof) | ||
{ | ||
if (is_string($service) && 0 === strpos($service, '@')) { | ||
$public = isset($defaults['public']) ? $defaults['public'] : true; | ||
|
@@ -253,12 +274,6 @@ private function parseDefinition($id, $service, $file, array $defaults) | |
$service = array(); | ||
} | ||
|
||
if (!is_array($service)) { | ||
throw new InvalidArgumentException(sprintf('A service definition must be an array or a string starting with "@" but %s found for service "%s" in %s. Check your YAML syntax.', gettype($service), $id, $file)); | ||
} | ||
|
||
static::checkDefinition($id, $service, $file); | ||
|
||
if (isset($service['alias'])) { | ||
$public = array_key_exists('public', $service) ? (bool) $service['public'] : (isset($defaults['public']) ? $defaults['public'] : true); | ||
$this->container->setAlias($id, new Alias($service['alias'], $public)); | ||
|
@@ -272,6 +287,43 @@ private function parseDefinition($id, $service, $file, array $defaults) | |
return; | ||
} | ||
|
||
$definition = $this->getDefinition($id, $service, $file, $defaults); | ||
$className = $definition->getClass() ?: $id; | ||
$parentId = $definition instanceof ChildDefinition ? $definition->getParent() : null; | ||
foreach ($instanceof as $type => $value) { | ||
if (!is_a($className, $type, true)) { | ||
continue; | ||
} | ||
|
||
if ($parentId) { | ||
$value['parent'] = $parentId; | ||
} | ||
$parentId = $this->generateInstanceofDefinitionId($id, $type, $file); | ||
$parentDefinition = $this->getDefinition($parentId, $value, $file, array()); | ||
$parentDefinition->setAbstract(true); | ||
if ($parentDefinition instanceof ChildDefinition) { | ||
$definition->setInheritTags(true); | ||
} | ||
$this->container->setDefinition($parentId, $parentDefinition); | ||
} | ||
|
||
if (null !== $parentId) { | ||
$service['parent'] = $parentId; | ||
$definition = $this->getDefinition($id, $service, $file, $defaults); | ||
$definition->setInheritTags(true); | ||
} | ||
|
||
$this->container->setDefinition($id, $definition); | ||
} | ||
|
||
private function getDefinition($id, $service, $file, array $defaults) | ||
{ | ||
if (!is_array($service)) { | ||
throw new InvalidArgumentException(sprintf('A service definition must be an array or a string starting with "@" but %s found for service "%s" in %s. Check your YAML syntax.', gettype($service), $id, $file)); | ||
} | ||
|
||
static::checkDefinition($id, $service, $file); | ||
|
||
if (isset($service['parent'])) { | ||
$definition = new ChildDefinition($service['parent']); | ||
|
||
|
@@ -430,7 +482,7 @@ private function parseDefinition($id, $service, $file, array $defaults) | |
} | ||
} | ||
|
||
$this->container->setDefinition($id, $definition); | ||
return $definition; | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
<?xml version="1.0" encoding="utf-8"?> | ||
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services http://symfony.com/schema/dic/services/services-1.0.xsd"> | ||
<services> | ||
<instanceof> | ||
<service id="Symfony\Component\DependencyInjection\Tests\Loader\BarInterface" lazy="true"> | ||
<autowire>__construct</autowire> | ||
<tag name="foo" /> | ||
<tag name="bar" /> | ||
</service> | ||
</instanceof> | ||
|
||
<service id="Symfony\Component\DependencyInjection\Tests\Loader\Bar" class="Symfony\Component\DependencyInjection\Tests\Loader\Bar" /> | ||
</services> | ||
</container> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
services: | ||
_instanceof: | ||
Symfony\Component\DependencyInjection\Tests\Loader\FooInterface: | ||
autowire: true | ||
lazy: true | ||
tags: | ||
- { name: foo } | ||
- { name: bar } | ||
|
||
Symfony\Component\DependencyInjection\Tests\Loader\Foo: ~ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is broken when the class is a parameter (IIRC, this is still supported in 3.x, even though Symfony itself does not use the feature anymore)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC, it cannot be fixed in the loader directly because all parameters may have not been set at this time...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have 2 options here:
For the sake of simplicity, I'm for the solution 1. Solution 2 will create new edges cases (when using inline definitions for instance).
Or maybe someone have a better idea?