Use the method map as authoritative list of factories for dumped containers

The initial implementation of the method factory discovery was based on a
naming convention for factory methods. However, this naming convention allowed
to generate the same name for multiple ids. In the meantime, a method map was
introduced to solve this issue (and others).
When accessing a service with a different id than the official one (thanks to
ambiguities), this breaks the sharing of the service, as it creates a new
instance each time and replaces the existing shared instance. This was also
inconsistent between a dumped container (affected by this) and a non-dumped
container (reporting a service not found error for the other id).

The method map is now the authoritative way to discover available service
factories. When the dumped container was generated with a method map (which
is the case when using the dumper shipped in the component), the logic based
on defined methods is not executed anymore. This forbids using another id than
the real one to access the service (preventing to trigger the broken behavior).
When using a dumper which does not fill the method map, the old logic is still
applied, but deprecation warnings are triggered on access to dumped services.
This commit is contained in:
Christophe Coevoet 2016-09-30 23:26:24 +02:00
parent 418944bb50
commit 03b91089e0
4 changed files with 191 additions and 14 deletions

View File

@ -17,6 +17,10 @@ use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
*/
class LazyServiceProjectServiceContainer extends Container
{
protected $methodMap = array(
'foo' => 'getFooService',
);
/**
* Constructor.
*/

View File

@ -17,6 +17,10 @@ use Symfony\Component\DependencyInjection\ParameterBag\FrozenParameterBag;
*/
class LazyServiceProjectServiceContainer extends Container
{
protected $methodMap = array(
'foo' => 'getFooService',
);
/**
* Constructor.
*/

View File

@ -206,15 +206,29 @@ class Container implements ResettableContainerInterface
) {
return true;
}
if (isset($this->privates[$id])) {
@trigger_error(sprintf('Checking for the existence of the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
}
if (isset($this->methodMap[$id])) {
return true;
}
if (--$i && $id !== $lcId = strtolower($id)) {
$id = $lcId;
} else {
if (isset($this->privates[$id])) {
@trigger_error(sprintf('Checking for the existence of the "%s" private service is deprecated since Symfony 3.2 and won\'t be supported anymore in Symfony 4.0.', $id), E_USER_DEPRECATED);
}
return method_exists($this, 'get'.strtr($id, $this->underscoreMap).'Service');
continue;
}
// We only check the convention-based factory in a compiled container (i.e. a child class other than a ContainerBuilder,
// and only when the dumper has not generated the method map (otherwise the method map is considered to be fully populated by the dumper)
if (!$this->methodMap && !$this instanceof ContainerBuilder && __CLASS__ !== static::class && method_exists($this, 'get'.strtr($id, $this->underscoreMap).'Service')) {
@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 true;
}
return false;
}
}
@ -262,7 +276,10 @@ class Container implements ResettableContainerInterface
} elseif (--$i && $id !== $lcId = strtolower($id)) {
$id = $lcId;
continue;
} elseif (method_exists($this, $method = 'get'.strtr($id, $this->underscoreMap).'Service')) {
} elseif (!$this->methodMap && !$this instanceof ContainerBuilder && __CLASS__ !== static::class && method_exists($this, $method = 'get'.strtr($id, $this->underscoreMap).'Service')) {
// We only check the convention-based factory in a compiled container (i.e. a child class other than a ContainerBuilder,
// 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);
// $method is set to the right value, proceed
} else {
if (self::EXCEPTION_ON_INVALID_REFERENCE === $invalidBehavior) {
@ -341,14 +358,21 @@ class Container implements ResettableContainerInterface
public function getServiceIds()
{
$ids = array();
foreach (get_class_methods($this) as $method) {
if (preg_match('/^get(.+)Service$/', $method, $match)) {
$ids[] = self::underscore($match[1]);
if (!$this->methodMap && !$this instanceof ContainerBuilder && __CLASS__ !== static::class) {
// We only check the convention-based factory in a compiled container (i.e. a child class other than a ContainerBuilder,
// 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);
foreach (get_class_methods($this) as $method) {
if (preg_match('/^get(.+)Service$/', $method, $match)) {
$ids[] = self::underscore($match[1]);
}
}
}
$ids[] = 'service_container';
return array_unique(array_merge($ids, array_keys($this->services)));
return array_unique(array_merge($ids, array_keys($this->methodMap), array_keys($this->services)));
}
/**

View File

@ -126,7 +126,25 @@ class ContainerTest extends \PHPUnit_Framework_TestCase
$sc = new ProjectServiceContainer();
$sc->set('foo', $obj = new \stdClass());
$this->assertEquals(array('internal', 'bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'service_container', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by getXXXService() methods, followed by service ids defined by set()');
$this->assertEquals(array('service_container', 'internal', 'bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by factory methods in the method map, followed by service ids defined by set()');
}
/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
*/
public function testGetLegacyServiceIds()
{
$sc = new LegacyProjectServiceContainer();
$sc->set('foo', $obj = new \stdClass());
$deprecations = array(
'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.',
);
ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () use ($sc) {
$this->assertEquals(array('internal', 'bar', 'foo_bar', 'foo.baz', 'circular', 'throw_exception', 'throws_exception_on_service_configuration', 'service_container', 'foo'), $sc->getServiceIds(), '->getServiceIds() returns defined service ids by getXXXService() methods, followed by service ids defined by set()');
});
}
public function testSet()
@ -160,7 +178,6 @@ class ContainerTest extends \PHPUnit_Framework_TestCase
$this->assertEquals($sc->__bar, $sc->get('bar'), '->get() returns the service for the given id');
$this->assertEquals($sc->__foo_bar, $sc->get('foo_bar'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo.baz'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo\\baz'), '->get() returns the service if a get*Method() is defined');
$sc->set('bar', $bar = new \stdClass());
$this->assertEquals($bar, $sc->get('bar'), '->get() prefers to return a service defined with set() than one defined with a getXXXMethod()');
@ -174,6 +191,43 @@ class ContainerTest extends \PHPUnit_Framework_TestCase
$this->assertNull($sc->get('', ContainerInterface::NULL_ON_INVALID_REFERENCE), '->get() returns null if the service is empty');
}
/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
*/
public function testLegacyGet()
{
$sc = new LegacyProjectServiceContainer();
$sc->set('foo', $foo = new \stdClass());
$deprecations = array(
'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.',
'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.',
'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.',
'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.',
);
ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () use ($sc, $foo) {
$this->assertEquals($foo, $sc->get('foo'), '->get() returns the service for the given id');
$this->assertEquals($foo, $sc->get('Foo'), '->get() returns the service for the given id, and converts id to lowercase');
$this->assertEquals($sc->__bar, $sc->get('bar'), '->get() returns the service for the given id');
$this->assertEquals($sc->__foo_bar, $sc->get('foo_bar'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo.baz'), '->get() returns the service if a get*Method() is defined');
$this->assertEquals($sc->__foo_baz, $sc->get('foo\\baz'), '->get() returns the service if a get*Method() is defined');
$sc->set('bar', $bar = new \stdClass());
$this->assertEquals($bar, $sc->get('bar'), '->get() prefers to return a service defined with set() than one defined with a getXXXMethod()');
try {
$sc->get('');
$this->fail('->get() throws a \InvalidArgumentException exception if the service is empty');
} catch (\Exception $e) {
$this->assertInstanceOf('Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException', $e, '->get() throws a ServiceNotFoundException exception if the service is empty');
}
$this->assertNull($sc->get('', ContainerInterface::NULL_ON_INVALID_REFERENCE), '->get() returns null if the service is empty');
});
}
public function testGetThrowServiceNotFoundException()
{
$sc = new ProjectServiceContainer();
@ -230,7 +284,32 @@ class ContainerTest extends \PHPUnit_Framework_TestCase
$this->assertTrue($sc->has('bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo_bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo.baz'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo\\baz'), '->has() returns true if a get*Method() is defined');
}
/**
* @group legacy
* @requires function Symfony\Bridge\PhpUnit\ErrorAssert::assertDeprecationsAreTriggered
*/
public function testLegacyHas()
{
$sc = new LegacyProjectServiceContainer();
$sc->set('foo', new \stdClass());
$deprecations = array(
'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.',
'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.',
'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.',
'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.',
);
ErrorAssert::assertDeprecationsAreTriggered($deprecations, function () use ($sc) {
$this->assertFalse($sc->has('foo1'), '->has() returns false if the service does not exist');
$this->assertTrue($sc->has('foo'), '->has() returns true if the service exists');
$this->assertTrue($sc->has('bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo_bar'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo.baz'), '->has() returns true if a get*Method() is defined');
$this->assertTrue($sc->has('foo\\baz'), '->has() returns true if a get*Method() is defined');
});
}
public function testInitialized()
@ -385,6 +464,72 @@ class ContainerTest extends \PHPUnit_Framework_TestCase
}
class ProjectServiceContainer extends Container
{
public $__bar;
public $__foo_bar;
public $__foo_baz;
public $__internal;
protected $methodMap = array(
'internal' => 'getInternalService',
'bar' => 'getBarService',
'foo_bar' => 'getFooBarService',
'foo.baz' => 'getFoo_BazService',
'circular' => 'getCircularService',
'throw_exception' => 'getThrowExceptionService',
'throws_exception_on_service_configuration' => 'getThrowsExceptionOnServiceConfigurationService',
);
public function __construct()
{
parent::__construct();
$this->__bar = new \stdClass();
$this->__foo_bar = new \stdClass();
$this->__foo_baz = new \stdClass();
$this->__internal = new \stdClass();
$this->privates = array('internal' => true);
$this->aliases = array('alias' => 'bar');
}
protected function getInternalService()
{
return $this->__internal;
}
protected function getBarService()
{
return $this->__bar;
}
protected function getFooBarService()
{
return $this->__foo_bar;
}
protected function getFoo_BazService()
{
return $this->__foo_baz;
}
protected function getCircularService()
{
return $this->get('circular');
}
protected function getThrowExceptionService()
{
throw new \Exception('Something went terribly wrong!');
}
protected function getThrowsExceptionOnServiceConfigurationService()
{
$this->services['throws_exception_on_service_configuration'] = $instance = new \stdClass();
throw new \Exception('Something was terribly wrong while trying to configure the service!');
}
}
class LegacyProjectServiceContainer extends Container
{
public $__bar;
public $__foo_bar;