Skip to content

[DI] Add "inherit-tags" with configurable defaults + same for "public", "tags" & "autowire" #21071

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Jan 7, 2017
Merged

[DI] Add "inherit-tags" with configurable defaults + same for "public", "tags" & "autowire" #21071

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7b4a18b
[DI] Add "_defaults" key to Yaml services configuration
nicolas-grekas Dec 27, 2016
05f24d5
[DI] Add "defaults" tag to XML services configuration
ogizanagi Dec 28, 2016
beec1cf
[DI] Allow definitions to inherit tags from parent context
nicolas-grekas Dec 28, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,13 @@ CHANGELOG
3.3.0
-----

* Add "iterator" argument type for lazy iteration over a set of values and services

* Using the `PhpDumper` with an uncompiled `ContainerBuilder` is deprecated and
will not be supported anymore in 4.0.

* added "iterator" argument type for lazy iteration over a set of values and services
* added "closure-proxy" argument type for turning services' methods into lazy callables
* added file-wide configurable defaults for service attributes "public", "tags",
"autowire" and a new "inherit-tags"
* made the "class" attribute optional, using the "id" as fallback
* using the `PhpDumper` with an uncompiled `ContainerBuilder` is deprecated and
will not be supported anymore in 4.0
* deprecated the `DefinitionDecorator` class in favor of `ChildDefinition`

3.2.0
Expand Down
25 changes: 25 additions & 0 deletions src/Symfony/Component/DependencyInjection/ChildDefinition.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
class ChildDefinition extends Definition
{
private $parent;
private $inheritTags = false;
private $changes = array();

/**
Expand Down Expand Up @@ -54,6 +55,30 @@ public function getChanges()
return $this->changes;
}

/**
* Sets whether tags should be inherited from the parent or not.
*
* @param bool $boolean
*
* @return $this
*/
public function setInheritTags($boolean)
{
$this->inheritTags = (bool) $boolean;

return $this;
}

/**
* Returns whether tags should be inherited from the parent or not.
*
* @return bool
*/
public function getInheritTags()
{
return $this->inheritTags;
}

/**
* {@inheritdoc}
*/
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/DependencyInjection/Compiler/ResolveDefinitionTemplatesPass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,15 @@ private function doResolveDefinition(ContainerBuilder $container, ChildDefinitio
$def->setShared($definition->isShared());
$def->setTags($definition->getTags());

// 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;
}
}
84 changes: 80 additions & 4 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,28 +118,77 @@ private function parseDefinitions(\DOMDocument $xml, $file)
}

foreach ($services as $service) {
if (null !== $definition = $this->parseDefinition($service, $file)) {
if (null !== $definition = $this->parseDefinition($service, $file, $this->getServiceDefaults($xml, $file))) {
$this->container->setDefinition((string) $service->getAttribute('id'), $definition);
}
}
}

/**
* Get service defaults.
*
* @return array
*/
private function getServiceDefaults(\DOMDocument $xml, $file)
{
$xpath = new \DOMXPath($xml);
$xpath->registerNamespace('container', self::NS);

if (null === $defaultsNode = $xpath->query('//container:services/container:defaults')->item(0)) {
return array();
}
$defaults = array(
'tags' => $this->getChildren($defaultsNode, 'tag'),
'autowire' => $this->getChildren($defaultsNode, 'autowire'),
);

foreach ($defaults['tags'] as $tag) {
if ('' === $tag->getAttribute('name')) {
throw new InvalidArgumentException(sprintf('The tag name for tag "<defaults>" in %s must be a non-empty string.', $file));
}
}
if ($defaultsNode->hasAttribute('public')) {
$defaults['public'] = XmlUtils::phpize($defaultsNode->getAttribute('public'));
}
if ($defaultsNode->hasAttribute('inherit-tags')) {
$defaults['inherit-tags'] = XmlUtils::phpize($defaultsNode->getAttribute('inherit-tags'));
}
if (!$defaultsNode->hasAttribute('autowire')) {
foreach ($defaults['autowire'] as $k => $v) {
$defaults['autowire'][$k] = $v->textContent;
}

return $defaults;
}
if ($defaults['autowire']) {
throw new InvalidArgumentException(sprintf('The "autowire" attribute cannot be used together with "<autowire>" tags for tag "<defaults>" in %s.', $file));
}
if (XmlUtils::phpize($defaultsNode->getAttribute('autowire'))) {
$defaults['autowire'][] = '__construct';
}

return $defaults;
}

