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

Already on GitHub? Sign in to your account

Replace Router with Http router in url view helper #2384

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
5 participants
Contributor

juriansluiman commented Sep 18, 2012

The url helper assembles urls solely for an http environment. When
the current request is a CLI request, the injected router is a CLI
router. This causes troubles when in a CLI environment html templates
will be assembled with URLs, which can't be assembled because the
router is a CLI router (and has no awareness of any URLs).

Tests run (locally) still fine, so only this file is modified.

Jurian Sluiman Replace Router with Http router in url view helper
The url helper assembles urls solely for an http environment. When
the current request is a CLI request, the injected router is a CLI
router. This causes troubles when in a CLI environment html templates
will be assembled with URLs, which can't be assembled because the
router is a CLI router (and has no awareness of any URLs).

Tests run (locally) still fine, so only this file is modified.
4525cbe
Member

DASPRiD commented Sep 19, 2012

Hi Jurians, what about the controller url helper?

Contributor

juriansluiman commented Sep 19, 2012

@DASPRiD thought about that. Not sure how to handle (and if it's required?). Currently the router is pulled from the MvcEvent, but obviously that's the wrong router if you want the http router in the cli env. You can use the SM from the controller to pull, or inject the correct router. Thoughts?

Member

Thinkscape commented Sep 19, 2012

Have you tested that ?

Contributor

juriansluiman commented Sep 19, 2012

Uhm, no this was completely made up with random assumptions... why you think that? In http the url keeps working, in the cli the correct router is injected. I can assemble urls as that's the point of this PR.

@weierophinney weierophinney added a commit that referenced this pull request Sep 19, 2012

@weierophinney weierophinney Merge branch 'hotfix/2384' into develop
Forward-port #2384
c198073
Member

Thinkscape commented Sep 20, 2012

Just make sure other things work. AFAIR router factory (my version at
least) will provide you with the proper router, but it will persist it (as
per SM specification), meaning that you'll effectively disable console
routes for the app. (or vice versa).

On Wed, Sep 19, 2012 at 8:44 PM, Jurian Sluiman notifications@github.comwrote:

Uhm, no this was completely made up with random assumptions... why you
think that? In http the url keeps working, in the cli the correct router is
injected. I can assemble urls as that's the point of this PR.

Contributor

juriansluiman commented Sep 20, 2012

@Thinkscape No, in this case (now the PR is merged) the logic is as follows:

  1. Http env: $sm->get('router') gives http router, url helper has http router
  2. Cli env: $sm->get('router') gives cli router, url helper has http router

The case for 2 was that the url helper got the cli router too. That makes it impossible to assemble routers in eg mails linking to your webserver. Before and after this merge, the application in a cli environment has the cli router. It only addresses the router injected into the url helper.

Member

Thinkscape commented Sep 20, 2012

Ah. That would suggest separate SM instance. Bohkay.

On Thu, Sep 20, 2012 at 10:11 AM, Jurian Sluiman
notifications@github.comwrote:

@Thinkscape https://github.com/Thinkscape No, in this case (now the PR
is merged) the logic is as follows:

  1. Http env: $sm->get('router') gives http router, url helper has http
    router
  2. Cli env: $sm->get('router') gives cli router, url helper has http
    router

The case for 2 was that the url helper got the cli router too. That makes
it impossible to assemble routers in eg mails linking to your webserver.
Before and after this merge, the application in a cli environment has
the cli router. It only addresses the router injected into the url helper.


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/2384#issuecomment-8719674.

Problem:
Uncaught exception 'Zend\Mvc\Router\Exception\RuntimeException' with message 'Request URI has not been set'

on something like: url('news', [], {'force_canonical': true})

Maybe is it fault of this fix?

Owner

weierophinney commented Sep 20, 2012

Unrelated. Please post an issue (http://framework.zend.com/issues) or ask
on the mailing list.

On Thursday, September 20, 2012, Oscar Fanelli wrote:

Problem:
Uncaught exception 'Zend\Mvc\Router\Exception\RuntimeException' with
message 'Request URI has not been set'

on something like: url('news', [], {'force_canonical': true})

Maybe is it fault of this fix?


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/2384#issuecomment-8749241.

Matthew Weier O'Phinney
matthew@weierophinney.net
http://mwop.net/

@weierophinney, i have posted on the newsletter as a reply of the "Zend Framework 2.0.1 Released" thread... however, i tried to replace HttpRouter with Router and it works for me... (so it's a fault of this fix)

Contributor

juriansluiman commented Sep 21, 2012

I have tested the http router in cli, but did not get thus
On Sep 21, 2012 1:36 AM, "Oscar Fanelli" notifications@github.com wrote:

weierophinney, i have posted on the newsletter as a reply of the "Zend
Framework 2.0.1 Released" thread... however, i tried to replace HttpRouter
with Router and it works for me...


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/2384#issuecomment-8749327.

well, i found this problem with a line like "url('news', array(), array('force_canonical' => true))" putted in http (not in cli)

however, i'm using the twig render... i don't know if maybe it's his fault

Contributor

juriansluiman commented Sep 21, 2012

I have found the cause of the problem. The root cause is that once I inject the HttpRouter in the url, this is another instance than the Router. In case of CLI, this is exactly what you want. In case of http env, you also get the two instances. HttpRouter is not an alias of the Router, so that makes it a little hard.

During Router::match() the request uri is set automatically, so if you later assemble a route with force_canonical, the uri is used for the scheme and host name. If you have set the Router in the application, that instance has the request uri set since that one is used to match. The HttpRouter has not the request uri set, but is used to assemble.

The only fix I can come up with now to make sure the http env has the same router, but the cli has a different router is by doing this:

$plugins->setFactory('url', function($sm) use($serviceLocator) {
    $helper = new ViewHelper\Url;
    $router = Console::isConsole() ? 'HttpRouter' : 'Router';
    $helper->setRouter($serviceLocator->get($router));

    $match = $serviceLocator->get('application')
                ->getMvcEvent()
                ->getRouteMatch();

    if ($match instanceof RouteMatch) {

        $helper->setRouteMatch($match);
    }

    return $helper;
});

Not sure if that's the best approach. Also, you still have trouble in a cli environment because no request uri is set, so you have to do that manually....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment