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

SEF wrong path computation - JTableNested::getPath() and JTableNested::rebuildPath() #1272

Closed
jms2win opened this Issue Jun 10, 2012 · 10 comments

Comments

Projects
None yet
4 participants
Contributor

jms2win commented Jun 10, 2012

In a live environment, we encountered a problem with the SEF URL that was erratic and using other menu alias.

After investigation, we discovered that the way that the path used to compute the SEF in JTableNested::getPath() and JTableNested::rebuildPath() was not perfect.

These function are in charged to compute into an array the list of menu item that must be taken in account for a specific menu entry and use for that the lft / rgt field present in the #__menu table.

The problem identified is that sometimes you can have menu item with multiple lft/rgt values that can come from back-end and front-end merged.

The query that is perform is something like

SELECT p.*
FROM #__menu AS n, #__menu AS p
WHERE n.lft BETWEEN p.lft AND p.rgt AND n.id = 132
ORDER BY p.lft

With a sample result that is

id menutype title alias path published parent_id level component_id access lft Ascending rgt language client_id
1 Menu_Item_Root root 1 0 0 0 0 0 115 * 0
142 main com_multisitescontent commultisitescontent commultisitescontent 0 1 1 10026 1 41 48 1
103 main Users users acymailing/users 0 102 2 0 1 42 43 1
143 main com_multisitescontent_purgeCache commultisitescontentpurgecache commultisitescontent/commultisitescontentpurgecach... 0 142 2 10026 1 42 43 1
132 mainmenu Présentation presentation presentation 1 1 1 22 1 43 50 * 0

As you can see, the result contain both back-end and front-end menu item resulting of the query performed in the getPath() and rebuildPath() functions.

To avoid this erratic path computation, we added a criteria in the where clause to ensure that the query is performed with the same "client_id" values. So that the path is computed either for the back-end or front-end but never the merge of the menu.

SELECT p.*
FROM #__menu AS n, #__menu AS p
WHERE n.lft BETWEEN p.lft AND p.rgt AND n.id = 132 AND p.client_id=n.client_id
ORDER BY p.lft

Where it is the code as we modified it
The code comes from Joomla 2.5.4

public function getPath($pk = null, $diagnostic = false)
{
    // Initialise variables.
    $k = $this->_tbl_key;
    $pk = (is_null($pk)) ? $this->$k : $pk;

    // Get the path from the node to the root.
    $query = $this->_db->getQuery(true);
    $select = ($diagnostic) ? 'p.' . $k . ', p.parent_id, p.level, p.lft, p.rgt' : 'p.*';
    $query->select($select);
    $query->from($this->_tbl . ' AS n, ' . $this->_tbl . ' AS p');
    $query->where('n.lft BETWEEN p.lft AND p.rgt');
    $query->where('n.' . $k . ' = ' . (int) $pk);
    $query->where('p.client_id=n.client_id');             // <============== Add here a filtering on the client_id to ensure use the correct Back-end / Front-end value.
    $query->order('p.lft');

    $this->_db->setQuery($query);
    $path = $this->_db->loadObjectList();



public function rebuildPath($pk = null)
{
    // If there is no alias or path field, just return true.
    if (!property_exists($this, 'alias') || !property_exists($this, 'path'))
    {
        return true;
    }

    // Initialise variables.
    $k = $this->_tbl_key;
    $pk = (is_null($pk)) ? $this->$k : $pk;

    // Get the aliases for the path from the node to the root node.
    $query = $this->_db->getQuery(true);
    $query->select('p.alias');
    $query->from($this->_tbl . ' AS n, ' . $this->_tbl . ' AS p');
    $query->where('n.lft BETWEEN p.lft AND p.rgt');
    $query->where('n.' . $this->_tbl_key . ' = ' . (int) $pk);
    $query->where('p.client_id=n.client_id');             // <============== Add here a filtering on the client_id to ensure use the correct Back-end / Front-end value.
    $query->order('p.lft');
    $this->_db->setQuery($query);

    $segments = $this->_db->loadColumn();

We suggest that you add this additional constrain in the path computation to avoid return a result with both front-end/back-end menu item.

This isssue is probably due to a wrong menu item lft/rgt computation that the joomla 2.5.4 "rebuild" menu button can resolve but before discovering that some SEF URL was wrong, google and other search engine can index wrong value.
Add this constrain should improve the quality of the SEF value produced.

Contributor

elinw commented Jun 10, 2012

Since you know the changes, why don't you make a pull request for them?
I know we had another issue (not in routing) in the past that was related to the problem of not using the menu client_id.

Contributor

jms2win commented Jun 10, 2012

This is the first time that I use this website. In the past, I used the bug tracker but it was written to post the bug in this site.
I thought that was the procedure to let somebody analyze the issue and decide if it can be put in the code like in the bug tracker.

Contributor

elinw commented Jun 13, 2012

Sure, but the cool think about Github is that you don't need any special software to propose a simple change, you just edit online and send a pull request. You'll get a lot more people looking at it if you include the code and it's your code, so you really should submit it.

Contributor

jms2win commented Jun 14, 2012

Sorry but I don't understand how to do that (the pull request) and when I go in the platform code, I don't know where is the JPlatform corresponding to Joomla 2.5.4 - the version 11.1. When I look in the current code, the table is no more at the same location. I prefer let a people familliar with GitHub to update the correct source and apply the fix.

Contributor

elinw commented Jun 26, 2012

Tables have been moved to their own package libraries/joomla/table ... So the problem as I understand it will certainly be true, that many methods in JTableNested will fail if the nested part of the table becomes corrupted. If the api is used perfectly at all times and no non perfect data are ever imported we will never have this problem. However we sadly do not live in that world. So the problem might not only happens because of tangling together of branches from two clients. So then the problem becomes whether some kind of sanity check should be routinely done and if it fails should rebuild() happen.

Contributor

jms2win commented Jun 27, 2012

I don't know the reason why the content of the DB has wrong right/left value and perhaps that each time that you install an extension or create a menu item, you should rebuild the menu automatically.

I agree that the additional "client_id" constrain that I added in the query just reduce the inconsistency and does not solve the reason why the right/left value are wrong.

I just suggest that the "client_id" constrain is added to at least return SEF path that is consistent with back-end of front-end only and not a merge of both. When the SEF has erratic value, that can impact the referencement.

As Joomla 2.5 is for a Long Term Support and that it was also announced at J and Beyond 2012 (where I was present) that Joomla 3.0 will contain a full legacy 2.5 compatibility, I think that improve the quality of the Joomla 2.5 is very important and that this simple fix can contribute to that.

rugar8 commented Aug 16, 2012

Thank you for posting this "band-aid fix" -- been stressing over this same issue on my sites.

Can you please advise what directory/file this code is located in?

Thank you kindly!

Contributor

jms2win commented Aug 16, 2012

The file is located in \libraries\joomla\database\tablenested.php

Contributor

elinw commented Aug 19, 2012

By any chance do you have the same name for a menu in the front and back end?
I agree it should be fixed but it's puzzling why this is happening.
Is it possible that some of the extensions you are installing are not installing correctly?
143 and 103 have the same lft and rgt which should never happen.

Contributor

jms2win commented Aug 20, 2012

I don't remember to have saw same name in the menu item but identified that the computation of the left/right was wrong for unknown reason.
I have started to add this "security fix" to ensure that this will reduce the number of cases where it could happen and after, I have rebuilt the menu in the back-end to let the left/right rebuilt correctly.

@eddieajau eddieajau closed this Apr 5, 2013

jms2win added a commit to jms2win/joomla-platform that referenced this issue Apr 10, 2014

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