From 22d15f55fe11e50cf8e5268b5b1d43d79f1d9fde Mon Sep 17 00:00:00 2001 From: Tom Lankhorst Date: Mon, 26 Mar 2018 15:26:10 +0200 Subject: [PATCH 1/6] Test resolving() callbacks being fired once for implementations --- tests/Container/ContainerTest.php | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/tests/Container/ContainerTest.php b/tests/Container/ContainerTest.php index 352e756f8d24..b9539254d6b8 100755 --- a/tests/Container/ContainerTest.php +++ b/tests/Container/ContainerTest.php @@ -378,6 +378,25 @@ public function testResolvingCallbacksAreCalledForType() $this->assertEquals('taylor', $instance->name); } + public function testResolvingCallbacksAreCalledOnceForImplementations() + { + $container = new Container; + $resolving_invocations = 0; + $after_resolving_invocations = 0; + + $container->resolving( IContainerContractStub::class, function( ) use ( &$resolving_invocations ) { + $resolving_invocations++; + } ); + $container->afterResolving( IContainerContractStub::class, function( ) use ( &$after_resolving_invocations ) { + $after_resolving_invocations++; + } ); + $container->bind(IContainerContractStub::class, ContainerImplementationStub::class ); + $container->make( IContainerContractStub::class ); + + $this->assertEquals( 1, $resolving_invocations ); + $this->assertEquals( 1, $after_resolving_invocations ); + } + public function testUnsetRemoveBoundInstances() { $container = new Container; From 83f9d83a2bba930aecce42b6584844f16b923acd Mon Sep 17 00:00:00 2001 From: Tom Lankhorst Date: Mon, 26 Mar 2018 15:59:19 +0200 Subject: [PATCH 2/6] Track abstract resolve stack and inhibit resolving callback when is already on this stack --- src/Illuminate/Container/Container.php | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index b75e734b30d1..331a889ec758 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -82,6 +82,13 @@ class Container implements ArrayAccess, ContainerContract */ protected $buildStack = []; + /** + * The stack of de-aliased abstractions currently being resolved. + * + * @var array + */ + protected $resolveStack = []; + /** * The parameter override stack. * @@ -624,6 +631,8 @@ protected function resolve($abstract, $parameters = []) { $abstract = $this->getAlias($abstract); + $this->resolveStack[] = $abstract; + $needsContextualBuild = ! empty($parameters) || ! is_null( $this->getContextualConcrete($abstract) ); @@ -670,6 +679,7 @@ protected function resolve($abstract, $parameters = []) $this->resolved[$abstract] = true; array_pop($this->with); + array_pop($this->resolveStack); return $object; } @@ -1036,7 +1046,7 @@ protected function getCallbacksForType($abstract, $object, array $callbacksPerTy $results = []; foreach ($callbacksPerType as $type => $callbacks) { - if ($type === $abstract || $object instanceof $type) { + if ($type === $abstract || ( $object instanceof $type && !in_array( $type, $this->resolveStack ) ) ) { $results = array_merge($results, $callbacks); } } From 36a8f53e75ae7c8b23c5373d76c7434070abbcf0 Mon Sep 17 00:00:00 2001 From: Tom Lankhorst Date: Mon, 26 Mar 2018 16:07:42 +0200 Subject: [PATCH 3/6] StyleCI --- src/Illuminate/Container/Container.php | 2 +- tests/Container/ContainerTest.php | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index 331a889ec758..e28accffd284 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -1046,7 +1046,7 @@ protected function getCallbacksForType($abstract, $object, array $callbacksPerTy $results = []; foreach ($callbacksPerType as $type => $callbacks) { - if ($type === $abstract || ( $object instanceof $type && !in_array( $type, $this->resolveStack ) ) ) { + if ($type === $abstract || ($object instanceof $type && ! in_array($type, $this->resolveStack))) { $results = array_merge($results, $callbacks); } } diff --git a/tests/Container/ContainerTest.php b/tests/Container/ContainerTest.php index b9539254d6b8..ef9d3da37d7f 100755 --- a/tests/Container/ContainerTest.php +++ b/tests/Container/ContainerTest.php @@ -384,17 +384,17 @@ public function testResolvingCallbacksAreCalledOnceForImplementations() $resolving_invocations = 0; $after_resolving_invocations = 0; - $container->resolving( IContainerContractStub::class, function( ) use ( &$resolving_invocations ) { + $container->resolving(IContainerContractStub::class, function () use (&$resolving_invocations) { $resolving_invocations++; - } ); - $container->afterResolving( IContainerContractStub::class, function( ) use ( &$after_resolving_invocations ) { + }); + $container->afterResolving(IContainerContractStub::class, function () use (&$after_resolving_invocations) { $after_resolving_invocations++; - } ); - $container->bind(IContainerContractStub::class, ContainerImplementationStub::class ); - $container->make( IContainerContractStub::class ); + }); + $container->bind(IContainerContractStub::class, ContainerImplementationStub::class); + $container->make(IContainerContractStub::class); - $this->assertEquals( 1, $resolving_invocations ); - $this->assertEquals( 1, $after_resolving_invocations ); + $this->assertEquals(1, $resolving_invocations); + $this->assertEquals(1, $after_resolving_invocations); } public function testUnsetRemoveBoundInstances() From 24c77c03c4c730bac099e4f65aba5937b242070c Mon Sep 17 00:00:00 2001 From: Tom Lankhorst Date: Mon, 26 Mar 2018 21:19:46 +0200 Subject: [PATCH 4/6] Missing array_pop --- src/Illuminate/Container/Container.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index e28accffd284..cf3a5f51c637 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -641,6 +641,8 @@ protected function resolve($abstract, $parameters = []) // just return an existing instance instead of instantiating new instances // so the developer can keep using the same objects instance every time. if (isset($this->instances[$abstract]) && ! $needsContextualBuild) { + array_pop($this->resolveStack); + return $this->instances[$abstract]; } From 7f8628e9bcac7dcce4980e347e231f264eea3c0d Mon Sep 17 00:00:00 2001 From: Tom Lankhorst Date: Mon, 26 Mar 2018 23:13:00 +0200 Subject: [PATCH 5/6] Check if resolve is pending when firing callbacks --- src/Illuminate/Container/Container.php | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/src/Illuminate/Container/Container.php b/src/Illuminate/Container/Container.php index cf3a5f51c637..03fbd3979611 100755 --- a/src/Illuminate/Container/Container.php +++ b/src/Illuminate/Container/Container.php @@ -1048,7 +1048,7 @@ protected function getCallbacksForType($abstract, $object, array $callbacksPerTy $results = []; foreach ($callbacksPerType as $type => $callbacks) { - if ($type === $abstract || ($object instanceof $type && ! in_array($type, $this->resolveStack))) { + if (! $this->pendingInResolveStack($object) && ($type === $abstract || $object instanceof $type)) { $results = array_merge($results, $callbacks); } } @@ -1070,6 +1070,24 @@ protected function fireCallbackArray($object, array $callbacks) } } + /** + * Check if object or a generalisation is pending in the resolve stack. + * + * @param object $object + * + * @return bool + */ + protected function pendingInResolveStack($object) + { + foreach ($this->resolveStack as $resolving) { + if ($resolving !== end($this->resolveStack) && $object instanceof $resolving) { + return true; + } + } + + return false; + } + /** * Get the container's bindings. * From eea9a56ca95045a0e609c61ac39160be9bf7a2b0 Mon Sep 17 00:00:00 2001 From: Tom Lankhorst Date: Mon, 26 Mar 2018 23:28:16 +0200 Subject: [PATCH 6/6] More extensive test --- tests/Container/ContainerTest.php | 90 ++++++++++++++++++++++++++++--- 1 file changed, 82 insertions(+), 8 deletions(-) diff --git a/tests/Container/ContainerTest.php b/tests/Container/ContainerTest.php index ef9d3da37d7f..5a2d398a0b3d 100755 --- a/tests/Container/ContainerTest.php +++ b/tests/Container/ContainerTest.php @@ -381,20 +381,59 @@ public function testResolvingCallbacksAreCalledForType() public function testResolvingCallbacksAreCalledOnceForImplementations() { $container = new Container; - $resolving_invocations = 0; - $after_resolving_invocations = 0; + $resolving_contract_invocations = 0; + $after_resolving_contract_invocations = 0; + $resolving_implementation_invocations = 0; + $after_resolving_implementation_invocations = 0; - $container->resolving(IContainerContractStub::class, function () use (&$resolving_invocations) { - $resolving_invocations++; + $container->resolving(IContainerContractStub::class, function () use (&$resolving_contract_invocations) { + $resolving_contract_invocations++; }); - $container->afterResolving(IContainerContractStub::class, function () use (&$after_resolving_invocations) { - $after_resolving_invocations++; + $container->afterResolving(IContainerContractStub::class, function () use (&$after_resolving_contract_invocations) { + $after_resolving_contract_invocations++; + }); + $container->resolving(ContainerImplementationStub::class, function () use (&$resolving_implementation_invocations) { + $resolving_implementation_invocations++; + }); + $container->afterResolving(ContainerImplementationStub::class, function () use (&$after_resolving_implementation_invocations) { + $after_resolving_implementation_invocations++; }); $container->bind(IContainerContractStub::class, ContainerImplementationStub::class); $container->make(IContainerContractStub::class); - $this->assertEquals(1, $resolving_invocations); - $this->assertEquals(1, $after_resolving_invocations); + $this->assertEquals(1, $resolving_contract_invocations); + $this->assertEquals(1, $after_resolving_contract_invocations); + $this->assertEquals(1, $resolving_implementation_invocations); + $this->assertEquals(1, $after_resolving_implementation_invocations); + } + + public function testResolvingCallbacksAreCalledForNestedDependencies() + { + $container = new Container; + $resolving_dependency_interface_invocations = 0; + $resolving_dependency_implementation_invocations = 0; + $resolving_dependent_invocations = 0; + + $container->bind(IContainerContractStub::class, ContainerImplementationStub::class); + + $container->resolving(IContainerContractStub::class, function () use (&$resolving_dependency_interface_invocations) { + $resolving_dependency_interface_invocations++; + }); + + $container->resolving(ContainerImplementationStub::class, function () use (&$resolving_dependency_implementation_invocations) { + $resolving_dependency_implementation_invocations++; + }); + + $container->resolving(ContainerNestedDependentStubTwo::class, function () use (&$resolving_dependent_invocations) { + $resolving_dependent_invocations++; + }); + + $container->make(ContainerNestedDependentStubTwo::class); + $container->make(ContainerNestedDependentStubTwo::class); + + $this->assertEquals(4, $resolving_dependency_interface_invocations); + $this->assertEquals(4, $resolving_dependency_implementation_invocations); + $this->assertEquals(2, $resolving_dependent_invocations); } public function testUnsetRemoveBoundInstances() @@ -937,6 +976,19 @@ public function testResolvingCallbacksShouldBeFiredWhenCalledWithAliases() $this->assertEquals('taylor', $instance->name); } + public function testInterfaceResolvingCallbacksShouldBeFiredWhenCalledWithAliases() + { + $container = new Container; + $container->alias(IContainerContractStub::class, 'foo'); + $container->resolving(IContainerContractStub::class, function ($object) { + return $object->name = 'taylor'; + }); + $container->bind('foo', ContainerImplementationStub::class); + $instance = $container->make('foo'); + + $this->assertEquals('taylor', $instance->name); + } + public function testMakeWithMethodIsAnAliasForMakeMethod() { $mock = $this->getMockBuilder(Container::class) @@ -1089,6 +1141,18 @@ public function __construct(IContainerContractStub $impl) } } +class ContainerDependentStubTwo +{ + public $implA; + public $implB; + + public function __construct(IContainerContractStub $implA, IContainerContractStub $implB) + { + $this->implA = $implA; + $this->implB = $implB; + } +} + class ContainerNestedDependentStub { public $inner; @@ -1099,6 +1163,16 @@ public function __construct(ContainerDependentStub $inner) } } +class ContainerNestedDependentStubTwo +{ + public $inner; + + public function __construct(ContainerDependentStubTwo $inner) + { + $this->inner = $inner; + } +} + class ContainerDefaultValueStub { public $stub;