/**
* Parses an individual Definition.
*
* @param \DOMElement $service
* @param string $file
* @param array $defaults
*
* @return Definition|null
*/
private function parseDefinition(\DOMElement $service, $file)
private function parseDefinition(\DOMElement $service, $file, array $defaults = array())
{
if ($alias = $service->getAttribute('alias')) {
$this->validateAlias($service, $file);

$public = true;
if ($publicAttr = $service->getAttribute('public')) {
$public = XmlUtils::phpize($publicAttr);
} elseif (isset($defaults['public'])) {
$public = $defaults['public'];
}
$this->container->setAlias((string) $service->getAttribute('id'), new Alias($alias, $public));

Expand All @@ -148,11 +197,24 @@ private function parseDefinition(\DOMElement $service, $file)

if ($parent = $service->getAttribute('parent')) {
$definition = new ChildDefinition($parent);

if ($value = $service->getAttribute('inherit-tags')) {
$definition->setInheritTags(XmlUtils::phpize($value));
} elseif (isset($defaults['inherit-tags'])) {
$definition->setInheritTags($defaults['inherit-tags']);
}
$defaults = array();
} else {
$definition = new Definition();
}

foreach (array('class', 'shared', 'public', 'synthetic', 'lazy', 'abstract') as $key) {
if ($publicAttr = $service->getAttribute('public')) {
$definition->setPublic(XmlUtils::phpize($publicAttr));
} elseif (isset($defaults['public'])) {
$definition->setPublic($defaults['public']);
}

foreach (array('class', 'shared', 'synthetic', 'lazy', 'abstract') as $key) {
if ($value = $service->getAttribute($key)) {
$method = 'set'.$key;
$definition->$method(XmlUtils::phpize($value));
Expand Down Expand Up @@ -216,7 +278,19 @@ private function parseDefinition(\DOMElement $service, $file)
$definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument'));
}

foreach ($this->getChildren($service, 'tag') as $tag) {
$tags = $this->getChildren($service, 'tag');

if (empty($defaults['tags'])) {
// no-op
} elseif (!$value = $service->getAttribute('inherit-tags')) {
if (!$tags) {
$tags = $defaults['tags'];
}
} elseif (XmlUtils::phpize($value)) {
$tags = array_merge($tags, $defaults['tags']);
}

foreach ($tags as $tag) {
$parameters = array();
foreach ($tag->attributes as $name => $node) {
if ('name' === $name) {
Expand Down Expand Up @@ -252,6 +326,8 @@ private function parseDefinition(\DOMElement $service, $file)
}

$definition->setAutowiredMethods($autowireTags);
} elseif (!$service->hasAttribute('autowire') && !empty($defaults['autowire'])) {
$definition->setAutowiredMethods($defaults['autowire']);
}

if ($value = $service->getAttribute('decorates')) {
Expand Down
100 changes: 84 additions & 16 deletions src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ class YamlFileLoader extends FileLoader
'configurator' => 'configurator',
'calls' => 'calls',
'tags' => 'tags',
'inherit_tags' => 'inherit_tags',
'decorates' => 'decorates',
'decoration_inner_name' => 'decoration_inner_name',
'decoration_priority' => 'decoration_priority',
Expand Down Expand Up @@ -148,9 +149,56 @@ private function parseDefinitions($content, $file)
if (!is_array($content['services'])) {
throw new InvalidArgumentException(sprintf('The "services" key should contain an array in %s. Check your YAML syntax.', $file));
}
if (isset($content['services']['_defaults'])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we check whether the array contains extra keys likely to be used by a real service, and treat it as a service instead in this case (but with a deprecation warning) ? I would say that alias, class and factory would cover it

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done with deprecation. Other comments addressed also.

if (!is_array($defaults = $content['services']['_defaults'])) {
throw new InvalidArgumentException(sprintf('Service defaults must be an array, "%s" given in "%s".', gettype($defaults), $file));
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would validate the default value for tags here (when defined) instead of letting the first service using it throw an exception. It would be more understandable.

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);
$defaults = array();
} else {
$defaultKeys = array('public', 'tags', 'inherit_tags', 'autowire');
unset($content['services']['_defaults']);

foreach ($defaults as $key => $default) {
if (!in_array($key, $defaultKeys)) {
throw new InvalidArgumentException(sprintf('The configuration key "%s" cannot be used to define a default value in "%s". Allowed keys are "%s".', $key, $file, implode('", "', $defaultKeys)));
}
}
if (isset($defaults['tags'])) {
if (!is_array($tags = $defaults['tags'])) {
throw new InvalidArgumentException(sprintf('Parameter "tags" in "_defaults" must be an array in %s. Check your YAML syntax.', $file));
}

foreach ($tags as $tag) {
if (!is_array($tag)) {
$tag = array('name' => $tag);
}

if (!isset($tag['name'])) {
throw new InvalidArgumentException(sprintf('A "tags" entry in "_defaults" is missing a "name" key in %s.', $file));
}
$name = $tag['name'];
unset($tag['name']);

if (!is_string($name) || '' === $name) {
throw new InvalidArgumentException(sprintf('The tag name in "_defaults" must be a non-empty string in %s.', $file));
}

foreach ($tag as $attribute => $value) {
if (!is_scalar($value) && null !== $value) {
throw new InvalidArgumentException(sprintf('Tag "%s", attribute "%s" in "_defaults" must be of a scalar-type in %s. Check your YAML syntax.', $name, $attribute, $file));
}
}
}
}
}
} else {
$defaults = array();
}

foreach ($content['services'] as $id => $service) {
$this->parseDefinition($id, $service, $file);
$this->parseDefinition($id, $service, $file, $defaults);
}
}

Expand All @@ -160,10 +208,11 @@ private function parseDefinitions($content, $file)
* @param string $id
* @param array $service
* @param string $file
* @param array $defaults
*
* @throws InvalidArgumentException When tags are invalid
*/
private function parseDefinition($id, $service, $file)
private function parseDefinition($id, $service, $file, array $defaults)
{
if (is_string($service) && 0 === strpos($service, '@')) {
$this->container->setAlias($id, substr($service, 1));
Expand All @@ -178,7 +227,7 @@ private function parseDefinition($id, $service, $file)
static::checkDefinition($id, $service, $file);

if (isset($service['alias'])) {
$public = !array_key_exists('public', $service) || (bool) $service['public'];
$public = array_key_exists('public', $service) ? (bool) $service['public'] : !empty($defaults['public']);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to define this variable twice? One doing array_key_exists other isset?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

these are not on the same path, and don't have the same semantic (don't forget about ChildDefinition on the other path.)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had services defined: (Symfony 3.1.8):

foo:
    class: My\FooService

bar:
    alias: foo

I could access bar service in my controller because it was public by default. After this change here my aliased service is private by default, thus the service is not accessible in controller.

Shouldn't this be mentioned somewhere? I couldn't find any info about it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. That's a bug. See #21219

$this->container->setAlias($id, new Alias($service['alias'], $public));

foreach ($service as $key => $value) {
Expand All @@ -192,6 +241,12 @@ private function parseDefinition($id, $service, $file)

if (isset($service['parent'])) {
$definition = new ChildDefinition($service['parent']);

$inheritTag = isset($service['inherit_tags']) ? $service['inherit_tags'] : (isset($defaults['inherit_tags']) ? $defaults['inherit_tags'] : null);
if (null !== $inheritTag) {
$definition->setInheritTags($inheritTag);
}
$defaults = array();
} else {
$definition = new Definition();
}
Expand All @@ -212,8 +267,9 @@ private function parseDefinition($id, $service, $file)
$definition->setLazy($service['lazy']);
}

if (isset($service['public'])) {
$definition->setPublic($service['public']);
$public = isset($service['public']) ? $service['public'] : (isset($defaults['public']) ? $defaults['public'] : null);
if (null !== $public) {
$definition->setPublic($public);
}

if (isset($service['abstract'])) {
Expand Down Expand Up @@ -262,27 +318,38 @@ private function parseDefinition($id, $service, $file)
}
}

if (isset($service['tags'])) {
if (!is_array($service['tags'])) {
$tags = isset($service['tags']) ? $service['tags'] : array();

if (!isset($defaults['tags'])) {
// no-op
} elseif (!isset($service['inherit_tags'])) {
if (!isset($service['tags'])) {
$tags = $defaults['tags'];
}
} elseif ($service['inherit_tags']) {
$tags = array_merge($tags, $defaults['tags']);
}

if (null !== $tags) {
if (!is_array($tags)) {
throw new InvalidArgumentException(sprintf('Parameter "tags" must be an array for service "%s" in %s. Check your YAML syntax.', $id, $file));
}

foreach ($service['tags'] as $tag) {
foreach ($tags as $tag) {
if (!is_array($tag)) {
$tag = array('name' => $tag);
}

if (!isset($tag['name'])) {
throw new InvalidArgumentException(sprintf('A "tags" entry is missing a "name" key for service "%s" in %s.', $id, $file));
}
$name = $tag['name'];
unset($tag['name']);

if (!is_string($tag['name']) || '' === $tag['name']) {
if (!is_string($name) || '' === $name) {
throw new InvalidArgumentException(sprintf('The tag name for service "%s" in %s must be a non-empty string.', $id, $file));
}

$name = $tag['name'];
unset($tag['name']);

foreach ($tag as $attribute => $value) {
if (!is_scalar($value) && null !== $value) {
throw new InvalidArgumentException(sprintf('A "tags" attribute must be of a scalar-type for service "%s", tag "%s", attribute "%s" in %s. Check your YAML syntax.', $id, $name, $attribute, $file));
Expand All @@ -303,11 +370,12 @@ private function parseDefinition($id, $service, $file)
$definition->setDecoratedService($service['decorates'], $renameId, $priority);
}

if (isset($service['autowire'])) {
if (is_array($service['autowire'])) {
$definition->setAutowiredMethods($service['autowire']);
$autowire = isset($service['autowire']) ? $service['autowire'] : (isset($defaults['autowire']) ? $defaults['autowire'] : null);
if (null !== $autowire) {
if (is_array($autowire)) {
$definition->setAutowiredMethods($autowire);
} else {
$definition->setAutowired($service['autowire']);
$definition->setAutowired($autowire);
}
}

Expand Down
Loading