Brain-WP / Cortex

Routing system for WordPress
MIT License
347 stars 20 forks source link

Routes->url() lacks request context #1

Closed Solinx closed 8 years ago

Solinx commented 9 years ago

Hello Guiseppe,

We're developing two websites build on top of one WordPress instance. Your routing component has been a great help in quickly and efficiently getting the routing right.

While using your component we found that there are some minor issues. The most important of these issues concerns the generation of urls based on the route ids. We use two different host for our two websites, but this contextual information is ignored when generating urls based on route ids.

Looking into the matter we found that the Symfony RequestContext is used as an empty shell. Normally this object is populated with information from the HttpFoundation Request object, but you also got your own Request object. Could you use either of these to populate RequestContext? Thanks!

Cheers, Wouter van Dam

gmazzap commented 9 years ago

Actually I'm using my Amygdala to get Request informations in Cortex, but I've not implemented it for url generation. I'll look into this very soon.

Solinx commented 9 years ago

Thanks. Do you think you'll have time to implement this enhancement before the end of the month?

gmazzap commented 9 years ago

Not sure but very probably.

gmazzap commented 9 years ago

@Solinx I just published my Dev branch. It should solve the issue of url generation with host context. Would you mind to test it? All unit tests pass, and I also done some tests on a very simple test environment, would love to see some tests from someone else. If you confirm it works I can push to master and so update packagist repo.

Solinx commented 9 years ago

Hello Giuseppe,

Thanks for looking into this already!

Overall it appears to work well. There are two pieces of feedback I can provide you with.

  1. When an invalid route_id is passed a fatal error occurs.
  2. Generating urls from one domain to another causes problems, which is probably because the context is only set once for all urls that are generated.
gmazzap commented 9 years ago

When an invalid route_id is passed a fatal error occurs

yep, I should fix that.

Generating urls from one domain to another causes problems

I've tested with different subdomains (generate urls for one subdomain while being in another) and it works. Sincerly I've not tested with different 1st levels domain. I'll try.

Solinx commented 9 years ago

Sorry, I should have been more specific. I was testing on kiosk.sub.domain.org and sub.domain.org.

After dumping some vars it looked like setting the context once caused issues with Symfony. Symfony ended up generating "//sub.domain.orghttp://sub.domain.org" for the website home route on kiosk.sub.domain.org. (edit: as direct result of "$this->getBrain()->get( 'symfony.generator' )->generate( $route_id, $args );")

I do not have time to provide more details right now, but hopefully I can add some more information either tomorrow or the day after.

gmazzap commented 9 years ago

Are you sure you are tested the last commit? That issue should be already fixed. Moreover, when you set host for routes, do you use full host e.g. 'sub.domain.org' or '{subdomain}.domain.org'?

Solinx commented 9 years ago

Ah, no. I pulled it in earlier today. I'll retest later.