Skip to content
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

[DI] Add support for "wither" methods - for greater immutable services #30212

Merged
merged 1 commit into from Apr 3, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Expand Up @@ -348,6 +348,9 @@ private function getContainerDefinitionDocument(Definition $definition, string $
foreach ($calls as $callData) {
$callsXML->appendChild($callXML = $dom->createElement('call'));
$callXML->setAttribute('method', $callData[0]);
if ($callData[2] ?? false) {
$callXML->setAttribute('returns-clone', 'true');
}
}
}

Expand Down
Expand Up @@ -140,11 +140,41 @@ protected function processValue($value, $isRoot = false)
$this->byConstructor = true;
$this->processValue($value->getFactory());
$this->processValue($value->getArguments());

$properties = $value->getProperties();
$setters = $value->getMethodCalls();

// Any references before a "wither" are part of the constructor-instantiation graph
$lastWitherIndex = null;
foreach ($setters as $k => $call) {
if ($call[2] ?? false) {
$lastWitherIndex = $k;
}
}

if (null !== $lastWitherIndex) {
$this->processValue($properties);
$setters = $properties = [];

foreach ($value->getMethodCalls() as $k => $call) {
if (null === $lastWitherIndex) {
$setters[] = $call;
continue;
}

if ($lastWitherIndex === $k) {
$lastWitherIndex = null;
}

$this->processValue($call);
}
}

$this->byConstructor = $byConstructor;

if (!$this->onlyConstructorArguments) {
$this->processValue($value->getProperties());
$this->processValue($value->getMethodCalls());
$this->processValue($properties);
$this->processValue($setters);
$this->processValue($value->getConfigurator());
}
$this->lazy = $lazy;
Expand Down
Expand Up @@ -35,6 +35,7 @@ protected function processValue($value, $isRoot = false)
}

$alreadyCalledMethods = [];
$withers = [];

foreach ($value->getMethodCalls() as list($method)) {
$alreadyCalledMethods[strtolower($method)] = true;
Expand All @@ -50,7 +51,11 @@ protected function processValue($value, $isRoot = false)
while (true) {
if (false !== $doc = $r->getDocComment()) {
if (false !== stripos($doc, '@required') && preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@required(?:\s|\*/$)#i', $doc)) {
$value->addMethodCall($reflectionMethod->name);
if (preg_match('#(?:^/\*\*|\n\s*+\*)\s*+@return\s++static[\s\*]#i', $doc)) {
$withers[] = [$reflectionMethod->name, [], true];
} else {
$value->addMethodCall($reflectionMethod->name, []);
}
break;
}
if (false === stripos($doc, '@inheritdoc') || !preg_match('#(?:^/\*\*|\n\s*+\*)\s*+(?:\{@inheritdoc\}|@inheritdoc)(?:\s|\*/$)#i', $doc)) {
Expand All @@ -65,6 +70,15 @@ protected function processValue($value, $isRoot = false)
}
}

if ($withers) {
// Prepend withers to prevent creating circular loops
$setters = $value->getMethodCalls();
$value->setMethodCalls($withers);
foreach ($setters as $call) {
$value->addMethodCall($call[0], $call[1], $call[2] ?? false);
}
}

return $value;
}
}
28 changes: 21 additions & 7 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -1139,8 +1139,15 @@ private function createService(Definition $definition, array &$inlineServices, $
}
}

