Optional parameter in url

[silentworks]

Slim doesn't support optional parameters as the underlying routing engine doesn't support them.

We need:

• Ability to specify optional parameters in the route path
• Ability to generate urls via urlFor for routes that have optional parameters.

This was originally reported as:

It would seem it currently isn't possible to add an optional paramater in the url if you are using urlFor to get the url via a named route.

$app->get('/test/{param1}[/{optional:[A-Za-z]}]')->setName('test');

$app->getContainer()['router']->urlFor('test', ['param1' => 'World']);

This will throw an error because urlFor method does not know how to handle optional regex. I saw another project using FastRoute and they handle it using this regex https://github.com/mrjgreen/phroute/blob/master/src/Phroute/RouteParser.php#L23-L29

[akrabat]

I don't think that FastRoute supports optional parameters.

This doesn't work for me:

$app->get('/hello/{name}[/{optional}]', function ($req, $res, $args) {
    $name = $req->getAttribute('name', '== not set ==');
    $optional = $req->getAttribute('optional', '== not set ==');

    return $res->write("<p>Hello $name. Optional: $optional</p>");
});

Am I missing something?

[akrabat]

The only way I've found to do optional parameters is like this:

$app->get('/hello2/{name}{a:/?}{optional:.*}', function ($req, $res, $args) {
    $name = $req->getAttribute('name', '== not set ==');
    $optional = $req->getAttribute('optional', '== not set ==');

    return $res->write("<p>Hello $name. Optional: $optional</p>");
});

[silentworks]

Correct FastRoute does not support optional paramaters, although the syntax I provided is what @nikic provided in the comment section on his blog http://nikic.github.io/2014/02/18/Fast-request-routing-using-regular-expressions.html#comment-1256023948.

You example will not work with urlFor method on the router, which is where the problem is at for me.

[akrabat]

So you get $app->get('/test/{param1}[/{optional:[A-Za-z]}]')->setName('test'); to route to a callable?

[geggleto]

the optional URL problem is only for calling via urlFor correct? //cc @silentworks

[silentworks]

@geggleto correct, and its because the regex in urlFor is broken, it doesn't take optionals into consideration.

[akrabat]

I am genuinely confused. Why have urlFor work with path definitions that do not work?

[geggleto]

So to clarify @silentworks

//Given this route
$app->get('/test/{param1}[/{optional:[A-Za-z]}]');

//the request 
GET /test/1/abcdefg
//would work in Slim currently?

[silentworks]

@akrabat https://github.com/slimphp/Slim/blob/develop/Slim/Router.php#L184 has no clue how to deal with optionals, it will think that {optional:.*} is required although its not in the example you provided.

$app->get('/hello2/{name}{a:/?}{optional:.*}', function ($req, $res, $args) {
    $name = $req->getAttribute('name', '== not set ==');
    $optional = $req->getAttribute('optional', '== not set ==');

    return $res->write("<p>Hello $name. Optional: $optional</p>");
})->setName('hello');

The below example breaks

$app->getContainer()['router']->urlFor('hello', ['name' => 'World']);

[silentworks]

@geggleto correct but

//Given this route
$app->get('/test/{param1}[/{optional:[A-Za-z]}]');

// the request
GET /test/1
// would break in Slim currently.

[akrabat]

Yeah - fundamentally, Slim doesn't support optional parameters as it can't dispatch them.

[geggleto]

In v2 the router did support them...

[silentworks]

@akrabat Slim 2 does and FastRoute can support it if you add it in.

[geggleto]

I very much rely on the optional parameters functionality. It needs to be in v3.

[akrabat]

I clearly don't understand, so am happy to review a PR :)

[codeguy]

:+1: for PR that adds optional params functionality to FastRoute

[akrabat]

I agree that a clean way to do optional parameters is required. I think it's deeper than a urlFor() issue.

[geggleto]

I guess we are at a point where we need to decide if we want to extend FastRoute to include the capabilities we need or use another Router that already has those capabilities.

Given the whole, lets not reinvent the wheel. I prefer the latter over the former since Aura.Router has all the features we need as well as strategies for deployment to Production.

[codeguy]

FastRoute as is lets us create custom named tokens that are expanded into an appropriate regex. See League Route for example implementation:

https://github.com/thephpleague/route/blob/master/src/RouteCollection.php#L202

[silentworks]

@codeguy I don't think this solves the optional parameter issue, this just allows custom named tokens.

