[#5446] - Routing: JComponentRouterRulesMenu to find right Itemid
- Closed
- 2 Aug 2015
- Medium
- Build: staging
- # 5446
- Diff
- Hackwar:componentrulesitemid
- Success The Travis CI build passed Details
User tests: Successful: Unsuccessful:
This rule implements finding the right Itemid as a generic rule for all component routers that extend from JComponentRouterView. It makes most of the parts of the *HelperRoute classes obsolete.
How to test
- Apply this change
- Change the router of com_content by replacing
class ContentRouter extends JComponentRouterBase
{with
class ContentRouter extends JComponentRouterView
{
function __construct($app = null, $menu = null) {
$categories = new JComponentRouterViewconfiguration('categories');
$categories->setKey('id');
$this->registerView($categories);
$category = new JComponentRouterViewconfiguration('category');
$category->setKey('id')->setParent($categories, 'id')->setNestable()->addLayout('blog');
$this->registerView($category);
$article = new JComponentRouterViewconfiguration('article');
$article->setKey('id')->setParent($category, 'catid');
$this->registerView($article);
$this->registerView(new JComponentRouterViewconfiguration('archive'));
$this->registerView(new JComponentRouterViewconfiguration('featured'));
$this->registerView(new JComponentRouterViewconfiguration('form'));
parent::__construct($app, $menu);
$this->attachRule(new JComponentRouterRulesMenu($this));
}
public function getCategorySegment($id, $query)
{
$category = JCategories::getInstance($this->getName())->get($id);
if ($category)
{
return array_reverse($category->getPath());
}
return array();
}
public function getCategoriesSegment($id, $query)
{
return $this->getCategorySegment($id, $query);
}
public function getArticleSegment($id, $query)
{
return array($id);
}- Comment line 62 and 111 from /components/com_content/helpers/route.php to disable the lookup of the Itemid there.
- See that the right menu items are still found for the items
This was updated on May 22nd 2015. All discussions prior to that point have either been integrated or are obsolete.
This was made possible through the generous donation of the people mentioned in the following link via an Indiegogo campaign: http://joomlager.de/crowdfunding/5-contributors
jissues-bot
- | Labels |
Added:
?
|
||
brianteeman
- | Category | ⇒ | Router / SEF |
Hi Hannes,
How can 3rd party extensions disable/bypass those rules that could be added by components, or even if they make their way int Joomla core pre-4.0? or at least are you storing the original request somewhere so that we can revert any change made by those rules, as discussed in #5264?
"By letting any Itemid being passed in a developer can hijack a route. Connecting a component to any random Itemid. Shouldn't that be prevented ? ": Not random, but manually selected by user - as allowed by most SEF extensions, yes, absolutely.
Cheers
I guess JComponentRouterAdvanced will get a "removeRule()" method, that takes a rule that you can first find with a "getRules()" method and remove that from the rules to process again. Remember that you will have the buildpreprocess() step, where you can attach yourself already to, to for example add your stuff to the query.
This is not about attaching things to the query. This is about being able to undo things that other rules have attached, or any change they might have done to the original query. In other words not being trapped again in the problems the language filter plugins rules have created (not by themselves, but by being set in stone and unmodifiable)
getRules() and removeRules() sound good, as long as we have an event we can use to be reasonably sure all rules have been attached (so we can also be sure to remove them all). Too bad there isn't an onBeforeRoute...
I guess even if we use onAfterInitialise, we can rely on the plugins ordering so we should be good in most cases.
For the time being, I would add all rules in the component routers constructors, so when you cycle through all components and then use JRouterSite::getComponentRouter() and do your own stuff by adding/removing rules again in onAfterInitialise, you would be good. Since I would like to have more than one set of component rules, I could also see us having a -1 option that disables all the rule-adding in the constructor and you can do your magic.
Regarding the language filter plugin: I hope that it is a lot saner now when #5140 is merged. I can't really think of anything else that you could want in that regard...
Hi Hannes,
Let me clear that up first: I have no problem with the language filter rules, how they were working or whatever. The only issue was that we couldn't bypass them, or even know what the actual original request was and what the language filter rules had done to it. The same would apply to any extension adding rules, whether for parsing or building.
It was a bit more difficult with the language filter because its parse rule is always executed first, regardless of the actual system plugins ordering set by user or otherwise.
As for adding/removing rules on onAfterInitialise, I can request users to be sure my system plugin is the last one in order (and I can check that periodically) and thus be sure to remove all rules that may have been added by other extensions. An onBeforeRoute event would be more secure, but maybe not worth the added perf cost I guess. I'll make sure to do some trials in due time.
@weeblr-dev @Hackwar Do we really need to have the ability to remove rules ? Removing rules requires knowledge about the rules being added. Wouldn't it make more sense to allow rules to override others ?
An idea can be to store the original query and do the query modifications into a different array. That way a rule can always use the original query and can also more easily override without needing to have knowledge about other rules.
That exactly would make it impossible to override rules. Right now, you can attach a rule to the router and clear out the query variables one by one, creating your own rule and in the end, your URL is done and the query variables are empty. Then the next part of the router only sees an empty query and directly returns. If we kept the original query, how would a later rule know that an earlier rule already processed that part?
Yes, removing rules requires you to have quite a knowledge about what has been added before, but the main situation where I see this to be used would be a plugin like sh404sef, which could remove the default behavior of Joomla and replace it with its own. It's not going to be a bunch of GUI options that let you influence that directly.
I've done further fixes and implemented unittests for this rule.
@Hackwar About the remove rule comments you made above. I understand, care should be taken though that if no rules are there the URL would still work. Otherwise things could go very wrong, without a working router Joomla would no longer work.
A solution would be to ensure that if no rules are present we fallback to a none-SEF form. Is that already the case ? If no, might be good to include this scenario also in the unit tests.
@johanjanssens I specifically chose JComponentRouterViewconfiguration since I did not want to use JComponentRouterView for this. I can see us wanting to have a JComponentRouterView class for different purposes.
Regarding the setName/setViewName: Please see the latest commits to this PR. Those have since been removed.
@Hackwar About setName/setViewName() I missed those changes. Looking much better indeed API wise.
About JComponentRouterViewconfiguration vs JComponentRouterView. I understand, still not 100% happy with JComponentRouterViewconfiguration, any alternatives ? JComponentRouterConfig maybe, or JComponentRouterContext ?
joomla-cms-bot
- | Labels |
Added:
?
|
||
I've updated this PR to work with the changed JComponentRouterView class. Please test.
simply B/C. This should not change the URLs.
| Status | Pending | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2015-08-02 16:21:33 |
| Closed_By | ⇒ | Hackwar |
@Hackwar Did a first pass of the proposed code and added inline comments. I like the simplicity that is being proposed.