Merge pull request #12 from stellarwp/fix/overwritten-nested-dependen…

…cies Let make() return a cached dependency if resolutionCacheDepth > 0
stellarwp · Mar 22, 2022 · 00b60d1 · 00b60d1
2 parents 1eea8f3 + a1b3cae
commit 00b60d1
Show file tree

Hide file tree

Showing 3 changed files with 29 additions and 7 deletions.
diff --git a/src/Container.php b/src/Container.php
@@ -184,12 +184,19 @@ public function make($abstract)
     {
         $config = array_merge($this->config(), $this->extensions);
 
+        // If caching is enabled and we have a resolution, return it immediately.
+        if ($this->resolutionCacheDepth > 0 && array_key_exists($abstract, $this->resolved)) {
+            return $this->resolved[$abstract];
+        }
+
+        // No definition exists in the config for this abstract.
         if (! array_key_exists($abstract, $config)) {
             throw new NotFoundException(
                 sprintf('No container definition could be found for "%s".', $abstract)
             );
         }
 
+        // Catch recursive resolutions.
         if (isset($this->currentlyResolving[$abstract])) {
             throw new RecursiveDependencyException(
                 sprintf('Recursion detected when attempting to resolve "%s"', $abstract)
@@ -230,7 +237,7 @@ public function make($abstract)
         }
 
         // If the cache is enabled, cache this resolution.
-        if ($this->resolutionCacheDepth > 0 && ! array_key_exists($abstract, $this->resolved)) {
+        if ($this->resolutionCacheDepth > 0) {
             $this->resolved[$abstract] = $resolved;
         }
 

diff --git a/tests/Stubs/Concrete.php b/tests/Stubs/Concrete.php
@@ -31,13 +31,19 @@ public function config()
                 return new \stdClass();
             },
             self::NESTED_GET_KEY       => function ($app) {
-                return new \ArrayObject($app->get(self::VALID_KEY));
+                return new \ArrayObject([
+                    'prop' => $app->get(self::VALID_KEY),
+                ]);
             },
             self::NESTED_MAKE_KEY      => function ($app) {
-                return new \ArrayObject($app->make(self::VALID_KEY));
+                return new \ArrayObject([
+                    'prop' => $app->make(self::VALID_KEY),
+                ]);
             },
             self::NESTED_UNDEFINED_KEY => function ($app) {
-                return new \ArrayObject($app->make(self::UNDEFINED_KEY));
+                return new \ArrayObject([
+                    'prop' => $app->make(self::UNDEFINED_KEY),
+                ]);
             },
             self::NULL_KEY             => null,
             self::ALIAS_KEY            => self::VALID_KEY,

diff --git a/tests/Unit/ContainerTest.php b/tests/Unit/ContainerTest.php
@@ -440,15 +440,24 @@ public function calling_make_should_not_overwrite_cached_resolutions()
 
     /**
      * @test
-     * @testdox Calling make() should not overwrite cached resolutions
+     * @testdox Calling make() should not overwrite cached, nested resolutions
      */
     public function calling_make_should_not_overwrite_nested_cached_resolutions()
     {
         $container = new Concrete();
         $valid     = $container->get(Concrete::VALID_KEY);
 
-        $container->get(Concrete::NESTED_MAKE_KEY);
-        $this->assertSame($valid, $container->get(Concrete::VALID_KEY));
+        $instance = $container->get(Concrete::NESTED_MAKE_KEY);
+        $this->assertSame(
+            $valid,
+            $instance->offsetGet('prop'),
+            'The resolved value should have been returned from cache.'
+        );
+        $this->assertSame(
+            $valid,
+            $container->get(Concrete::VALID_KEY),
+            'The cache should not have been overwritten'
+        );
     }
 
     /**