[codeguy]

hmm. okay. If necessary this can wait until 3.1.

[lalop]

I don't know how fastroute works, but maybe we can add multiple path to a route. Something like

$app->get(['/hello', '/hello/{name}'])->setName('hello-world');

Than for the urlFor we return the first one matching the maximum of the given parameters.

[geggleto]

This was a few hours ago, but I do believe FastRoute does not support optional parameters at all. urlFor is a pure Slim item, which spawned this whole fiasco with FastRoute.

[akrabat]

I've started looking at replacing FastRoute with Aura.Router.

Initial work here: https://github.com/slimphp/Slim/compare/develop...akrabat:aura-router?expand=1

Testbed that I'm using: https://gist.github.com/akrabat/66d04bb7a8be8082a53f

Things that I know work:

• Optional parameters
• Middleware attached to routes
• urlFor() with query parameters
• Route groups ($app->group()) with names and middleware

BC breaks with Slim 3 so far:

• ->setName() is replaced by $name as last parameter as Aura.Router requires the name up front. (Null is allowed though, so I made $name an optional last parameter to map(), get(), post(), etc.)

Notes:

• Names for route groups prepends with a full stop separator for use with urlFor(). i.e. a route call foo in a group called bar will be accessed as $url = $router->urlFor('bar.foo');
• Middleware for route groups is done as an array as the fourth parameter to group() because as we add each route within the group immediately, we can't use an ->add() construct.

[Melbournite]

Generally speaking, are there any downsides to using aura over FastRoute? Assuming there's minimal work required to switch them out... I know FastRoute was originally chosen due to its speed. At the same time, optional paramters is an absolute must IMO.

I can't really find any reliable real world bench marking between the two besides here: https://github.com/tyler-sommer/php-router-benchmark

[geggleto]

Currently in v3 the api for attaching middleware is fragmented. Route groups are handled internally in Slim and not actually in the router and that seems a bit weird when we are using 3rd party components. Using a more feature rich router would likely pull some of the legacy code out of Slim and unify the API :+1:

[akrabat]

I can't see any way around the middleware attachment issues for route groups. Even though Aura.Router has the concept of route groups, they are implemented the same way Slim does it by prefixing the path and then adding the individual routes. As a result, you don't have the concept of a group object after adding the route group on which you can then set some middleware.

This is why I've made Slim's add route group function signature as:

public function group($pattern, $callback, $name = null, $middleware = [])

I couldn't work out a way to do ->add() for this case. If we wanted consistency between adding route middleware and route group middleware, then we'd have to change the function signatures for adding routes. In Slim 2 we have variable arguments and the ones we don't know are assumed to be middleware. If we were to go back to defining middleware as a method parameter, then I would prefer an array as I've done for group, so you'd end up with:

$app->get($pattern, $callable, $name=null, $middleware=[]);
$app->map(array $methods, $pattern, $callable, $name=null, $middleware=[])

Obviously, we can't change the way we add middleware to the app itself, so $app->add() would still exist, regardless.

One final option. Don't allow middleware on route groups :)

[Melbournite]

Middleware on groups is more just a convenience thing I guess. I am still interested to see any performance degradation with Aura being used. I'll try do some tests tomorrow.

Is the general consensus that Aura should be switched in agreed?

[geggleto]

:-1: @akrabat There shalt be no mention of this removal of a feature cries

Yeah it is a huge convenience thing. Given that it makes it very easy to apply say an authentication layer to an entire slim application. In v3 I do not believe there is pass-thru anymore so that option is also gone as a mitigation. ;(

[jaapverloop]

+1 for changing all the function signatures for adding routes. As a result, it's not necessary to keep the route objects container aware, right?

Maybe we could introduce something like https://gist.github.com/jaapverloop/142b2857ee005e91068f for convenience.

[geggleto]

@akrabat looking at

public function group($pattern, $callback, $name = null, $middleware = []);

I can :+1: that signature, its not as clean as doing ->add() but at least I don't have to apply middleware to all the subsequent routes.

[nikic]

fyi FastRoute now supports trailing optional segments using the syntax /user/{id:\d+}[/{name}].

[akrabat]

That's great timing! Thanks @nikic :)

[geggleto]

Yeah!!! :100:

[codeguy]

Awesome news. Because I really like FastRoute.

[silentworks]

Thanks @nikic.

Now we just need to fix urlFor to work with the new syntax for optionals.

[silentworks]

Fixed.