[DI] Throw accurate failures when accessing removed services

symfony · fabpot · Oct 10, 2017 · Oct 7, 2017 · Oct 7, 2017 · fe7f26d4f32afe6c05dcd67ffd094158020cfdcd
commit fe7f26d4f32afe6c05dcd67ffd094158020cfdcd
diff --git a/src/Symfony/Component/DependencyInjection/Container.php b/src/Symfony/Component/DependencyInjection/Container.php
@@ -50,6 +50,7 @@ class Container implements ResettableContainerInterface
    protected $aliases = array();
    protected $loading = array();
    protected $resolving = array();
+    protected $syntheticIds = array();

    /**
     * @internal
@@ -179,31 +180,34 @@ public function set($id, $service)
            throw new InvalidArgumentException('You cannot set service "service_container".');
        }

-        if (isset($this->aliases[$id])) {
-            unset($this->aliases[$id]);
-        }
-
-        $wasSet = isset($this->services[$id]);
-        $this->services[$id] = $service;
-
-        if (null === $service) {
-            unset($this->services[$id]);
-        }
-
-        if (isset($this->privates[$id])) {
-            if (null === $service) {
+        if (isset($this->privates[$id]) || !(isset($this->fileMap[$id]) || isset($this->methodMap[$id]))) {
+            if (isset($this->syntheticIds[$id]) || (!isset($this->privates[$id]) && !isset($this->getRemovedIds()[$id]))) {
+                // no-op
+            } elseif (null === $service) {
                @trigger_error(sprintf('The "%s" service is private, unsetting it is deprecated since Symfony 3.2 and will fail in 4.0.', $id), E_USER_DEPRECATED);
                unset($this->privates[$id]);
            } else {
                @trigger_error(sprintf('The "%s" service is private, replacing it is deprecated since Symfony 3.2 and will fail in 4.0.', $id), E_USER_DEPRECATED);
            }
-        } elseif ($wasSet && (isset($this->fileMap[$id]) || isset($this->methodMap[$id]))) {
+        } elseif (isset($this->services[$id])) {
            if (null === $service) {
                @trigger_error(sprintf('The "%s" service is already initialized, unsetting it is deprecated since Symfony 3.3 and will fail in 4.0.', $id), E_USER_DEPRECATED);
            } else {
                @trigger_error(sprintf('The "%s" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.', $id), E_USER_DEPRECATED);
            }
        }
+
+        if (isset($this->aliases[$id])) {
+            unset($this->aliases[$id]);
+        }
+
+        if (null === $service) {
+            unset($this->services[$id]);
+
+            return;
+        }
+
+        $this->services[$id] = $service;
    }

    /**
@@ -275,7 +279,7 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
        // calling $this->normalizeId($id) unless necessary.
        for ($i = 2;;) {
            if (isset($this->privates[$id])) {
-                @trigger_error(sprintf('The "%s" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.', $id), E_USER_DEPRECATED);
+                @trigger_error(sprintf('The "%s" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.', $id), E_USER_DEPRECATED);
            }
            if (isset($this->aliases[$id])) {
                $id = $this->aliases[$id];
@@ -325,6 +329,12 @@ public function get($id, $invalidBehavior = self::EXCEPTION_ON_INVALID_REFERENCE
            if (!$id) {
                throw new ServiceNotFoundException($id);
            }
+            if (isset($this->syntheticIds[$id])) {
+                throw new ServiceNotFoundException($id, null, null, array(), sprintf('The "%s" service is synthetic, it needs to be set at boot time before it can be used.', $id));
+            }
+            if (isset($this->getRemovedIds()[$id])) {
+                throw new ServiceNotFoundException($id, null, null, array(), sprintf('The "%s" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.', $id));
+            }

            $alternatives = array();
            foreach ($this->getServiceIds() as $knownId) {
@@ -397,6 +407,16 @@ public function getServiceIds()
        return array_unique(array_merge($ids, array_keys($this->methodMap), array_keys($this->fileMap), array_keys($this->services)));
    }

+    /**
+     * Gets service ids that existed at compile time.
+     *
+     * @return array
+     */
+    public function getRemovedIds()
+    {
+        return array();
+    }
+
    /**
     * Camelizes a string.
     *

diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -121,6 +121,8 @@ class ContainerBuilder extends Container implements TaggedContainerInterface

    private $autoconfiguredInstanceof = array();

+    private $removedIds = array();
+
    public function __construct(ParameterBagInterface $parameterBag = null)
    {
        parent::__construct($parameterBag);
@@ -517,7 +519,7 @@ public function set($id, $service)
            throw new BadMethodCallException(sprintf('Setting service "%s" for an unknown or non-synthetic service definition on a compiled container is not allowed.', $id));
        }

-        unset($this->definitions[$id], $this->aliasDefinitions[$id]);
+        unset($this->definitions[$id], $this->aliasDefinitions[$id], $this->removedIds[$id]);

        parent::set($id, $service);
    }
@@ -529,7 +531,10 @@ public function set($id, $service)
     */
    public function removeDefinition($id)
    {
-        unset($this->definitions[$this->normalizeId($id)]);
+        if (isset($this->definitions[$id = $this->normalizeId($id)])) {
+            unset($this->definitions[$id]);
+            $this->removedIds[$id] = true;
+        }
    }

    /**
@@ -793,6 +798,16 @@ public function getServiceIds()
        return array_unique(array_merge(array_keys($this->getDefinitions()), array_keys($this->aliasDefinitions), parent::getServiceIds()));
    }

+    /**
+     * Gets removed service or alias ids.
+     *
+     * @return array
+     */
+    public function getRemovedIds()
+    {
+        return $this->removedIds;
+    }
+
    /**
     * Adds the service aliases.
     *
@@ -841,7 +856,7 @@ public function setAlias($alias, $id)
            throw new InvalidArgumentException(sprintf('An alias can not reference itself, got a circular reference on "%s".', $alias));
        }

-        unset($this->definitions[$alias]);
+        unset($this->definitions[$alias], $this->removedIds[$alias]);

        return $this->aliasDefinitions[$alias] = $id;
    }
@@ -853,7 +868,10 @@ public function setAlias($alias, $id)
     */
    public function removeAlias($alias)
    {
-        unset($this->aliasDefinitions[$this->normalizeId($alias)]);
+        if (isset($this->aliasDefinitions[$alias = $this->normalizeId($alias)])) {
+            unset($this->aliasDefinitions[$alias]);
+            $this->removedIds[$alias] = true;
+        }
    }

    /**
@@ -981,7 +999,7 @@ public function setDefinition($id, Definition $definition)

        $id = $this->normalizeId($id);

-        unset($this->aliasDefinitions[$id]);
+        unset($this->aliasDefinitions[$id], $this->removedIds[$id]);

        return $this->definitions[$id] = $definition;
    }

diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php
@@ -172,6 +172,16 @@ public function dump(array $options = array())

 EOF;
            $files = array();
+
+            if ($ids = array_keys($this->container->getRemovedIds())) {
+                sort($ids);
+                $c = "<?php\n\nreturn array(\n";
+                foreach ($ids as $id) {
+                    $c .= '    '.$this->export($id)." => true,\n";
+                }
+                $files['removed-ids.php'] = $c .= ");\n";
+            }
+
            foreach ($this->generateServiceFiles() as $file => $c) {
                $files[$file] = $fileStart.$c;
            }
@@ -888,6 +898,7 @@ public function __construct()
        }

        $code .= $this->addNormalizedIds();
+        $code .= $this->addSyntheticIds();
        $code .= $this->addMethodMap();
        $code .= $this->asFiles ? $this->addFileMap() : '';
        $code .= $this->addPrivateServices();
@@ -896,6 +907,8 @@ public function __construct()
    }

 EOF;
+        $code .= $this->addRemovedIds();
+
        if ($this->container->isCompiled()) {
            $code .= <<<EOF

@@ -978,6 +991,58 @@ private function addNormalizedIds()
        return $code ? "        \$this->normalizedIds = array(\n".$code."        );\n" : '';
    }

+    /**
+     * Adds the syntheticIds definition.
+     *
+     * @return string
+     */
+    private function addSyntheticIds()
+    {
+        $code = '';
+        $definitions = $this->container->getDefinitions();
+        ksort($definitions);
+        foreach ($definitions as $id => $definition) {
+            if ($definition->isSynthetic() && 'service_container' !== $id) {
+                $code .= '            '.$this->export($id)." => true,\n";
+            }
+        }
+
+        return $code ? "        \$this->syntheticIds = array(\n{$code}        );\n" : '';
+    }
+
+    /**
+     * Adds the removedIds definition.
+     *
+     * @return string
+     */
+    private function addRemovedIds()
+    {
+        if (!$ids = $this->container->getRemovedIds()) {
+            return '';
+        }
+        if ($this->asFiles) {
+            $code = "require __DIR__.'/removed-ids.php'";
+        } else {
+            $code = '';
+            $ids = array_keys($ids);
+            sort($ids);
+            foreach ($ids as $id) {
+                $code .= '            '.$this->export($id)." => true,\n";
+            }
+
+            $code = "array(\n{$code}        )";
+        }
+
+        return <<<EOF
+
+    public function getRemovedIds()
+    {
+        return {$code};
+    }
+
+EOF;
+    }
+
    /**
     * Adds the methodMap property definition.
     *

diff --git a/src/Symfony/Component/DependencyInjection/Exception/ServiceNotFoundException.php b/src/Symfony/Component/DependencyInjection/Exception/ServiceNotFoundException.php
@@ -24,9 +24,11 @@ class ServiceNotFoundException extends InvalidArgumentException implements NotFo
    private $sourceId;
    private $alternatives;

-    public function __construct($id, $sourceId = null, \Exception $previous = null, array $alternatives = array())
+    public function __construct($id, $sourceId = null, \Exception $previous = null, array $alternatives = array(), $msg = null)
    {
-        if (null === $sourceId) {
+        if (null !== $msg) {
+            // no-op
+        } elseif (null === $sourceId) {
            $msg = sprintf('You have requested a non-existent service "%s".', $id);
        } else {
            $msg = sprintf('The service "%s" has a dependency on a non-existent service "%s".', $sourceId, $id);

diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerTest.php
@@ -333,16 +333,28 @@ public function testGetCircularReference()

    /**
     * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
-     * @expectedExceptionMessage You have requested a non-existent service "request".
+     * @expectedExceptionMessage The "request" service is synthetic, it needs to be set at boot time before it can be used.
     */
    public function testGetSyntheticServiceThrows()
    {
-        require_once __DIR__.'/Fixtures/php/services9.php';
+        require_once __DIR__.'/Fixtures/php/services9_compiled.php';

        $container = new \ProjectServiceContainer();
        $container->get('request');
    }

