Joomla! Issue Tracker - CMS

Download
Launch
Feeling Lucky

[#28870] - [4.0] Optimizing routing in languagefilter plugin

? Pending

User tests: Successful: Unsuccessful:

avatar Hackwar
Hackwar
29 Apr 2020

This optimizes the routing slightly in the languagefilter plugin. The buildRule() method only does anything when SEF is enabled and thus we can drop those checks and instead make the call to the method conditional based on SEF being enabled. This also further optimizes the onAfterRoute() call, since that second setVar() call is unnecessary.

Unfortunately this is nearly untestable and should be merged on a code review.

This has come up while trying to look at a change to remove the trailing slash in our URLs.

avatar Hackwar Hackwar - open - 29 Apr 2020
avatar Hackwar Hackwar - change - 29 Apr 2020
Status New ⇒ Pending
avatar joomla-cms-bot joomla-cms-bot - change - 29 Apr 2020
Category ⇒ Front End Plugins
avatar infograf768
infograf768 - comment - 29 Apr 2020

Some of the patch are indeed obvious.
This part below, can you explain a bit more and what will it change concretely? URL example.

		// Attach build rules for language SEF.
 		$router->attachBuildRule(array($this, 'preprocessBuildRule'), Router::PROCESS_BEFORE);
-		$router->attachBuildRule(array($this, 'buildRule'), Router::PROCESS_BEFORE);
 
 		if ($this->mode_sef)
 		{
+			$router->attachBuildRule(array($this, 'buildRule'), Router::PROCESS_BEFORE);
 			$router->attachBuildRule(array($this, 'postprocessSEFBuildRule'), Router::PROCESS_AFTER);

Note: see my NOTE for J4 here:
#28814 (comment)
which is a BUG imho.

avatar Hackwar
Hackwar - comment - 29 Apr 2020

It doesn't change anything. All URLs stay the same. But the code of that method only does something when we have SEF enabled. For that it checks that mode_sef parameter each time it is called, which could potentially be a few hundred times for each pageload. We can drop that check and instead only add the method to our pipeline when SEF is enabled. Long story short: This change improves the readability of what buildRule() does and is a tiny performance improvement.

avatar wilsonge wilsonge - change - 2 May 2020
Status Pending ⇒ Fixed in Code Base
Closed_Date 0000-00-00 00:00:00 ⇒ 2020-05-02 17:03:51
Closed_By ⇒ wilsonge
Labels Added: ?
avatar wilsonge wilsonge - close - 2 May 2020
avatar wilsonge wilsonge - merge - 2 May 2020
avatar wilsonge
wilsonge - comment - 2 May 2020

Merged on careful review!

Add a Comment

Login with GitHub to post a comment