Added recursive dependency detection

Solution for #10
Dhii · Sep 15, 2020 · c2aeb03 · c2aeb03
1 parent 55d5278
commit c2aeb03
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 17 deletions.
diff --git a/src/DelegatingContainer.php b/src/DelegatingContainer.php
@@ -38,43 +38,88 @@ public function __construct(ServiceProviderInterface $provider, PsrContainerInte
      * {@inheritDoc}
      */
     public function get($id)
+    {
+        static $stack = [];
+
+        if (array_key_exists($id, $stack)) {
+            $trace = implode(' -> ', array_keys($stack)) . ' -> ' . $id;
+
+            throw new ContainerException(
+                $this->__("Circular dependency detected:\n%s", [$trace]),
+                0,
+                null
+            );
+        }
+
+        $stack[$id] = true;
+
+        try {
+            return $this->_createService($id);
+        } finally {
+            unset($stack[$id]);
+        }
+    }
+
+    /**
+     * {@inheritDoc}
+     */
+    public function has($id)
+    {
+        $services = $this->provider->getFactories();
+
+        return array_key_exists($id, $services);
+    }
+
+    /**
+     * Creates a service, using the factory that corresponds to a specific key.
+     *
+     * @since [*next-version*]
+     *
+     * @param string $key The key of the service to be created.
+     *
+     * @return mixed The created service.
+     *
+     * @throws NotFoundException If no factory corresponds to the given $key.
+     * @throws ContainerException If an error occurred while creating the service.
+     */
+    protected function _createService(string $key)
     {
         $provider = $this->provider;
         $services = $provider->getFactories();
 
-        if (!array_key_exists($id, $services)) {
+        if (!array_key_exists($key, $services)) {
             throw new NotFoundException(
-                $this->__('Service not found for key "%1$s"', [$id]),
+                $this->__('Service not found for key "%1$s"', [$key]),
                 0,
                 null
             );
         }
 
-        $service = $services[$id];
+        $service = $services[$key];
 
         try {
             $service = $this->_invokeFactory($service);
         } catch (UnexpectedValueException $e) {
             throw new ContainerException(
-                $this->__('Could not create service "%1$s"', [$id]),
+                $this->__('Could not create service "%1$s"', [$key]),
                 0,
                 $e
             );
         }
 
         $extensions = $provider->getExtensions();
 
-        if (!array_key_exists($id, $extensions)) {
+        if (!array_key_exists($key, $extensions)) {
             return $service;
         }
 
-        $extension = $extensions[$id];
+        $extension = $extensions[$key];
 
         try {
             $service = $this->_invokeExtension($extension, $service);
         } catch (UnexpectedValueException $e) {
             throw new ContainerException(
-                $this->__('Could not extend service "%1$s"', [$id]),
+                $this->__('Could not extend service "%1$s"', [$key]),
                 0,
                 $e
             );
@@ -83,16 +128,6 @@ public function get($id)
         return $service;
     }
 
-    /**
-     * {@inheritDoc}
-     */
-    public function has($id)
-    {
-        $services = $this->provider->getFactories();
-
-        return array_key_exists($id, $services);
-    }
-
     /**
      * Retrieves a service by invoking its factory.
      *

diff --git a/test/functional/DelegatingContainerTest.php b/test/functional/DelegatingContainerTest.php
@@ -8,6 +8,7 @@
 use Dhii\Container\TestHelpers\ServiceProviderMock;
 use Exception;
 use PHPUnit\Framework\TestCase;
+use Psr\Container\ContainerExceptionInterface;
 use Psr\Container\ContainerInterface;
 
 class DelegatingContainerTest extends TestCase
@@ -87,4 +88,35 @@ public function testHasFalse()
             $this->assertFalse($result, 'Wrongly determined not having');
         }
     }
+
+    public function testRecursiveDetection()
+    {
+        {
+            $provider = ServiceProviderMock::create($this, [
+                'a' => function (ContainerInterface $c) {
+                    return $c->get('b');
+                },
+                'b' => function (ContainerInterface $c) {
+                    return $c->get('c');
+                },
+                'c' => function (ContainerInterface $c) {
+                    return $c->get('a');
+                },
+            ]);
+
+            $subject = new DelegatingContainer($provider);
+        }
+        {
+            try {
+                $subject->get('a');
+                $this->fail('Expected exception to be thrown');
+            } catch (ContainerExceptionInterface $exception) {
+                $this->assertStringContainsString(
+                    'a -> b -> c -> a',
+                    $exception->getMessage(),
+                    'Exception message does not properly report circular dependency'
+                );
+            }
+        }
+    }
 }