if ($tryProxy || !$definition->isLazy()) {
// share only if proxying failed, or if not a proxy
$lastWitherIndex = null;
foreach ($definition->getMethodCalls() as $k => $call) {
if ($call[2] ?? false) {
$lastWitherIndex = $k;
}
}

if (null === $lastWitherIndex && ($tryProxy || !$definition->isLazy())) {
// share only if proxying failed, or if not a proxy, and if no withers are found
$this->shareService($definition, $service, $id, $inlineServices);
}

Expand All @@ -1149,8 +1156,13 @@ private function createService(Definition $definition, array &$inlineServices, $
$service->$name = $value;
}

foreach ($definition->getMethodCalls() as $call) {
$this->callMethod($service, $call, $inlineServices);
foreach ($definition->getMethodCalls() as $k => $call) {
$service = $this->callMethod($service, $call, $inlineServices);

if ($lastWitherIndex === $k && ($tryProxy || !$definition->isLazy())) {
// share only if proxying failed, or if not a proxy, and this is the last wither
$this->shareService($definition, $service, $id, $inlineServices);
}
}

if ($callable = $definition->getConfigurator()) {
Expand Down Expand Up @@ -1568,16 +1580,18 @@ private function callMethod($service, $call, array &$inlineServices)
{
foreach (self::getServiceConditionals($call[1]) as $s) {
if (!$this->has($s)) {
return;
return $service;
}
}
foreach (self::getInitializedConditionals($call[1]) as $s) {
if (!$this->doGet($s, ContainerInterface::IGNORE_ON_UNINITIALIZED_REFERENCE, $inlineServices)) {
return;
return $service;
}
}

$service->{$call[0]}(...$this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));
$result = $service->{$call[0]}(...$this->doResolveServices($this->getParameterBag()->unescapeValue($this->getParameterBag()->resolveValue($call[1])), $inlineServices));

return empty($call[2]) ? $service : $result;
}

/**
Expand Down
11 changes: 6 additions & 5 deletions src/Symfony/Component/DependencyInjection/Definition.php
Expand Up @@ -330,7 +330,7 @@ public function setMethodCalls(array $calls = [])
{
$this->calls = [];
foreach ($calls as $call) {
$this->addMethodCall($call[0], $call[1]);
$this->addMethodCall($call[0], $call[1], $call[2] ?? false);
}

return $this;
Expand All @@ -339,19 +339,20 @@ public function setMethodCalls(array $calls = [])
/**
* Adds a method to call after service initialization.
*
* @param string $method The method name to call
* @param array $arguments An array of arguments to pass to the method call
* @param string $method The method name to call
* @param array $arguments An array of arguments to pass to the method call
* @param bool $returnsClone Whether the call returns the service instance or not
*
* @return $this
*
* @throws InvalidArgumentException on empty $method param
*/
public function addMethodCall($method, array $arguments = [])
public function addMethodCall($method, array $arguments = []/*, bool $returnsClone = false*/)
{
if (empty($method)) {
throw new InvalidArgumentException('Method name cannot be empty.');
}
$this->calls[] = [$method, $arguments];
$this->calls[] = 2 < \func_num_args() && \func_get_arg(2) ? [$method, $arguments, true] : [$method, $arguments];
fabpot marked this conversation as resolved.
Show resolved Hide resolved

return $this;
}
Expand Down
33 changes: 28 additions & 5 deletions src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
Expand Up @@ -506,7 +506,14 @@ private function addServiceInstance(string $id, Definition $definition, bool $is
$isProxyCandidate = $this->getProxyDumper()->isProxyCandidate($definition);
$instantiation = '';

if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id])) {
$lastWitherIndex = null;
foreach ($definition->getMethodCalls() as $k => $call) {
if ($call[2] ?? false) {
$lastWitherIndex = $k;
}
}

if (!$isProxyCandidate && $definition->isShared() && !isset($this->singleUsePrivateIds[$id]) && null === $lastWitherIndex) {
$instantiation = sprintf('$this->%s[\'%s\'] = %s', $this->container->getDefinition($id)->isPublic() ? 'services' : 'privates', $id, $isSimpleInstance ? '' : '$instance');
} elseif (!$isSimpleInstance) {
$instantiation = '$instance';
Expand Down Expand Up @@ -563,16 +570,32 @@ private function isTrivialInstance(Definition $definition): bool
return true;
}

private function addServiceMethodCalls(Definition $definition, string $variableName = 'instance'): string
private function addServiceMethodCalls(Definition $definition, string $variableName, ?string $sharedNonLazyId): string
{
$lastWitherIndex = null;
foreach ($definition->getMethodCalls() as $k => $call) {
if ($call[2] ?? false) {
$lastWitherIndex = $k;
}
}

$calls = '';
foreach ($definition->getMethodCalls() as $call) {
foreach ($definition->getMethodCalls() as $k => $call) {
$arguments = [];
foreach ($call[1] as $value) {
$arguments[] = $this->dumpValue($value);
}

$calls .= $this->wrapServiceConditionals($call[1], sprintf(" \$%s->%s(%s);\n", $variableName, $call[0], implode(', ', $arguments)));
$witherAssignation = '';

if ($call[2] ?? false) {
if (null !== $sharedNonLazyId && $lastWitherIndex === $k) {
$witherAssignation = sprintf('$this->%s[\'%s\'] = ', $definition->isPublic() ? 'services' : 'privates', $sharedNonLazyId);
}
$witherAssignation .= sprintf('$%s = ', $variableName);
}

$calls .= $this->wrapServiceConditionals($call[1], sprintf(" %s\$%s->%s(%s);\n", $witherAssignation, $variableName, $call[0], implode(', ', $arguments)));
}

return $calls;
Expand Down Expand Up @@ -814,7 +837,7 @@ private function addInlineService(string $id, Definition $definition, Definition
}

$code .= $this->addServiceProperties($inlineDef, $name);
$code .= $this->addServiceMethodCalls($inlineDef, $name);
$code .= $this->addServiceMethodCalls($inlineDef, $name, !$this->getProxyDumper()->isProxyCandidate($inlineDef) && $inlineDef->isShared() && !isset($this->singleUsePrivateIds[$id]) ? $id : null);
$code .= $this->addServiceConfigurator($inlineDef, $name);
}

Expand Down
Expand Up @@ -84,6 +84,9 @@ private function addMethodCalls(array $methodcalls, \DOMElement $parent)
if (\count($methodcall[1])) {
$this->convertParameters($methodcall[1], 'argument', $call);
}
if ($methodcall[2] ?? false) {
$call->setAttribute('returns-clone', 'true');
}
$parent->appendChild($call);
}
}
Expand Down
Expand Up @@ -337,7 +337,7 @@ private function parseDefinition(\DOMElement $service, $file, array $defaults)
}

