Quantifier in route regex
Closed this issue · 12 comments
Because Phroutes uses {
and }
in the matcher, you cannot use quantifiers in the regex for it, for example: {year:\d{4}}
to only match if 4 digits are found.
Yes you are right. Thanks! I'll try and add something in for this. I don't want to change the parameter delimiters, but I'll see if I can work it into the route parser.
Thanks. I'd love this feature. I use Phroute in my own framework, it's a great package thank you.
Looking at your shortcuts, it'd be nice if you could extend those to work like {year:i,4}
and that will be swapped to {year:\d{4}}
and {year:i,2,4}
would go to {year:\d{2,4}}
Know what I mean?
Yeah that would be very cool. A concern I have is that this may not be the correct place to validate at this kind of detail. If the route parameter doesn't match exactly, a generic HttpRouteNotFoundException
exception will be thrown, which offers no feedback to the requester to explain what was wrong with the request.
In my own routes, I rarely specify any paramter rules in the route and if I do I generally go no further than specifying whether or not the parameter should be an integer, eg. {user:i}
. Resource parameters that must be an integer ID seem to me to have an obvious and intuitive distinction from parameters that can be any string and it may be acceptable for the /user/s*a)!j£
route to not exist at all - It's not even remotely close to being a valid resource ID.
On the other hand, a {year}
or {month}
parameter could require one of a number of formats: 1990 vs 90
or july vs jul vs 07 vs 7
. I'm not sure I'd be happy with an API that returned a 404 for /posts/year/90/month/jul
instead of a message telling me to specify a year and month in a particular format /posts/year/YYYY/month/MM
etc.
Sorry for rambling on... :)
I'd be interested to hear your uses cases and how you use parameters - whether you agree with my thoughts or not?
I agree with you there, here's what made me realize this issue.
I have this method that is being called by the route /{year:i}/{month:i}?/{day:i}?
public function getArchive($year = null, $month = null, $day = null)
{
$date = Carbon::create($year, $month ?: 1, $day ?: 1, 0, 0, 0);
$start = $date->toDateTimeString();
if (is_null($day) && !is_null($month)) {
$end = $date->addMonth()->toDateTimeString();
}
elseif (is_null($day) && is_null($month)) {
$end = $date->addYear()->toDateTimeString();
} else {
$end = $date->addDay()->toDateTimeString();
}
$posts = Post::whereBetween('published_at', [$start, $end])->get();
return View::render('posts.php', compact('posts'));
}
So if someone requests /2014/08/07
it'll be fine. But if they request /20142/083/071
it'll cause Carbon to freak out because the digits passed aren't the right amount of digits.
Yeah I could put in checks in the controller, but that's what route regex quantifies are for. :)
I will certainly ensure you can add the regex quantifiers. I'm not sure yet if I will create shortcuts for them, but I will have a think about it. It might open up a slippery slope for shortcuts, which end up being as complicated as the original regex themselves :)
In this case, perhaps something like this would be more appropriate?
public function getArchive($year = null, $month = null, $day = null)
{
try{
$date = \Carbon\Carbon::create($year, $month ?: 1, $day ?: 1, 0, 0, 0);
}
catch(\InvalidArgumentException $e)
{
$error = "Please ensure the parameters are in the correct format YYYY/MM/DD";
return View::render('posts.php', compact('error'));
}
$start = $date->toDateTimeString();
if(!is_null($day))
{
$end = $date->addDay()->toDateTimeString();
}
elseif(!is_null($month))
{
$end = $date->addMonth()->toDateTimeString();
}
else{
$end = $date->addYear()->toDateTimeString();
}
$posts = Post::whereBetween('published_at', [$start, $end])->get();
return View::render('posts.php', compact('posts'));
}
Thanks for all the feedback! I'll leave this issue open until I have added support for quantifiers in the form \d{2,4}
Cheers
Yeah I'm gonna have to do some checks on the controller itself for now, but quantifiers would be good. Thanks man! Keep up the good work.
I've pushed a fix for this to master in this commit a5ccd30
I've bumped the minor version for this release so use "1.2.*" in your composer.json
Let me know how you get on!
Awesome! I'll try it out this week.
So I tested this out, and while this works {year:\d{4}}/{month:\d+}
This breaks {year:\d{4}}/{month:\d{2}}
Seems once I do it in 2 variables it goes wonky.
Whoops - sorry about that. Needed to make the match non-greedy. Fixed in commit: ca119ce RouteParser.php
Thanks! Really appreciate your help with that one.
Awesome it works! Appreciate the quick addition.