[5.5] Optional parameter binding (#17521)

* Resolve default value for optional parameter if available * Fail when bound model not found even with optional parameter
laravel · Feb 3, 2017 · 391d960 · 391d960
1 parent d0fab7e
commit 391d960
Show file tree

Hide file tree

Showing 3 changed files with 44 additions and 10 deletions.
diff --git a/src/Illuminate/Routing/ImplicitRouteBinding.php b/src/Illuminate/Routing/ImplicitRouteBinding.php
@@ -22,14 +22,12 @@ public static function resolveForRoute($container, $route)
 
             if (array_key_exists($parameter->name, $parameters) &&
                 ! $route->parameter($parameter->name) instanceof Model) {
-                $method = $parameter->isDefaultValueAvailable() ? 'first' : 'firstOrFail';
-
                 $model = $container->make($class->name);
 
                 $route->setParameter(
                     $parameter->name, $model->where(
                         $model->getRouteKeyName(), $parameters[$parameter->name]
-                    )->{$method}()
+                    )->firstOrFail()
                 );
             }
         }

diff --git a/src/Illuminate/Routing/RouteDependencyResolverTrait.php b/src/Illuminate/Routing/RouteDependencyResolverTrait.php
@@ -68,7 +68,9 @@ protected function transformDependency(ReflectionParameter $parameter, $paramete
         // the list of parameters. If it is we will just skip it as it is probably a model
         // binding and we do not want to mess with those; otherwise, we resolve it here.
         if ($class && ! $this->alreadyInParameters($class->name, $parameters)) {
-            return $this->container->make($class->name);
+            return $parameter->isDefaultValueAvailable()
+                ? $parameter->getDefaultValue()
+                : $this->container->make($class->name);
         }
     }
 

diff --git a/tests/Routing/RoutingRouteTest.php b/tests/Routing/RoutingRouteTest.php
@@ -1151,7 +1151,7 @@ public function testImplicitBindings()
         $this->assertEquals('taylor', $router->dispatch(Request::create('foo/taylor', 'GET'))->getContent());
     }
 
-    public function testImplicitBindingsWithOptionalParameter()
+    public function testImplicitBindingsWithOptionalParameterWithExistingKeyInUri()
     {
         $phpunit = $this;
         $router = $this->getRouter();
@@ -1164,13 +1164,35 @@ public function testImplicitBindingsWithOptionalParameter()
             },
         ]);
         $this->assertEquals('taylor', $router->dispatch(Request::create('foo/taylor', 'GET'))->getContent());
+    }
 
+    public function testImplicitBindingsWithOptionalParameterWithNoKeyInUri()
+    {
+        $phpunit = $this;
         $router = $this->getRouter();
-        $router->get('bar/{foo?}', function (RoutingTestUserModel $foo = null) use ($phpunit) {
-            $phpunit->assertInstanceOf(RoutingTestUserModel::class, $foo);
-            $phpunit->assertNull($foo->value);
-        });
-        $router->dispatch(Request::create('bar', 'GET'))->getContent();
+        $router->get('foo/{bar?}', [
+            'middleware' => SubstituteBindings::class,
+            'uses' => function (RoutingTestUserModel $bar = null) use ($phpunit) {
+                $phpunit->assertNull($bar);
+            },
+        ]);
+        $router->dispatch(Request::create('foo', 'GET'))->getContent();
+    }
+
+    /**
+     * @expectedException \Illuminate\Database\Eloquent\ModelNotFoundException
+     */
+    public function testImplicitBindingsWithOptionalParameterWithNonExistingKeyInUri()
+    {
+        $phpunit = $this;
+        $router = $this->getRouter();
+        $router->get('foo/{bar?}', [
+            'middleware' => SubstituteBindings::class,
+            'uses' => function (RoutingTestNonExistingUserModel $bar = null) use ($phpunit) {
+                $phpunit->fail('ModelNotFoundException was expected.');
+            },
+        ]);
+        $router->dispatch(Request::create('foo/nonexisting', 'GET'))->getContent();
     }
 
     public function testImplicitBindingThroughIOC()
@@ -1431,6 +1453,18 @@ class RoutingTestExtendedUserModel extends RoutingTestUserModel
 {
 }
 
+class RoutingTestNonExistingUserModel extends RoutingTestUserModel
+{
+    public function first()
+    {
+    }
+
+    public function firstOrFail()
+    {
+        throw new \Illuminate\Database\Eloquent\ModelNotFoundException();
+    }
+}
+
 class ActionStub
 {
     public function __invoke()