Skip to content

[DI] Add ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE #24033

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

Merged
merged 1 commit into from
Aug 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ CHANGELOG
3.4.0
-----

* added support for ignore-on-uninitialized references
* deprecated service auto-registration while autowiring
* deprecated the ability to check for the initialization of a private service with the `Container::initialized()` method
* deprecated support for top-level anonymous services in XML
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Argument\ArgumentInterface;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Definition;
use Symfony\Component\DependencyInjection\ExpressionLanguage;
use Symfony\Component\DependencyInjection\Reference;
Expand Down Expand Up @@ -96,7 +97,8 @@ protected function processValue($value, $isRoot = false)
$this->getDefinitionId((string) $value),
$targetDefinition,
$value,
$this->lazy || ($targetDefinition && $targetDefinition->isLazy())
$this->lazy || ($targetDefinition && $targetDefinition->isLazy()),
ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior()
);

return $value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\Reference;
Expand All @@ -24,14 +25,16 @@ class CheckExceptionOnInvalidReferenceBehaviorPass extends AbstractRecursivePass
{
protected function processValue($value, $isRoot = false)
{
if ($value instanceof Reference && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior()) {
$destId = (string) $value;

if (!$this->container->has($destId)) {
throw new ServiceNotFoundException($destId, $this->currentId);
}
if (!$value instanceof Reference) {
return parent::processValue($value, $isRoot);
}
if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE === $value->getInvalidBehavior() && !$this->container->has($id = (string) $value)) {
throw new ServiceNotFoundException($id, $this->currentId);
}
if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $value->getInvalidBehavior() && $this->container->has($id = (string) $value) && !$this->container->findDefinition($id)->isShared()) {
throw new InvalidArgumentException(sprintf('Invalid ignore-on-uninitialized reference found in service "%s": target service "%s" is not shared.', $this->currentId, $id));
}

return parent::processValue($value, $isRoot);
return $value;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,9 @@ private function isInlineableDefinition($id, Definition $definition, ServiceRefe

$ids = array();
foreach ($graph->getNode($id)->getInEdges() as $edge) {
if ($edge->isWeak()) {
return false;
}
$ids[] = $edge->getSourceNode()->getId();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ protected function processValue($value, $isRoot = false)
if ($optionalBehavior = '?' === $type[0]) {
$type = substr($type, 1);
$optionalBehavior = ContainerInterface::IGNORE_ON_INVALID_REFERENCE;
} elseif ($optionalBehavior = '!' === $type[0]) {
$type = substr($type, 1);
$optionalBehavior = ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE;
}
if (is_int($key)) {
$key = $type;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,9 @@ public function process(ContainerBuilder $container)
$referencingAliases = array();
$sourceIds = array();
foreach ($edges as $edge) {
if ($edge->isWeak()) {
continue;
}
$node = $edge->getSourceNode();
$sourceIds[] = $node->getId();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
* it themselves which improves performance quite a lot.
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*
* @final since version 3.4
*/
class ServiceReferenceGraph
{
Expand Down Expand Up @@ -85,23 +87,16 @@ public function clear()
* @param string $destValue
* @param string $reference
* @param bool $lazy
* @param bool $weak
*/
public function connect($sourceId, $sourceValue, $destId, $destValue = null, $reference = null/*, bool $lazy = false*/)
public function connect($sourceId, $sourceValue, $destId, $destValue = null, $reference = null/*, bool $lazy = false, bool $weak = false*/)
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 tag this class as @final to allow us to avoid having to do such magic for future changes to the container ? I'm not sure extending the ServiceReferenceGraph actually makes sense.

Tagging it as @internal would be even more strict, but tools wanting to analyze the DIC to display debugging info may be a valid use case for using this class so I would not go that far (for instance, https://github.com/schmittjoh/JMSDebuggingBundle relies on it, even though I'm not sure it plays well with recent versions as I haven't used this bundle since years)

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, @final it is now

{
if (func_num_args() >= 6) {
$lazy = func_get_arg(5);
} else {
if (__CLASS__ !== get_class($this)) {
$r = new \ReflectionMethod($this, __FUNCTION__);
if (__CLASS__ !== $r->getDeclaringClass()->getName()) {
@trigger_error(sprintf('Method %s() will have a 6th `bool $lazy = false` argument in version 4.0. Not defining it is deprecated since 3.3.', __METHOD__), E_USER_DEPRECATED);
}
}
$lazy = false;
}
$lazy = func_num_args() >= 6 ? func_get_arg(5) : false;
$weak = func_num_args() >= 7 ? func_get_arg(6) : false;

$sourceNode = $this->createNode($sourceId, $sourceValue);
$destNode = $this->createNode($destId, $destValue);
$edge = new ServiceReferenceGraphEdge($sourceNode, $destNode, $reference, $lazy);
$edge = new ServiceReferenceGraphEdge($sourceNode, $destNode, $reference, $lazy, $weak);

$sourceNode->addOutEdge($edge);
$destNode->addInEdge($edge);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,19 +24,22 @@ class ServiceReferenceGraphEdge
private $destNode;
private $value;
private $lazy;
private $weak;

/**
* @param ServiceReferenceGraphNode $sourceNode
* @param ServiceReferenceGraphNode $destNode
* @param string $value
* @param bool $lazy
* @param bool $weak
*/
public function __construct(ServiceReferenceGraphNode $sourceNode, ServiceReferenceGraphNode $destNode, $value = null, $lazy = false)
public function __construct(ServiceReferenceGraphNode $sourceNode, ServiceReferenceGraphNode $destNode, $value = null, $lazy = false, $weak = false)
{
$this->sourceNode = $sourceNode;
$this->destNode = $destNode;
$this->value = $value;
$this->lazy = $lazy;
$this->weak = $weak;
}

/**
Expand Down Expand Up @@ -78,4 +81,14 @@ public function isLazy()
{
return $this->lazy;
}

/**
* Returns true if the edge is weak, meaning it shouldn't prevent removing the target service.
*
* @return bool
*/
public function isWeak()
{
return $this->weak;
}
}
23 changes: 6 additions & 17 deletions src/Symfony/Component/DependencyInjection/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -23,26 +23,15 @@
* Container is a dependency injection container.
*
* It gives access to object instances (services).
*
* Services and parameters are simple key/pair stores.
*
* Parameter and service keys are case insensitive.
*
* A service can also be defined by creating a method named
* getXXXService(), where XXX is the camelized version of the id:
*
* <ul>
* <li>request -> getRequestService()</li>
* <li>mysql_session_storage -> getMysqlSessionStorageService()</li>
* <li>symfony.mysql_session_storage -> getSymfony_MysqlSessionStorageService()</li>
* </ul>
*
* The container can have three possible behaviors when a service does not exist:
* The container can have four possible behaviors when a service
* does not exist (or is not initialized for the last case):
*
* * EXCEPTION_ON_INVALID_REFERENCE: Throws an exception (the default)
* * NULL_ON_INVALID_REFERENCE: Returns null
* * IGNORE_ON_INVALID_REFERENCE: Ignores the wrapping command asking for the reference
* (for instance, ignore a setter if the service does not exist)
* * IGNORE_ON_UNINITIALIZED_REFERENCE: Ignores/returns null for uninitialized services or invalid references
*
* @author Fabien Potencier <fabien@symfony.com>
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
Expand Down Expand Up @@ -304,9 +293,9 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE

try {
if (isset($this->fileMap[$id])) {
return $this->load($this->fileMap[$id]);
return self::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior ? null : $this->load($this->fileMap[$id]);
Copy link
Contributor

Choose a reason for hiding this comment

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

can't the value self::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior be stored in a variable ?

Copy link
Member Author

@nicolas-grekas nicolas-grekas Aug 31, 2017

Choose a reason for hiding this comment

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

it can, but it's better as is to me (wouldn't help readability, and the extra var is just overhead, so that's two reasons)

Copy link
Contributor

Choose a reason for hiding this comment

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

Np then ;)

} elseif (isset($this->methodMap[$id])) {
return $this->{$this->methodMap[$id]}();
return self::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior ? null : $this->{$this->methodMap[$id]}();
} elseif (--$i && $id !== $normalizedId = $this->normalizeId($id)) {
$id = $normalizedId;
continue;
Expand All @@ -315,7 +304,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
// and only when the dumper has not generated the method map (otherwise the method map is considered to be fully populated by the dumper)
@trigger_error('Generating a dumped container without populating the method map is deprecated since 3.2 and will be unsupported in 4.0. Update your dumper to generate the method map.', E_USER_DEPRECATED);

return $this->{$method}();
return self::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior ? null : $this->{$method}();
}

break;
Expand Down
48 changes: 45 additions & 3 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -565,6 +565,9 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
{
$id = $this->normalizeId($id);

if (ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $invalidBehavior) {
return parent::get($id, $invalidBehavior);
}
if ($service = parent::get($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)) {
return $service;
}
Expand Down Expand Up @@ -1160,6 +1163,11 @@ public function resolveServices($value)
continue 2;
}
}
foreach (self::getInitializedConditionals($v) as $s) {
if (!$this->get($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE)) {
continue 2;
}
}

yield $k => $this->resolveServices($v);
}
Expand All @@ -1171,6 +1179,11 @@ public function resolveServices($value)
continue 2;
}
}
foreach (self::getInitializedConditionals($v) as $s) {
if (!$this->get($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE)) {
continue 2;
}
}

++$count;
}
Expand Down Expand Up @@ -1397,6 +1410,8 @@ public function log(CompilerPassInterface $pass, $message)
* @param mixed $value An array of conditionals to return
*
* @return array An array of Service conditionals
*
* @internal since version 3.4
*/
public static function getServiceConditionals($value)
{
Expand All @@ -1413,6 +1428,30 @@ public static function getServiceConditionals($value)
return $services;
}

/**
* Returns the initialized conditionals.
*
* @param mixed $value An array of conditionals to return
*
* @return array An array of uninitialized conditionals
*
* @internal
*/
public static function getInitializedConditionals($value)
{
$services = array();

if (is_array($value)) {
foreach ($value as $v) {
$services = array_unique(array_merge($services, self::getInitializedConditionals($v)));
}
} elseif ($value instanceof Reference && $value->getInvalidBehavior() === ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE) {
$services[] = (string) $value;
}

return $services;
}

/**
* Computes a reasonably unique hash of a value.
*
Expand Down Expand Up @@ -1465,13 +1504,16 @@ private function getProxyInstantiator()

private function callMethod($service, $call)
{
$services = self::getServiceConditionals($call[1]);

foreach ($services as $s) {
foreach (self::getServiceConditionals($call[1]) as $s) {
if (!$this->has($s)) {
return;
}
}
foreach (self::getInitializedConditionals($call[1]) as $s) {
if (!$this->get($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE)) {
return;
}
}

call_user_func_array(array($service, $call[0]), $this->resolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1]))));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface ContainerInterface extends PsrContainerInterface
const EXCEPTION_ON_INVALID_REFERENCE = 1;
const NULL_ON_INVALID_REFERENCE = 2;
const IGNORE_ON_INVALID_REFERENCE = 3;
const IGNORE_ON_UNINITIALIZED_REFERENCE = 4;

/**
* Sets a service.
Expand Down
22 changes: 13 additions & 9 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra
if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE === $behavior[$id]) {
$code .= sprintf($template, $name, $this->getServiceCall($id));
} else {
$code .= sprintf($template, $name, $this->getServiceCall($id, new Reference($id, ContainerInterface::NULL_ON_INVALID_REFERENCE)));
$code .= sprintf($template, $name, $this->getServiceCall($id, new Reference($id, $behavior[$id])));
}
}
}
Expand Down Expand Up @@ -1295,12 +1295,14 @@ private function wrapServiceConditionals($value, $code)
*/
private function getServiceConditionals($value)
{
if (!$services = ContainerBuilder::getServiceConditionals($value)) {
return null;
}

$conditions = array();
foreach ($services as $service) {
foreach (ContainerBuilder::getInitializedConditionals($value) as $service) {
if (!$this->container->hasDefinition($service)) {
return 'false';
}
$conditions[] = sprintf("isset(\$this->services['%s'])", $service);
}
foreach (ContainerBuilder::getServiceConditionals($value) as $service) {
if ($this->container->hasDefinition($service) && !$this->container->getDefinition($service)->isPublic()) {
continue;
}
Expand Down Expand Up @@ -1335,8 +1337,8 @@ private function getServiceCallsFromArguments(array $arguments, array &$calls, a
}
if (!isset($behavior[$id])) {
$behavior[$id] = $argument->getInvalidBehavior();
} elseif (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $behavior[$id]) {
$behavior[$id] = $argument->getInvalidBehavior();
} else {
$behavior[$id] = min($behavior[$id], $argument->getInvalidBehavior());
}

++$calls[$id];
Expand Down Expand Up @@ -1665,7 +1667,9 @@ private function getServiceCall($id, Reference $reference = null)
return '$this';
}

if ($this->asFiles && $this->container->hasDefinition($id)) {
if (null !== $reference && ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE === $reference->getInvalidBehavior()) {
$code = 'null';
} elseif ($this->asFiles && $this->container->hasDefinition($id)) {
if ($this->container->getDefinition($id)->isShared()) {
$code = sprintf("\$this->load(__DIR__.'/%s.php')", $this->generateMethodName($id));
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -309,6 +309,8 @@ private function convertParameters(array $parameters, $type, \DOMElement $parent
$element->setAttribute('on-invalid', 'null');
} elseif ($behaviour == ContainerInterface::IGNORE_ON_INVALID_REFERENCE) {
$element->setAttribute('on-invalid', 'ignore');
} elseif ($behaviour == ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE) {
$element->setAttribute('on-invalid', 'ignore_uninitialized');
}
} elseif ($value instanceof Definition) {
$element->setAttribute('type', 'service');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -304,8 +304,12 @@ private function dumpValue($value)
*/
private function getServiceCall($id, Reference $reference = null)
{
if (null !== $reference && ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE !== $reference->getInvalidBehavior()) {
return sprintf('@?%s', $id);
if (null !== $reference) {
Copy link
Contributor

Choose a reason for hiding this comment

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

can't we do ?

if (null === $reference) {
    return sprintf('@%s', $id);
}

switch...

Copy link
Member Author

Choose a reason for hiding this comment

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

that would force duplicating the return sprintf('@%s', $id); line.

switch ($reference->getInvalidBehavior()) {
case ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE: break;
case ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE: return sprintf('@!%s', $id);
default: return sprintf('@?%s', $id);
}
}

return sprintf('@%s', $id);
Expand Down
Loading