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

Multiple url for single controller #34

Closed
sandline opened this issue Feb 1, 2015 · 5 comments
Closed

Multiple url for single controller #34

sandline opened this issue Feb 1, 2015 · 5 comments

Comments

@sandline
Copy link

sandline commented Feb 1, 2015

Hi,

I would like to use the same controller for multiple urls (because I show exactly the same data, but just in different languages) and I've tried this code:

    $r->addRoute('GET', '/{lang:(it|ch)}/',       'Home');
    $r->addRoute('GET', '/{lang:(it|ch)}/blog/',  'Blog');
    $r->addRoute('GET', '/sitemap/',       'Sitemap');

but this does not work. I get this error:

Notice: Undefined offset: 4 in /data/www/www.mysite.lan/lib/FastRoute/Dispatcher/GroupCountBased.php on line 16

Call Stack:
    0.0004     273928   1. {main}() /data/www/www.mysite.lan/index.php:0
    0.0257    1815680   2. FastRoute\Dispatcher\RegexBasedAbstract->dispatch() /data/www/www.mysite.lan/index.php:46
    0.0257    1815832   3. FastRoute\Dispatcher\GroupCountBased->dispatchVariableRoute() /data/www/www.mysite.lan/lib/FastRoute/Dispatcher/RegexBasedAbstract.php:20

Warning: Invalid argument supplied for foreach() in /data/www/www.mysite.lan/lib/FastRoute/Dispatcher/GroupCountBased.php on line 20

Call Stack:
    0.0004     273928   1. {main}() /data/www/www.mysite.lan/index.php:0
    0.0257    1815680   2. FastRoute\Dispatcher\RegexBasedAbstract->dispatch() /data/www/www.mysite.lan/index.php:46
    0.0257    1815832   3. FastRoute\Dispatcher\GroupCountBased->dispatchVariableRoute() /data/www/www.mysite.lan/lib/FastRoute/Dispatcher/RegexBasedAbstract.php:20

and with

$r = $d->dispatch(Core\Site::getRequestMethod(), Core\Site::getRequestURL());

$r[0] is empty, so the application does not know what is the controller to use.

I've tried to dump, in GroupCountBased, the route data inside the foreach:

        foreach ($routeData as $data) {
            var_dump($data);
            if (!preg_match($data['regex'], $uri, $matches)) {
                continue;
            }

just to try to understand the problem, and I've got:

array(2) {
  'regex' =>
  string(38) "~^(?|/((it|ch))/|/((it|ch))/blog/())$~"
  [...]

so I think that the problem is related to the fact that routes is "collected" and the use of | (pipe) between multiple urls broke the original regexs.

I've workarounded it redefining urls:

    $r->addRoute('GET', '/it/',       'HomeIT');
    $r->addRoute('GET', '/ch/',       'HomeCH');
    $r->addRoute('GET', '/it/blog/',  'BlogIT');
    $r->addRoute('GET', '/ch/blog/',  'BlogCH');

but this does not satisfy me. There is a way to achieve the use of variable url parts for the same controller and at the same time to obtain from the dispatcher the first part of url (it or ch) as a variable?

Consider that I can't simply do

    $r->addRoute('GET', '/{lang:.*}/',       'Home');

because this will also route the /sitemap/ url (and also any other url) and I want to avoid to serve everything with the same controller.

Thank you for reading,
Marco

@rdlowrey
Copy link
Contributor

rdlowrey commented Feb 1, 2015

Hi Marco,

The problem here is your regex pattern is invalid. This won't actually group the different language options into a single captured element:

$r->addRoute('GET', '/{lang:(it|ch)}/blog/',  'Blog');

If you want to actually group multiple options for a single variable your regex needs to define the languages as a non-capturing group like this:

$r->addRoute('GET', '/{lang:(?:it|ch)}/blog/',  'Blog');

The wrapping (?: ) characters mean "group these different options together but don't actually capture them." You need the non-capture group because the routing process performs its own capture to pull the lang variable out of the pattern. Otherwise you'll break the overall regex.

@sandline
Copy link
Author

sandline commented Feb 1, 2015

Hi,

thank you very much - that's awesome. :D

Regards,
Marco

@sandline sandline closed this as completed Feb 1, 2015
@nikic
Copy link
Owner

nikic commented Feb 1, 2015

I think it would be good to check that the regex doesn't contain capturing groups. Many people aren't familiar with things like ( vs (?:, so it's not obvious. Could check this with code similar to: https://github.com/nikic/Phlexy/blob/master/lib/Phlexy/LexerDataGenerator.php#L30

Btw, it would be enough to write '/{lang:it|ch}/blog/' here :)

@nikic nikic reopened this Feb 1, 2015
@sandline
Copy link
Author

sandline commented Feb 2, 2015

This will make it much simplier, but also may decrease a little bit the performances?
I think that an example for this case in the readme.md should be enough.

@nikic
Copy link
Owner

nikic commented Feb 11, 2015

I've added a note about capturing groups to the readme: d651b35

@nikic nikic closed this as completed Feb 11, 2015
nikic added a commit that referenced this issue Apr 4, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants