[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 <[email protected]>
laravel · Dec 14, 2024 · a306193 · a306193
1 parent 83c3855
commit a306193
Show file tree

Hide file tree

Showing 2 changed files with 111 additions and 5 deletions.
diff --git 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
@@ -0,0 +1,52 @@
+<?php
+
+namespace Illuminate\Tests\Routing;
+
+use Illuminate\Container\Container;
+use Illuminate\Contracts\Routing\Registrar;
+use Illuminate\Events\Dispatcher;
+use Illuminate\Routing\Controller;
+use Illuminate\Routing\Router;
+use PHPUnit\Framework\TestCase;
+
+class RouteDependencyInjectionTest extends TestCase
+{
+    public function test_it_can_resolve_multiple_interfaces_with_the_same_implementation()
+    {
+        $container = new Container;
+        $router = new Router(new Dispatcher, $container);
+        $container->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];
+    }
+}