foreach ($this->getChildren($service, 'call') as $call) {
$definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file));
$definition->addMethodCall($call->getAttribute('method'), $this->getArgumentsAsPhp($call, 'argument', $file), XmlUtils::phpize($call->getAttribute('returns-clone')));
}

$tags = $this->getChildren($service, 'tag');
Expand Down
Expand Up @@ -463,15 +463,17 @@ private function parseDefinition($id, $service, $file, array $defaults)
if (isset($call['method'])) {
$method = $call['method'];
$args = isset($call['arguments']) ? $this->resolveServices($call['arguments'], $file) : [];
$returnsClone = $call['returns_clone'] ?? false;
} else {
$method = $call[0];
$args = isset($call[1]) ? $this->resolveServices($call[1], $file) : [];
$returnsClone = $call[2] ?? false;
}

if (!\is_array($args)) {
throw new InvalidArgumentException(sprintf('The second parameter for function call "%s" must be an array of its arguments for service "%s" in %s. Check your YAML syntax.', $method, $id, $file));
}
$definition->addMethodCall($method, $args);
$definition->addMethodCall($method, $args, $returnsClone);
}
}

Expand Down
Expand Up @@ -243,6 +243,7 @@
<xsd:element name="argument" type="argument" maxOccurs="unbounded" />
</xsd:choice>
<xsd:attribute name="method" type="xsd:string" />
<xsd:attribute name="returns-clone" type="boolean" />
</xsd:complexType>

<xsd:simpleType name="parameter_type">
Expand Down
Expand Up @@ -77,4 +77,26 @@ public function testExplicitMethodInjection()
);
$this->assertEquals([], $methodCalls[0][1]);
}

public function testWitherInjection()
{
$container = new ContainerBuilder();
$container->register(Foo::class);

$container
->register('wither', Wither::class)
->setAutowired(true);

(new ResolveClassPass())->process($container);
(new AutowireRequiredMethodsPass())->process($container);

$methodCalls = $container->getDefinition('wither')->getMethodCalls();

$expected = [
['withFoo1', [], true],
['withFoo2', [], true],
['setFoo', []],
];
$this->assertSame($expected, $methodCalls);
}
}
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\DependencyInjection\Tests;

require_once __DIR__.'/Fixtures/includes/autowiring_classes.php';
require_once __DIR__.'/Fixtures/includes/classes.php';
require_once __DIR__.'/Fixtures/includes/ProjectExtension.php';

Expand All @@ -36,6 +37,8 @@
use Symfony\Component\DependencyInjection\ParameterBag\ParameterBag;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\ServiceLocator;
use Symfony\Component\DependencyInjection\Tests\Compiler\Foo;
use Symfony\Component\DependencyInjection\Tests\Compiler\Wither;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CaseSensitiveClass;
use Symfony\Component\DependencyInjection\Tests\Fixtures\CustomDefinition;
use Symfony\Component\DependencyInjection\Tests\Fixtures\SimilarArgumentsDummy;
Expand Down Expand Up @@ -1565,6 +1568,22 @@ public function testDecoratedSelfReferenceInvolvingPrivateServices()

$this->assertSame(['service_container'], array_keys($container->getDefinitions()));
}

public function testWither()
{
$container = new ContainerBuilder();
$container->register(Foo::class);

$container
->register('wither', Wither::class)
->setPublic(true)
->setAutowired(true);

$container->compile();

$wither = $container->get('wither');
$this->assertInstanceOf(Foo::class, $wither->foo);
}
}

class FooClass
Expand Down
Expand Up @@ -95,10 +95,16 @@ public function testMethodCalls()
$this->assertEquals([['foo', ['foo']]], $def->getMethodCalls(), '->getMethodCalls() returns the methods to call');
$this->assertSame($def, $def->addMethodCall('bar', ['bar']), '->addMethodCall() implements a fluent interface');
$this->assertEquals([['foo', ['foo']], ['bar', ['bar']]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
$this->assertSame($def, $def->addMethodCall('foobar', ['foobar'], true), '->addMethodCall() implements a fluent interface with third parameter');
$this->assertEquals([['foo', ['foo']], ['bar', ['bar']], ['foobar', ['foobar'], true]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
$this->assertTrue($def->hasMethodCall('bar'), '->hasMethodCall() returns true if first argument is a method to call registered');
$this->assertFalse($def->hasMethodCall('no_registered'), '->hasMethodCall() returns false if first argument is not a method to call registered');
$this->assertSame($def, $def->removeMethodCall('bar'), '->removeMethodCall() implements a fluent interface');
$this->assertTrue($def->hasMethodCall('foobar'), '->hasMethodCall() returns true if first argument is a method to call registered');
$this->assertSame($def, $def->removeMethodCall('foobar'), '->removeMethodCall() implements a fluent interface');
$this->assertEquals([['foo', ['foo']]], $def->getMethodCalls(), '->removeMethodCall() removes a method to call');
$this->assertSame($def, $def->setMethodCalls([['foobar', ['foobar'], true]]), '->setMethodCalls() implements a fluent interface with third parameter');
$this->assertEquals([['foobar', ['foobar'], true]], $def->getMethodCalls(), '->addMethodCall() adds a method to call');
}

/**
Expand Down