Skip to content

Commit 27acc09

Browse files
stancltaylorotwell
andauthored
[12.x] Various URL generation bugfixes (#54811)
* Add regression test: passed parameters have precedence over defaults * Add RouteUrlGenerator::formatParameters() This method turns a list of passed parameters into a list of *named* parameters (where possible) reducing ambiguity and making other code work more accurately with positional route parameters, especially when URL::defaults() is involved. This commit also resolves a known bug -- removing a 'mark skipped' mark for a test. * Add test: complex route generation with defaults and binding fields * Add support for URL::defaults(['model:slug' => 'foo']) * Code style: Apply StyleCI diff * Cleanup, additional test cases * formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
1 parent c45c361 commit 27acc09

File tree

3 files changed

+967
-20
lines changed

3 files changed

+967
-20
lines changed

src/Illuminate/Routing/RouteUrlGenerator.php

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,11 @@
22

33
namespace Illuminate\Routing;
44

5+
use BackedEnum;
6+
use Illuminate\Contracts\Routing\UrlRoutable;
57
use Illuminate\Routing\Exceptions\UrlGenerationException;
68
use Illuminate\Support\Arr;
9+
use Illuminate\Support\Collection;
710

811
class RouteUrlGenerator
912
{
@@ -74,6 +77,8 @@ public function __construct($url, $request)
7477
*/
7578
public function to($route, $parameters = [], $absolute = false)
7679
{
80+
$parameters = $this->formatParameters($route, $parameters);
81+
7782
$domain = $this->getRouteDomain($route, $parameters);
7883

7984
// First we will construct the entire URI including the root and query string. Once it
@@ -167,6 +172,95 @@ protected function addPortToDomain($domain)
167172
: $domain.':'.$port;
168173
}
169174

175+
/**
176+
* Format the array of route parameters.
177+
*
178+
* @param \Illuminate\Routing\Route $route
179+
* @param mixed $parameters
180+
* @return array
181+
*/
182+
protected function formatParameters(Route $route, $parameters)
183+
{
184+
$parameters = Arr::wrap($parameters);
185+
186+
$passedParameterCount = count($parameters);
187+
188+
$namedParameters = [];
189+
$namedQueryParameters = [];
190+
$routeParametersWithoutDefaultsOrNamedParameters = [];
191+
192+
$routeParameters = $route->parameterNames();
193+
194+
foreach ($routeParameters as $name) {
195+
if (isset($parameters[$name])) {
196+
// Named parameters don't need any special handling...
197+
$namedParameters[$name] = $parameters[$name];
198+
unset($parameters[$name]);
199+
200+
continue;
201+
} elseif (! isset($this->defaultParameters[$name])) {
202+
// No named parameter or default value, try to match to positional parameter below...
203+
array_push($routeParametersWithoutDefaultsOrNamedParameters, $name);
204+
}
205+
206+
$namedParameters[$name] = '';
207+
}
208+
209+
// Named parameters that don't have route parameters will be used for query string...
210+
foreach ($parameters as $key => $value) {
211+
if (is_string($key)) {
212+
$namedQueryParameters[$key] = $value;
213+
214+
unset($parameters[$key]);
215+
}
216+
}
217+
218+
// Match positional parameters to the route parameters that didn't have a value in order...
219+
if (count($parameters) == count($routeParametersWithoutDefaultsOrNamedParameters)) {
220+
foreach (array_reverse($routeParametersWithoutDefaultsOrNamedParameters) as $name) {
221+
if (count($parameters) === 0) {
222+
break;
223+
}
224+
225+
$namedParameters[$name] = array_pop($parameters);
226+
}
227+
}
228+
229+
// If there are extra parameters, just fill left to right... if not, fill right to left and try to use defaults...
230+
$extraParameters = $passedParameterCount > count($routeParameters);
231+
232+
foreach ($extraParameters ? $namedParameters : array_reverse($namedParameters) as $key => $value) {
233+
$bindingField = $route->bindingFieldFor($key);
234+
235+
$defaultParameterKey = $bindingField ? "$key:$bindingField" : $key;
236+
237+
if ($value !== '') {
238+
continue;
239+
} elseif (! empty($parameters)) {
240+
$namedParameters[$key] = $extraParameters ? array_shift($parameters) : array_pop($parameters);
241+
} elseif (isset($this->defaultParameters[$defaultParameterKey])) {
242+
$namedParameters[$key] = $this->defaultParameters[$defaultParameterKey];
243+
}
244+
}
245+
246+
// Any remaining values in $parameters are unnamed query string parameters...
247+
$parameters = array_merge($namedParameters, $namedQueryParameters, $parameters);
248+
249+
$parameters = Collection::wrap($parameters)->map(function ($value, $key) use ($route) {
250+
return $value instanceof UrlRoutable && $route->bindingFieldFor($key)
251+
? $value->{$route->bindingFieldFor($key)}
252+
: $value;
253+
})->all();
254+
255+
array_walk_recursive($parameters, function (&$item) {
256+
if ($item instanceof BackedEnum) {
257+
$item = $item->value;
258+
}
259+
});
260+
261+
return $this->url->formatParameters($parameters);
262+
}
263+
170264
/**
171265
* Replace the parameters on the root path.
172266
*

src/Illuminate/Routing/UrlGenerator.php

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -538,20 +538,8 @@ public function route($name, $parameters = [], $absolute = true)
538538
*/
539539
public function toRoute($route, $parameters, $absolute)
540540
{
541-
$parameters = Collection::wrap($parameters)->map(function ($value, $key) use ($route) {
542-
return $value instanceof UrlRoutable && $route->bindingFieldFor($key)
543-
? $value->{$route->bindingFieldFor($key)}
544-
: $value;
545-
})->all();
546-
547-
array_walk_recursive($parameters, function (&$item) {
548-
if ($item instanceof BackedEnum) {
549-
$item = $item->value;
550-
}
551-
});
552-
553541
return $this->routeUrl()->to(
554-
$route, $this->formatParameters($parameters), $absolute
542+
$route, $parameters, $absolute
555543
);
556544
}
557545

0 commit comments

Comments
 (0)