+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
+     * @expectedExceptionMessage The "inlined" service or alias has been removed or inlined when the container was compiled. You should either make it public, or stop using the container directly and use dependency injection instead.
+     */
+    public function testGetRemovedServiceThrows()
+    {
+        require_once __DIR__.'/Fixtures/php/services9_compiled.php';
+
+        $container = new \ProjectServiceContainer();
+        $container->get('inlined');
+    }
+
    public function testHas()
    {
        $sc = new ProjectServiceContainer();
@@ -505,7 +517,7 @@ public function testCheckExistenceOfAnInternalPrivateServiceIsDeprecated()

    /**
     * @group legacy
-     * @expectedDeprecation The "internal" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
+     * @expectedDeprecation The "internal" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
     */
    public function testRequestAnInternalSharedPrivateServiceIsDeprecated()
    {

diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php
@@ -287,37 +287,17 @@ public function testFrozenContainerWithoutAliases()

    /**
     * @group legacy
-     * @expectedDeprecation The "bar" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.
+     * @expectedDeprecation The "decorator_service" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.
     */
    public function testOverrideServiceWhenUsingADumpedContainer()
    {
-        require_once self::$fixturesPath.'/php/services9.php';
-        require_once self::$fixturesPath.'/includes/foo.php';
+        require_once self::$fixturesPath.'/php/services9_compiled.php';

        $container = new \ProjectServiceContainer();
-        $container->setParameter('foo_bar', 'foo_bar');
-        $container->get('bar');
-        $container->set('bar', $bar = new \stdClass());
-
-        $this->assertSame($bar, $container->get('bar'), '->set() overrides an already defined service');
-    }
-
-    /**
-     * @group legacy
-     * @expectedDeprecation The "bar" service is already initialized, replacing it is deprecated since Symfony 3.3 and will fail in 4.0.
-     */
-    public function testOverrideServiceWhenUsingADumpedContainerAndServiceIsUsedFromAnotherOne()
-    {
-        require_once self::$fixturesPath.'/php/services9.php';
-        require_once self::$fixturesPath.'/includes/foo.php';
-        require_once self::$fixturesPath.'/includes/classes.php';
-
-        $container = new \ProjectServiceContainer();
-        $container->setParameter('foo_bar', 'foo_bar');
-        $container->get('bar');
-        $container->set('bar', $bar = new \stdClass());
+        $container->get('decorator_service');
+        $container->set('decorator_service', $decorator = new \stdClass());

-        $this->assertSame($bar, $container->get('foo')->bar, '->set() overrides an already defined service');
+        $this->assertSame($decorator, $container->get('decorator_service'), '->set() overrides an already defined service');
    }

    /**
@@ -799,14 +779,14 @@ public function testDumpHandlesLiteralClassWithRootNamespace()

    /**
     * @group legacy
-     * @expectedDeprecation The "private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "decorated_private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "decorated_private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "private_not_inlined" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "private_not_removed" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "private_child" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
-     * @expectedDeprecation The "private_parent" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop getting services directly from the container and use dependency injection instead.
+     * @expectedDeprecation The "private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "decorated_private" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "decorated_private_alias" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "private_not_inlined" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "private_not_removed" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "private_child" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
+     * @expectedDeprecation The "private_parent" service is private, getting it from the container is deprecated since Symfony 3.2 and will fail in 4.0. You should either make the service public, or stop using the container directly and use dependency injection instead.
     */
    public function testLegacyPrivateServices()
    {