From a30619310dd739c22f561579c3de07f33d109612 Mon Sep 17 00:00:00 2001 From: Jamie Thorpe Date: Sat, 14 Dec 2024 17:35:25 -0500 Subject: [PATCH] [11.x] fix: allows injection using multiple interfaces with the same concrete implementation (#53275) * fix: allows injection of multiple interfaces with the same implementation * fix: ensure interfaces are only resolved once * fix: allows injection using multiple interfaces with the same concrete implementation * wip: working but ugly * test: ensure all tests pass * chore: fix code style issues * chore: fix code style issues * remove forgotten dd * wip * test: add test to ensure multiple interfaces with the same implementation can be injected in controllers * chore: remove unused imports * chore: adhere to style guidelines * formatting * formatting * formatting * formatting * formatting --------- Co-authored-by: Taylor Otwell --- .../Routing/ResolvesRouteDependencies.php | 64 +++++++++++++++++-- .../Routing/RouteDependencyInjectionTest.php | 52 +++++++++++++++ 2 files changed, 111 insertions(+), 5 deletions(-) create mode 100644 tests/Routing/RouteDependencyInjectionTest.php diff --git a/src/Illuminate/Routing/ResolvesRouteDependencies.php b/src/Illuminate/Routing/ResolvesRouteDependencies.php index bd3139fc769..b1eac11c6b1 100644 --- a/src/Illuminate/Routing/ResolvesRouteDependencies.php +++ b/src/Illuminate/Routing/ResolvesRouteDependencies.php @@ -3,9 +3,11 @@ namespace Illuminate\Routing; use Illuminate\Container\Util; +use Illuminate\Contracts\Container\BindingResolutionException; use Illuminate\Support\Arr; use Illuminate\Support\Reflector; use ReflectionClass; +use ReflectionException; use ReflectionFunctionAbstract; use ReflectionMethod; use ReflectionParameter; @@ -47,15 +49,24 @@ public function resolveMethodDependencies(array $parameters, ReflectionFunctionA $skippableValue = new stdClass; + $resolvedInterfaces = []; + foreach ($reflector->getParameters() as $key => $parameter) { - $instance = $this->transformDependency($parameter, $parameters, $skippableValue); + $className = Reflector::getParameterClassName($parameter); + + $instance = $this->transformDependency($parameter, $parameters, $className, $skippableValue, $resolvedInterfaces); + + if ($instance !== $skippableValue && + ! $this->alreadyInResolvedInterfaces($className, $resolvedInterfaces)) { + $resolvedInterfaces[] = $className; + } if ($instance !== $skippableValue) { $instanceCount++; $this->spliceIntoParameters($parameters, $key, $instance); } elseif (! isset($values[$key - $instanceCount]) && - $parameter->isDefaultValueAvailable()) { + $parameter->isDefaultValueAvailable()) { $this->spliceIntoParameters($parameters, $key, $parameter->getDefaultValue()); } @@ -68,18 +79,27 @@ public function resolveMethodDependencies(array $parameters, ReflectionFunctionA /** * Attempt to transform the given parameter into a class instance. * - * @param \ReflectionParameter $parameter + * @param ReflectionParameter $parameter * @param array $parameters + * @param string $className * @param object $skippableValue + * @param $resolvedInterfaces * @return mixed + * + * @throws BindingResolutionException + * @throws ReflectionException */ - protected function transformDependency(ReflectionParameter $parameter, $parameters, $skippableValue) + protected function transformDependency(ReflectionParameter $parameter, $parameters, $className, object $skippableValue, $resolvedInterfaces) { if ($attribute = Util::getContextualAttributeFromDependency($parameter)) { return $this->container->resolveFromAttribute($attribute); } - $className = Reflector::getParameterClassName($parameter); + if ($this->isSimilarConcreteToExistingParameterButDifferentInterface( + $className, $parameters, $resolvedInterfaces + )) { + return $this->container->make($className); + } // If the parameter has a type-hinted class, we will check to see if it is already in // the list of parameters. If it is we will just skip it as it is probably a model @@ -95,6 +115,24 @@ protected function transformDependency(ReflectionParameter $parameter, $paramete return $skippableValue; } + /** + * Determines if an instance of the given class is already in the parameters, but the route is type-hinting another interface that hasn't yet been resolved. + * + * @param string $className + * @param array $parameters + * @param array $resolvedInterfaces + * @return bool + */ + protected function isSimilarConcreteToExistingParameterButDifferentInterface($className, array $parameters, array $resolvedInterfaces) + { + // See: https://github.com/laravel/framework/pull/53275 + return $className && + $this->alreadyInParameters($className, $parameters) && + interface_exists($className) && + ! $this->alreadyInResolvedInterfaces($className, $resolvedInterfaces) && + (new ReflectionClass($className))->isInterface(); + } + /** * Determine if an object of the given class is in a list of parameters. * @@ -107,6 +145,22 @@ protected function alreadyInParameters($class, array $parameters) return ! is_null(Arr::first($parameters, fn ($value) => $value instanceof $class)); } + /** + * Determine if the given class name is already in the list of resolved interfaces. + * + * @param string|null $class + * @param array $resolvedInterfaces + * @return bool + */ + protected function alreadyInResolvedInterfaces($class, array $resolvedInterfaces) + { + if (! is_null($class)) { + return in_array($class, $resolvedInterfaces); + } + + return false; + } + /** * Splice the given value into the parameter list. * diff --git a/tests/Routing/RouteDependencyInjectionTest.php b/tests/Routing/RouteDependencyInjectionTest.php new file mode 100644 index 00000000000..633fb2f1e1d --- /dev/null +++ b/tests/Routing/RouteDependencyInjectionTest.php @@ -0,0 +1,52 @@ +instance(Registrar::class, $router); + + $container->bind(TestDependencyInterfaceA::class, TestDependencyImplementation::class); + $container->bind(TestDependencyInterfaceB::class, TestDependencyImplementation::class); + + $controller = new TestDependencyController(); + $result = $controller->index( + $container->make(TestDependencyInterfaceA::class), + $container->make(TestDependencyInterfaceB::class) + ); + + $this->assertInstanceOf(TestDependencyImplementation::class, $result[0]); + $this->assertInstanceOf(TestDependencyImplementation::class, $result[1]); + } +} + +interface TestDependencyInterfaceA +{ +} + +interface TestDependencyInterfaceB +{ +} + +class TestDependencyImplementation implements TestDependencyInterfaceA, TestDependencyInterfaceB +{ +} + +class TestDependencyController extends Controller +{ + public function index(TestDependencyInterfaceA $a, TestDependencyInterfaceB $b) + { + return [$a, $b]; + } +}