Add missing tests for the resolving callbacks on the container

+ bug fix related to multiple calls to resolving callbacks
laravel · Dec 31, 2018 · e2194f0 · e2194f0
1 parent d329797
commit e2194f0
Show file tree

Hide file tree

Showing 2 changed files with 228 additions and 4 deletions.
diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php
@@ -133,6 +133,13 @@ class Container implements ArrayAccess, ContainerContract
      */
     protected $afterResolvingCallbacks = [];
 
+    /**
+     * All the abstract keys which are bound with class names.
+     *
+     * @var array
+     */
+    protected $boundToClassName = [];
+
     /**
      * Define a contextual binding.
      *
@@ -234,6 +241,7 @@ public function bind($abstract, $concrete = null, $shared = false)
         // bound into this container to the abstract type and we will just wrap it
         // up inside its own Closure to give us more convenience when extending.
         if (! $concrete instanceof Closure) {
+            $this->boundToClassName[$abstract] = null;
             $concrete = $this->getClosure($abstract, $concrete);
         }
 
@@ -1011,9 +1019,13 @@ protected function fireResolvingCallbacks($abstract, $object)
     {
         $this->fireCallbackArray($object, $this->globalResolvingCallbacks);
 
-        $this->fireCallbackArray(
-            $object, $this->getCallbacksForType($abstract, $object, $this->resolvingCallbacks)
-        );
+        // We don't fire "resolving" callbacks for interface bound with class path syntax
+        // since the callback will fire later, when the bounded concrete is resolving.
+        if (! $this->interfaceBoundedToClass($abstract)) {
+            $this->fireCallbackArray(
+                $object, $this->getCallbacksForType($abstract, $object, $this->resolvingCallbacks)
+            );
+        }
 
         $this->fireAfterResolvingCallbacks($abstract, $object);
     }
@@ -1137,7 +1149,7 @@ public function forgetExtenders($abstract)
      */
     protected function dropStaleInstances($abstract)
     {
-        unset($this->instances[$abstract], $this->aliases[$abstract]);
+        unset($this->instances[$abstract], $this->aliases[$abstract], $this->boundToClassName[$abstract]);
     }
 
     /**
@@ -1269,4 +1281,15 @@ public function __set($key, $value)
     {
         $this[$key] = $value;
     }
+
+    /**
+     * Determine if the abstract is both an interface and is bounded to a class path (not a closure).
+     *
+     * @param  string  $abstract
+     * @return bool
+     */
+    protected function interfaceBoundedToClass($abstract)
+    {
+        return interface_exists($abstract) && array_key_exists($abstract, $this->boundToClassName);
+    }
 }
diff --git a/tests/Container/ContainerTest.php b/tests/Container/ContainerTest.php
@@ -1022,6 +1022,207 @@ public function testResolvingCallbacksShouldBeFiredWhenCalledWithAliases()
         $this->assertEquals('taylor', $instance->name);
     }
 
+    public function testResolvingCallbacksAreCalledOnceForImplementation()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(IContainerContractStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, ContainerImplementationStub::class);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(1, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledOnceForStringAbstractions()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving('foo', function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind('foo', ContainerImplementationStub::class);
+
+        $container->make('foo');
+        $this->assertEquals(1, $callCounter);
+
+        $container->make('foo');
+        $this->assertEquals(2, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledOnceForImplementation2()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(IContainerContractStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, function () {
+            return new ContainerImplementationStub;
+        });
+
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(1, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(3, $callCounter);
+
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(4, $callCounter);
+    }
+
+    public function testRebindingDoesNotAffectResolvingCallbacks()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(IContainerContractStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, ContainerImplementationStub::class);
+        $container->bind(IContainerContractStub::class, function () {
+            return new ContainerImplementationStub;
+        });
+
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(1, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(3, $callCounter);
+
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(4, $callCounter);
+    }
+
+    public function testRebindingDoesNotAffectMultipleResolvingCallbacks()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+
+        $container->resolving(IContainerContractStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->resolving(ContainerImplementationStubTwo::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, ContainerImplementationStub::class);
+
+        // it should call the callback for interface
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(1, $callCounter);
+
+        // it should call the callback for interface
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+
+        // should call the callback for the interface it implements
+        // plus the callback for ContainerImplementationStubTwo.
+        $container->make(ContainerImplementationStubTwo::class);
+        $this->assertEquals(4, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledForInterfaces()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(IContainerContractStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, ContainerImplementationStub::class);
+
+        $container->make(IContainerContractStub::class);
+
+        $this->assertEquals(1, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledForConcretesWhenAttachedOnInterface()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(ContainerImplementationStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, ContainerImplementationStub::class);
+
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(1, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledForConcretesWhenAttachedOnConcretes()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(ContainerImplementationStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->bind(IContainerContractStub::class, ContainerImplementationStub::class);
+
+        $container->make(IContainerContractStub::class);
+        $this->assertEquals(1, $callCounter);
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledForConcretesWithNoBinding()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(ContainerImplementationStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(1, $callCounter);
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+    }
+
+    public function testResolvingCallbacksAreCalledForInterFacesWithNoBinding()
+    {
+        $container = new Container;
+
+        $callCounter = 0;
+        $container->resolving(IContainerContractStub::class, function ($some) use (&$callCounter) {
+            $callCounter++;
+        });
+
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(1, $callCounter);
+        $container->make(ContainerImplementationStub::class);
+        $this->assertEquals(2, $callCounter);
+    }
+
     public function testMakeWithMethodIsAnAliasForMakeMethod()
     {
         $mock = $this->getMockBuilder(Container::class)