Skip to content

Commit

Permalink
Call extension method in registry
Browse files Browse the repository at this point in the history
The definition given to Symfony container would always call the method
which creates a service in the registry. It led to unexpected behaviours
that could not be perceived in current tests because the extension
factories don't return a different value.
  • Loading branch information
romm committed Apr 12, 2020
1 parent 7ac2d64 commit 639f166
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 6 deletions.
7 changes: 4 additions & 3 deletions src/ServiceProviderCompilationPass.php
Expand Up @@ -95,7 +95,7 @@ private function registerService($serviceName, $serviceProviderKey, $callable, C
}

private function extendService($serviceName, $serviceProviderKey, $callable, ContainerBuilder $container) {
$factoryDefinition = $this->getServiceDefinitionFromCallable($serviceName, $serviceProviderKey, $callable);
$factoryDefinition = $this->getServiceDefinitionFromCallable($serviceName, $serviceProviderKey, $callable, true);

if (!$container->has($serviceName)) {
$container->setDefinition($serviceName, $factoryDefinition);
Expand Down Expand Up @@ -129,7 +129,7 @@ private function getDecoratedServiceName($serviceName, ContainerBuilder $contain
return $serviceName.'_decorated_'.$counter;
}

private function getServiceDefinitionFromCallable($serviceName, $serviceProviderKey, callable $callable)
private function getServiceDefinitionFromCallable($serviceName, $serviceProviderKey, callable $callable, $isExtension = false)
{
/*if ($callable instanceof DefinitionInterface) {
// TODO: plug the definition-interop converter here!
Expand All @@ -141,7 +141,8 @@ private function getServiceDefinitionFromCallable($serviceName, $serviceProvider
$factoryDefinition->setFactory($callable);
$factoryDefinition->addArgument(new Reference('interop_service_provider_acclimated_container'));
} else {
$factoryDefinition->setFactory([ new Reference('service_provider_registry_'.$this->registryId), 'createService' ]);
$method = $isExtension ? 'extendService' : 'createService';
$factoryDefinition->setFactory([ new Reference('service_provider_registry_'.$this->registryId), $method ]);
$factoryDefinition->addArgument($serviceProviderKey);
$factoryDefinition->addArgument($serviceName);
$factoryDefinition->addArgument($containerDefinition);
Expand Down
6 changes: 5 additions & 1 deletion src/Tests/Fixtures/TestServiceProvider.php
Expand Up @@ -35,6 +35,10 @@ public static function createServiceB(ContainerInterface $container)

public function getExtensions()
{
return [];
return [
'stringValue' => function (ContainerInterface $container, $value) {
return $value . '1';
},
];
}
}
9 changes: 7 additions & 2 deletions src/Tests/Fixtures/TestServiceProviderOverride.php
Expand Up @@ -8,7 +8,9 @@ class TestServiceProviderOverride implements ServiceProviderInterface
{
public function getFactories()
{
return [];
return [
'stringValue' => function () { return 'foo'; },
];
}

public static function overrideServiceA(ContainerInterface $container, \stdClass $serviceA = null)
Expand All @@ -20,7 +22,10 @@ public static function overrideServiceA(ContainerInterface $container, \stdClass
public function getExtensions()
{
return [
'serviceA' => [ TestServiceProviderOverride::class, 'overrideServiceA' ]
'serviceA' => [ TestServiceProviderOverride::class, 'overrideServiceA' ],
'stringValue' => function (ContainerInterface $container, $value) {
return $value . '2';
},
];
}
}
12 changes: 12 additions & 0 deletions tests/ServiceProviderCompilationPassTest.php
Expand Up @@ -56,6 +56,18 @@ public function testServiceProviderOverrides()
$this->assertEquals('bar', $serviceA->newProperty2);
}

public function testExtensionsAreCalledInCorrectOrder()
{
$container = $this->getContainer([
new TestServiceProvider(),
new TestServiceProviderOverride(),
]);

$value = $container->get('stringValue');

$this->assertSame('foo12', $value);
}

/**
* @expectedException \TheCodingMachine\Interop\ServiceProviderBridgeBundle\Exception\InvalidArgumentException
*/
Expand Down

0 comments on commit 639f166

Please sign in to comment.