Skip to content

Commit

Permalink
bug #20566 [DI] Initialize properties before method calls (ro0NL)
Browse files Browse the repository at this point in the history
This PR was squashed before being merged into the 2.7 branch (closes #20566).

Discussion
----------

[DI] Initialize properties before method calls

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes-ish
| New feature?  | no
| BC breaks?    | not sure
| Deprecations? | no
| Tests pass?   | not yet (only dumps seem to fail)
| Fixed tickets | comma-separated list of tickets fixed by the PR, if any
| License       | MIT
| Doc PR        | reference to the documentation PR, if any

Given

```yml
services:
    handler:
        class: AppBundle\Handler
        properties:
            debug: '%kernel.debug%'
        calls:
            - [handle]
```

I totally expected `Handler::$debug` to be set before `Handler::handle` is called. It was not..  and it's really annoying :)

Commits
-------

0af433b [DI] Initialize properties before method calls
  • Loading branch information
nicolas-grekas committed Nov 25, 2016
2 parents 3eb8a34 + 0af433b commit e62a390
Show file tree
Hide file tree
Showing 7 changed files with 63 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -902,15 +902,15 @@ public function createService(Definition $definition, $id, $tryProxy = true)
$this->shareService($definition, $service, $id);
}

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

$properties = $this->resolveServices($parameterBag->unescapeValue($parameterBag->resolveValue($definition->getProperties())));
foreach ($properties as $name => $value) {
$service->$name = $value;
}

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

if ($callable = $definition->getConfigurator()) {
if (is_array($callable)) {
$callable[0] = $parameterBag->resolveValue($callable[0]);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,8 +335,8 @@ private function addServiceInlinedDefinitions($id, $definition)
$code .= $this->addNewInstance($id, $sDefinition, '$'.$name, ' = ');

if (!$this->hasReference($id, $sDefinition->getMethodCalls(), true) && !$this->hasReference($id, $sDefinition->getProperties(), true)) {
$code .= $this->addServiceMethodCalls(null, $sDefinition, $name);
$code .= $this->addServiceProperties(null, $sDefinition, $name);
$code .= $this->addServiceMethodCalls(null, $sDefinition, $name);
$code .= $this->addServiceConfigurator(null, $sDefinition, $name);
}

Expand Down Expand Up @@ -507,8 +507,8 @@ private function addServiceInlinedDefinitionsSetup($id, Definition $definition)
}

$name = (string) $this->definitionVariables->offsetGet($iDefinition);
$code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
$code .= $this->addServiceProperties(null, $iDefinition, $name);
$code .= $this->addServiceMethodCalls(null, $iDefinition, $name);
$code .= $this->addServiceConfigurator(null, $iDefinition, $name);
}

Expand Down Expand Up @@ -663,8 +663,8 @@ private function addService($id, Definition $definition)
$this->addServiceInlinedDefinitions($id, $definition).
$this->addServiceInstance($id, $definition).
$this->addServiceInlinedDefinitionsSetup($id, $definition).
$this->addServiceMethodCalls($id, $definition).
$this->addServiceProperties($id, $definition).
$this->addServiceMethodCalls($id, $definition).
$this->addServiceConfigurator($id, $definition).
$this->addServiceReturn($id, $definition)
;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -819,6 +819,20 @@ public function testLazyLoadedService()

$this->assertTrue($classInList);
}

public function testInitializePropertiesBeforeMethodCalls()
{
$container = new ContainerBuilder();
$container->register('foo', 'stdClass');
$container->register('bar', 'MethodCallClass')
->setProperty('simple', 'bar')
->setProperty('complex', new Reference('foo'))
->addMethodCall('callMe');

$container->compile();

$this->assertTrue($container->get('bar')->callPassed(), '->compile() initializes properties before method calls');
}
}

class FooClass
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,4 +267,23 @@ public function testInlinedDefinitionReferencingServiceContainer()
$dumper = new PhpDumper($container);
$this->assertStringEqualsFile(self::$fixturesPath.'/php/services13.php', $dumper->dump(), '->dump() dumps inline definitions which reference service_container');
}

public function testInitializePropertiesBeforeMethodCalls()
{
require_once self::$fixturesPath.'/includes/classes.php';

$container = new ContainerBuilder();
$container->register('foo', 'stdClass');
$container->register('bar', 'MethodCallClass')
->setProperty('simple', 'bar')
->setProperty('complex', new Reference('foo'))
->addMethodCall('callMe');
$container->compile();

$dumper = new PhpDumper($container);
eval('?>'.$dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls')));

$container = new \Symfony_DI_PhpDumper_Test_Properties_Before_Method_Calls();
$this->assertTrue($container->get('bar')->callPassed(), '->dump() initializes properties before method calls');
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -59,3 +59,20 @@ public function __construct(BarClass $bar)
$this->bar = $bar;
}
}

class MethodCallClass
{
public $simple;
public $complex;
private $callPassed = false;

public function callMe()
{
$this->callPassed = is_scalar($this->simple) && is_object($this->complex);
}

public function callPassed()
{
return $this->callPassed;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,11 @@ protected function getFooService()

$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo')), true, $this);

$instance->setBar($this->get('bar'));
$instance->initialize();
$instance->foo = 'bar';
$instance->moo = $a;
$instance->qux = array($this->getParameter('foo') => 'foo is '.$this->getParameter('foo').'', 'foobar' => $this->getParameter('foo'));
$instance->setBar($this->get('bar'));
$instance->initialize();
sc_configure($instance);

return $instance;
Expand Down Expand Up @@ -333,8 +333,8 @@ protected function getInlinedService()
{
$this->services['inlined'] = $instance = new \Bar();

$instance->setBaz($this->get('baz'));
$instance->pub = 'pub';
$instance->setBaz($this->get('baz'));

return $instance;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,11 +179,11 @@ protected function getFooService()

$this->services['foo'] = $instance = \Bar\FooClass::getInstance('foo', $a, array('bar' => 'foo is bar', 'foobar' => 'bar'), true, $this);

$instance->setBar($this->get('bar'));
$instance->initialize();
$instance->foo = 'bar';
$instance->moo = $a;
$instance->qux = array('bar' => 'foo is bar', 'foobar' => 'bar');
$instance->setBar($this->get('bar'));
$instance->initialize();
sc_configure($instance);

return $instance;
Expand Down Expand Up @@ -230,8 +230,8 @@ protected function getFooWithInlineService()

$this->services['foo_with_inline'] = $instance = new \Foo();

$a->setBaz($this->get('baz'));
$a->pub = 'pub';
$a->setBaz($this->get('baz'));

$instance->setBar($a);

Expand Down

0 comments on commit e62a390

Please sign in to comment.