diff --git a/packages/container/src/DependencyChain.php b/packages/container/src/DependencyChain.php index 210cfb4c4..542f46ecd 100644 --- a/packages/container/src/DependencyChain.php +++ b/packages/container/src/DependencyChain.php @@ -32,6 +32,19 @@ public function add(Reflector|Closure|string $dependency): self return $this; } + /** + * Removes the last added dependency from the active resolution chain. + * + * Because dependencies are resolved in nested order, the most recently + * added item is always the one that just finished. + */ + public function pop(): self + { + array_pop($this->dependencies); + + return $this; + } + public function first(): Dependency { return array_first($this->dependencies); diff --git a/packages/container/src/GenericContainer.php b/packages/container/src/GenericContainer.php index 583ed878e..8901e4900 100644 --- a/packages/container/src/GenericContainer.php +++ b/packages/container/src/GenericContainer.php @@ -207,13 +207,15 @@ public function config(object $config): self */ public function get(string $className, string|UnitEnum|null $tag = null, mixed ...$params): object { - $this->resolveChain(); - - $dependency = $this->resolve($className, $tag, ...$params); + $previousChain = $this->chain; - $this->stopChain(); + $this->resolveChain(); - return $dependency; + try { + return $this->resolve($className, $tag, ...$params); + } finally { + $this->stopChain($previousChain); + } } public function invoke(ClassReflector|MethodReflector|FunctionReflector|callable|string $method, mixed ...$params): mixed @@ -250,33 +252,43 @@ public function invoke(ClassReflector|MethodReflector|FunctionReflector|callable private function invokeClosure(Closure $closure, mixed ...$params): mixed { - $this->resolveChain(); + $previousChain = $this->chain; - $parameters = $this->autowireDependencies( - method: $reflector = new FunctionReflector($closure), - parameters: $params, - ); + $this->resolveChain(); - $this->stopChain(); + try { + $parameters = $this->autowireDependencies( + method: $reflector = new FunctionReflector($closure), + parameters: $params, + ); + } finally { + $this->stopChain($previousChain); + } return $reflector->invokeArgs($parameters); } private function invokeMethod(MethodReflector $method, mixed ...$params): mixed { - $this->resolveChain(); + $previousChain = $this->chain; - $object = $this->get($method->getDeclaringClass()->getName()); + $this->resolveChain(); - $parameters = $this->autowireDependencies($method, $params); + try { + $object = $this->get($method->getDeclaringClass()->getName()); - $this->stopChain(); + $parameters = $this->autowireDependencies($method, $params); + } finally { + $this->stopChain($previousChain); + } return $method->invokeArgs($object, $parameters); } private function invokeFunction(FunctionReflector|Closure $callback, mixed ...$params): mixed { + $previousChain = $this->chain; + $this->resolveChain(); $reflector = match (true) { @@ -284,9 +296,11 @@ private function invokeFunction(FunctionReflector|Closure $callback, mixed ...$p default => new ReflectionFunction($callback), }; - $parameters = $this->autowireDependencies($reflector, $params); - - $this->stopChain(); + try { + $parameters = $this->autowireDependencies($reflector, $params); + } finally { + $this->stopChain($previousChain); + } return $reflector->invokeArgs($parameters); } @@ -371,20 +385,23 @@ private function resolveDependency(string $className, string|UnitEnum|null $tag $dependencyName = $this->resolveTaggedName($className, $tag); - // Check if a resolved singleton is present + // Check if a resolved singleton is present. A singleton is only registered once it is fully + // constructed, so it can never be in flight and cannot take part in a cycle. if ($instance = $this->resolvedSingletons[$dependencyName] ?? null) { - $this->resolveChain()->add($class); - return $instance; } // Check if the class has been registered as a singleton. if ($singletonDefinition = $this->singletonDefinitions[$dependencyName] ?? null) { - $instance = $singletonDefinition instanceof Closure ? $singletonDefinition($this) : $singletonDefinition; + $this->resolveChain()->add($class); - $this->resolvedSingletons[$dependencyName] = $instance; + try { + $instance = $singletonDefinition instanceof Closure ? $singletonDefinition($this) : $singletonDefinition; + } finally { + $this->resolveChain()->pop(); + } - $this->resolveChain()->add($class); + $this->resolvedSingletons[$dependencyName] = $instance; return $instance; } @@ -393,7 +410,11 @@ private function resolveDependency(string $className, string|UnitEnum|null $tag if ($definition = $this->definitions[$dependencyName] ?? null) { $this->resolveChain()->add(new FunctionReflector($definition)); - return $definition($this); + try { + return $definition($this); + } finally { + $this->resolveChain()->pop(); + } } // Next we check if any of our default initializers can initialize this class. @@ -402,10 +423,14 @@ private function resolveDependency(string $className, string|UnitEnum|null $tag $this->resolveChain()->add($initializerClass); - $object = match (true) { - $initializer instanceof Initializer => $initializer->initialize($this->clone()), - $initializer instanceof DynamicInitializer => $initializer->initialize($class, $tag, $this->clone()), - }; + try { + $object = match (true) { + $initializer instanceof Initializer => $initializer->initialize($this->clone()), + $initializer instanceof DynamicInitializer => $initializer->initialize($class, $tag, $this->clone()), + }; + } finally { + $this->resolveChain()->pop(); + } $singleton = $initializerClass->getAttribute(Singleton::class) ?? $initializerClass->getMethod('initialize')->getAttribute(Singleton::class); @@ -543,12 +568,16 @@ private function autowireDependencies(MethodReflector|FunctionReflector $method, // Build the class by iterating through its // dependencies and resolving them. - foreach ($method->getParameters() as $parameter) { - $dependencies[] = $this->clone()->autowireDependency( - parameter: $parameter, - tag: $parameter->getAttribute(Tag::class)?->name, - providedValue: $parameters[$parameter->getName()] ?? null, - ); + try { + foreach ($method->getParameters() as $parameter) { + $dependencies[] = $this->clone()->autowireDependency( + parameter: $parameter, + tag: $parameter->getAttribute(Tag::class)?->name, + providedValue: $parameters[$parameter->getName()] ?? null, + ); + } + } finally { + $this->resolveChain()->pop(); } return $dependencies; @@ -679,9 +708,12 @@ private function resolveChain(): DependencyChain return $this->chain; } - private function stopChain(): void + /** + * Restores the parent resolution chain, or resets it to `null` at the top level. + */ + private function stopChain(?DependencyChain $previous = null): void { - $this->chain = null; + $this->chain = $previous; } public function __clone(): void diff --git a/packages/container/tests/Exceptions/CircularDependencyExceptionTest.php b/packages/container/tests/Exceptions/CircularDependencyExceptionTest.php index 4b436656d..1f0134192 100644 --- a/packages/container/tests/Exceptions/CircularDependencyExceptionTest.php +++ b/packages/container/tests/Exceptions/CircularDependencyExceptionTest.php @@ -10,6 +10,11 @@ use Tempest\Container\GenericContainer; use Tempest\Container\Tests\Fixtures\CircularA; use Tempest\Container\Tests\Fixtures\CircularZ; +use Tempest\Container\Tests\Fixtures\NestedResolutionA; +use Tempest\Container\Tests\Fixtures\NestedResolutionAInitializer; +use Tempest\Container\Tests\Fixtures\NestedResolutionBInitializer; +use Tempest\Container\Tests\Fixtures\ResolvedTwiceParent; +use Tempest\Container\Tests\Fixtures\ResolvedTwiceParentInitializer; /** * @internal @@ -74,4 +79,27 @@ public function circular_dependency_as_a_child_test(): void throw $circularDependencyException; } } + + #[Test] + public function circular_dependency_after_a_nested_resolution_test(): void + { + $this->expectException(CircularDependencyEncountered::class); + + $container = new GenericContainer(); + $container->addInitializer(NestedResolutionAInitializer::class); + $container->addInitializer(NestedResolutionBInitializer::class); + + $container->get(NestedResolutionA::class); + } + + #[Test] + public function the_same_dependency_resolved_twice_in_one_chain_is_not_circular_test(): void + { + $container = new GenericContainer(); + $container->addInitializer(ResolvedTwiceParentInitializer::class); + + $parent = $container->get(ResolvedTwiceParent::class); + + $this->assertInstanceOf(ResolvedTwiceParent::class, $parent); + } } diff --git a/packages/container/tests/Fixtures/NestedResolutionA.php b/packages/container/tests/Fixtures/NestedResolutionA.php new file mode 100644 index 000000000..10346777e --- /dev/null +++ b/packages/container/tests/Fixtures/NestedResolutionA.php @@ -0,0 +1,7 @@ +get(Unrelated::class); + $container->get(NestedResolutionB::class); + + return new NestedResolutionA(); + } +} diff --git a/packages/container/tests/Fixtures/NestedResolutionB.php b/packages/container/tests/Fixtures/NestedResolutionB.php new file mode 100644 index 000000000..fd0cfef61 --- /dev/null +++ b/packages/container/tests/Fixtures/NestedResolutionB.php @@ -0,0 +1,7 @@ +get(NestedResolutionA::class); + + return new NestedResolutionB(); + } +} diff --git a/packages/container/tests/Fixtures/ResolvedTwiceLeaf.php b/packages/container/tests/Fixtures/ResolvedTwiceLeaf.php new file mode 100644 index 000000000..dde4f83c6 --- /dev/null +++ b/packages/container/tests/Fixtures/ResolvedTwiceLeaf.php @@ -0,0 +1,12 @@ +get(ResolvedTwiceLeaf::class), + $container->get(ResolvedTwiceLeaf::class), + ); + } +} diff --git a/packages/container/tests/Fixtures/Unrelated.php b/packages/container/tests/Fixtures/Unrelated.php new file mode 100644 index 000000000..480e0d224 --- /dev/null +++ b/packages/container/tests/Fixtures/Unrelated.php @@ -0,0 +1,7 @@ +