Skip to content

[DI] Add logging and better failure recovery to AutowirePass #22095

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 1 commit into from
Mar 21, 2017
Merged

[DI] Add logging and better failure recovery to AutowirePass #22095

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 48 additions & 14 deletions src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
use Symfony\Component\DependencyInjection\TypedReference;

/**
* Guesses constructor arguments of services definitions and try to instantiate services if necessary.
* Inspects existing service definitions and wires the autowired ones using the type hints of their classes.
*
* @author Kévin Dunglas <dunglas@gmail.com>
* @author Nicolas Grekas <p@tchwork.com>
*/
class AutowirePass extends AbstractRecursivePass
{
Expand Down Expand Up @@ -95,6 +96,8 @@ protected function processValue($value, $isRoot = false)
if ($value instanceof TypedReference && $this->currentDefinition->isAutowired() && !$this->container->has((string) $value)) {
if ($ref = $this->getAutowiredReference($value->getType(), $value->canBeAutoregistered())) {
$value = new TypedReference((string) $ref, $value->getType(), $value->getInvalidBehavior(), $value->canBeAutoregistered());
} else {
$this->container->log($this, $this->createTypeNotFoundMessage($value->getType(), 'typed reference'));
}
}
if (!$value instanceof Definition) {
Expand Down Expand Up @@ -275,18 +278,17 @@ private function autowireMethod(\ReflectionMethod $reflectionMethod, array $argu

if ($value = $this->getAutowiredReference($type)) {
$this->usedTypes[$type] = $this->currentId;
} elseif ($parameter->isDefaultValueAvailable()) {
$value = $parameter->getDefaultValue();
} elseif ($parameter->allowsNull()) {
$value = null;
} else {
if ($classOrInterface = class_exists($type, false) ? 'class' : (interface_exists($type, false) ? 'interface' : null)) {
$message = sprintf('Unable to autowire argument of type "%s" for the service "%s". No services were found matching this %s and it cannot be auto-registered.', $type, $this->currentId, $classOrInterface);
$failureMessage = $this->createTypeNotFoundMessage($type, 'argument $'.$parameter->name.' of method '.$reflectionMethod->class.'::'.$reflectionMethod->name.'()');

if ($parameter->isDefaultValueAvailable()) {
$value = $parameter->getDefaultValue();
} elseif ($parameter->allowsNull()) {
$value = null;
} else {
$message = sprintf('Cannot autowire argument $%s of method %s::%s() for service "%s": Class %s does not exist.', $parameter->name, $reflectionMethod->class, $reflectionMethod->name, $this->currentId, $type);
throw new RuntimeException($failureMessage);
}

throw new RuntimeException($message);
$this->container->log($this, $failureMessage);
}

$arguments[$index] = $value;
Expand Down Expand Up @@ -320,6 +322,7 @@ private function autowireOverridenGetters(array $overridenGetters, array $autowi
}

if (!$typeRef = $this->getAutowiredReference($type)) {
$this->container->log($this, $this->createTypeNotFoundMessage($type, 'return value of method '.$reflectionMethod->class.'::'.$reflectionMethod->name.'()'));
continue;
}

Expand All @@ -344,6 +347,8 @@ private function getAutowiredReference($type, $autoRegister = true)
}

if (isset($this->types[$type])) {
$this->container->log($this, sprintf('Service "%s" matches type "%s" and has been autowired into service "%s".', $this->types[$type], $type, $this->currentId));

return new Reference($this->types[$type]);
}

Expand Down Expand Up @@ -449,24 +454,53 @@ private function createAutowiredDefinition(\ReflectionClass $typeHint)
}

if (!$typeHint->isInstantiable()) {
$this->container->log($this, sprintf('Type "%s" is not instantiable thus cannot be auto-registered for service "%s".', $typeHint->name, $this->currentId));

return;
}

$ambiguousServiceTypes = $this->ambiguousServiceTypes;
$currentDefinition = $this->currentDefinition;
$definitions = $this->container->getDefinitions();
$currentId = $this->currentId;
$this->currentId = $argumentId = sprintf('autowired.%s', $typeHint->name);

$argumentDefinition = $this->container->register($argumentId, $typeHint->name);
$this->currentDefinition = $argumentDefinition = new Definition($typeHint->name);
$argumentDefinition->setPublic(false);
$argumentDefinition->setAutowired(true);

$this->populateAvailableType($argumentId, $argumentDefinition);

$this->processValue($argumentDefinition, true);
$this->currentId = $currentId;
try {
$this->processValue($argumentDefinition, true);
$this->container->setDefinition($argumentId, $argumentDefinition);
} catch (RuntimeException $e) {
// revert any changes done to our internal state
Copy link
Member

Choose a reason for hiding this comment

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

can we have tests covering this ?

Copy link
Member Author

Choose a reason for hiding this comment

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

test case added

unset($this->types[$typeHint->name]);
$this->ambiguousServiceTypes = $ambiguousServiceTypes;
$this->container->setDefinitions($definitions);
$this->container->log($this, $e->getMessage());

return;
} finally {
$this->currentId = $currentId;
$this->currentDefinition = $currentDefinition;
}

$this->container->log($this, sprintf('Type "%s" has been auto-registered for service "%s".', $typeHint->name, $this->currentId));

return new Reference($argumentId);
}

private function createTypeNotFoundMessage($type, $label)
{
if (!$classOrInterface = class_exists($type, false) ? 'class' : (interface_exists($type, false) ? 'interface' : null)) {
return sprintf('Cannot autowire %s for service "%s": Class or interface "%s" does not exist.', $label, $this->currentId, $type);
}
$message = sprintf('No services were found matching the "%s" %s and it cannot be auto-registered', $type, $classOrInterface);

return sprintf('Cannot autowire %s for service "%s": %s.', $label, $this->currentId, $message);
}

/**
* @deprecated since version 3.3, to be removed in 4.0.
*/
Expand Down
35 changes: 31 additions & 4 deletions src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowirePassTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ public function testTypeNotGuessableWithSubclass()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Unable to autowire argument of type "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" for the service "a". No services were found matching this interface and it cannot be auto-registered.
* @expectedExceptionMessage Cannot autowire argument $collision of method Symfony\Component\DependencyInjection\Tests\Compiler\CannotBeAutowired::__construct() for service "a": No services were found matching the "Symfony\Component\DependencyInjection\Tests\Compiler\CollisionInterface" interface and it cannot be auto-registered.
*/
public function testTypeNotGuessableNoServicesFound()
{
Expand Down Expand Up @@ -295,7 +295,7 @@ public function testDontTriggerAutowiring()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument::__construct() for service "a": Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist.
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadTypeHintedArgument::__construct() for service "a": Class or interface "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" does not exist.
*/
public function testClassNotFoundThrowsException()
{
Expand All @@ -310,7 +310,7 @@ public function testClassNotFoundThrowsException()

/**
* @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct() for service "a": Class Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass does not exist.
* @expectedExceptionMessage Cannot autowire argument $r of method Symfony\Component\DependencyInjection\Tests\Compiler\BadParentTypeHintedArgument::__construct() for service "a": Class or interface "Symfony\Component\DependencyInjection\Tests\Compiler\OptionalServiceClass" does not exist.
*/
public function testParentClassNotFoundThrowsException()
{
Expand Down Expand Up @@ -716,14 +716,27 @@ public function testNotWireableCalls($method, $expectedMsg)
public function provideNotWireableCalls()
{
return array(
array('setNotAutowireable', 'Cannot autowire argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() for service "foo": Class Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass does not exist.'),
array('setNotAutowireable', 'Cannot autowire argument $n of method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setNotAutowireable() for service "foo": Class or interface "Symfony\Component\DependencyInjection\Tests\Compiler\NotARealClass" does not exist.'),
array('setBar', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setBar() has only optional arguments, thus must be wired explicitly.'),
array('setOptionalNotAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNotAutowireable() has only optional arguments, thus must be wired explicitly.'),
array('setOptionalNoTypeHint', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalNoTypeHint() has only optional arguments, thus must be wired explicitly.'),
array('setOptionalArgNoAutowireable', 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setOptionalArgNoAutowireable() has only optional arguments, thus must be wired explicitly.'),
array(null, 'Cannot autowire service "foo": method Symfony\Component\DependencyInjection\Tests\Compiler\NotWireable::setProtectedMethod() must be public.'),
);
}

public function testAutoregisterRestoresStateOnFailure()
{
$container = new ContainerBuilder();

$container->register('e', E::class)
->setAutowired(true);

$pass = new AutowirePass();
$pass->process($container);

$this->assertSame(array('service_container', 'e'), array_keys($container->getDefinitions()));
}
}

class Foo
Expand Down Expand Up @@ -786,6 +799,20 @@ public function __construct(B $b, DInterface $d)
}
}

class D
{
public function __construct(A $a, DInterface $d)
{
}
}

class E
{
public function __construct(D $d = null)
{
}
}

interface CollisionInterface
{
}
Expand Down