Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[8.x] Give a more meaningul message when route parameters are missing #35706

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/Illuminate/Routing/Exceptions/UrlGenerationException.php
Expand Up @@ -3,17 +3,35 @@
namespace Illuminate\Routing\Exceptions;

use Exception;
use Illuminate\Routing\Route;
use Illuminate\Support\Str;

class UrlGenerationException extends Exception
{
/**
* Create a new exception for missing route parameters.
*
* @param \Illuminate\Routing\Route $route
* @param array $parameters
* @return static
*/
public static function forMissingParameters($route)
public static function forMissingParameters(Route $route, array $parameters = [])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll have to be aware that this is technically a breaking change for people extending this exception and method.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could just re-target this PR at master. It's not long till another release anyway (indeed, it never really is, with the 6 month release cycle).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm happy with whatever. :)

{
return new static("Missing required parameters for [Route: {$route->getName()}] [URI: {$route->uri()}].");
$parameterLabel = Str::plural('parameter', count($parameters));

$message = sprintf(
'Missing required %s for [Route: %s] [URI: %s]',
$parameterLabel,
$route->getName(),
$route->uri()
);

if (count($parameters) > 0) {
$message .= sprintf(' [Missing %s: %s]', $parameterLabel, implode(', ', $parameters));
}

$message .= '.';

return new static($message);
}
}
4 changes: 2 additions & 2 deletions src/Illuminate/Routing/RouteUrlGenerator.php
Expand Up @@ -87,8 +87,8 @@ public function to($route, $parameters = [], $absolute = false)
$route
), $parameters);

if (preg_match('/\{.*?\}/', $uri)) {
throw UrlGenerationException::forMissingParameters($route);
if (preg_match_all('/{(.*?)}/', $uri, $matchedMissingParameters)) {
throw UrlGenerationException::forMissingParameters($route, $matchedMissingParameters[1]);
}

// Once we have ensured that there are no missing parameters in the URI we will encode
Expand Down
2 changes: 1 addition & 1 deletion tests/Routing/RoutingRouteTest.php
Expand Up @@ -1831,7 +1831,7 @@ public function testRouteRedirectStripsMissingStartingForwardSlash()
public function testRouteRedirectExceptionWhenMissingExpectedParameters()
{
$this->expectException(UrlGenerationException::class);
$this->expectExceptionMessage('Missing required parameters for [Route: laravel_route_redirect_destination] [URI: users/{user}].');
$this->expectExceptionMessage('Missing required parameter for [Route: laravel_route_redirect_destination] [URI: users/{user}] [Missing parameter: user].');

$container = new Container;
$router = new Router(new Dispatcher, $container);
Expand Down
55 changes: 55 additions & 0 deletions tests/Routing/RoutingUrlGeneratorTest.php
Expand Up @@ -550,6 +550,61 @@ public function testUrlGenerationForControllersRequiresPassingOfRequiredParamete
$this->assertSame('http://www.foo.com:8080/foo?test=123', $url->route('foo', $parameters));
}

public function provideParametersAndExpectedMeaningfulExceptionMessages()
{
return [
'Missing parameters "one", "two" and "three"' => [
[],
'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: one, two, three].',
],
'Missing parameters "two" and "three"' => [
['one' => '123'],
'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: two, three].',
],
'Missing parameters "one" and "three"' => [
['two' => '123'],
'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: one, three].',
],
'Missing parameters "one" and "two"' => [
['three' => '123'],
'Missing required parameters for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameters: one, two].',
],
'Missing parameter "three"' => [
['one' => '123', 'two' => '123'],
'Missing required parameter for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameter: three].',
],
'Missing parameter "two"' => [
['one' => '123', 'three' => '123'],
'Missing required parameter for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameter: two].',
],
'Missing parameter "one"' => [
['two' => '123', 'three' => '123'],
'Missing required parameter for [Route: foo] [URI: foo/{one}/{two}/{three}/{four?}] [Missing parameter: one].',
],
];
}

/**
* @dataProvider provideParametersAndExpectedMeaningfulExceptionMessages
*/
public function testUrlGenerationThrowsExceptionForMissingParametersWithMeaningfulMessage($parameters, $expectedMeaningfulExceptionMessage)
{
$this->expectException(UrlGenerationException::class);
$this->expectExceptionMessage($expectedMeaningfulExceptionMessage);

$url = new UrlGenerator(
$routes = new RouteCollection,
Request::create('http://www.foo.com:8080/')
);

$route = new Route(['GET'], 'foo/{one}/{two}/{three}/{four?}', ['as' => 'foo', function () {
//
}]);
$routes->add($route);

$url->route('foo', $parameters);
}

public function testForceRootUrl()
{
$url = new UrlGenerator(
Expand Down