Skip to content
New issue

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

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Jroute between sites #16879

Merged
merged 31 commits into from
Mar 9, 2018
Merged

Jroute between sites #16879

Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
31 commits
Select commit Hold shift + click to select a range
69f4f3f
allow to force site client when we are in admin
andrepereiradasilva Apr 22, 2016
d8e7a5d
allow from site to admin too
andrepereiradasilva Apr 22, 2016
9c1a3a8
first try to make language filter router work on admin too when calle…
andrepereiradasilva Jul 4, 2016
7933bdb
revert
andrepereiradasilva Jul 4, 2016
173c231
revert 2
andrepereiradasilva Jul 4, 2016
0faa289
make language filter pluign work with JRoute from admin
andrepereiradasilva Jul 4, 2016
2ad9ffd
cs
andrepereiradasilva Jul 9, 2016
0f1d95b
cs 2
andrepereiradasilva Jul 9, 2016
4502d6f
Merge branch 'staging' into jroute-between-sites
andrepereiradasilva Jul 9, 2016
d07ee16
use app
andrepereiradasilva Jul 10, 2016
3ef9ba2
solve php notices
andrepereiradasilva Jul 13, 2016
a4eaa94
Merge branch 'staging' into jroute-between-sites
andrepereiradasilva Oct 28, 2016
219438b
Merge branch 'staging' into jroute-between-sites
andrepereiradasilva Oct 29, 2016
129d14c
Merge joomla-cms/staging into jroute-between-sites
izharaazmi Jun 27, 2017
671b504
Few tweaks for the PR
izharaazmi Jun 27, 2017
94bdfc5
Suggestions from @mbabker, and few other improvements.
izharaazmi Jun 27, 2017
c7dd17d
Moved active client detection logic to JRoute::_() and disallow null …
izharaazmi Jun 28, 2017
1b233d5
Merge branch '3.8-dev' of github.com:joomla/joomla-cms into pr-16879
mbabker Jul 18, 2017
0686d07
Expand mocking in JHtmlBehaviorTest
mbabker Jul 18, 2017
eb26908
Mocking in JHtmlIconsTest
mbabker Jul 18, 2017
0b5ad2e
Fix reset of router property
mbabker Jul 18, 2017
12633a3
Mocking in JPaginationTest
mbabker Jul 18, 2017
fadc304
Mocking in JDocumentOpensearchTest
mbabker Jul 18, 2017
a783d23
Mocking in JDocumentRendererHtmlModulesTest
mbabker Jul 18, 2017
fdfa09b
Mocking in JFormFieldRulesTest
mbabker Jul 18, 2017
5bf6b0a
Test fixes
mbabker Jul 18, 2017
1d79429
Merge pull request #3 from mbabker/pr-16879
izharaazmi Jul 18, 2017
300699d
Moved SEF mode detection to CMSApplication for it being called from a…
izharaazmi Aug 9, 2017
124aab1
Merge branch 'staging' into jroute-between-sites
izharaazmi Aug 9, 2017
b3136c4
Remove magic methods to force using of JRoute::link() directly
izharaazmi Aug 9, 2017
a91cd6d
Merge origin/staging into jroute-between-sites
izharaazmi Sep 15, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions libraries/cms/router/administrator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,10 @@ public function build($url)
// Get the path data
$route = $uri->getPath();

// Add basepath to the uri
$uri->setPath(JUri::base(true) . '/' . $route);
// Add backend basepath to the uri
$basePath = JUri::root(true) . '/' . basename(JPATH_ADMINISTRATOR);

$uri->setPath($basePath . '/' . $route);

return $uri;
}
Expand Down
4 changes: 2 additions & 2 deletions libraries/cms/router/site.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,8 @@ public function build($url)
}
}

// Add basepath to the uri
$uri->setPath(JUri::base(true) . '/' . $route);
// Add frontend basepath to the uri
$uri->setPath(JUri::root(true) . '/' . $route);

return $uri;
}
Expand Down
77 changes: 55 additions & 22 deletions libraries/joomla/application/route.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,45 +12,54 @@
/**
* Route handling class
*
* @method static string site($url, $xhtml = true, $ssl = null) Create a frontend route.
* @method static string administrator($url, $xhtml = true, $ssl = null) Create a backend route.
*
* @since 11.1
*/
class JRoute
{
/**
* The route object so we don't have to keep fetching it.
*
* @var JRouter
* @var JRouter[]
* @since 12.2
*/
private static $_router = null;
private static $_router = array();

/**
* Translates an internal Joomla URL to a humanly readable URL.
*
* @param string $url Absolute or Relative URI to Joomla resource.
* @param boolean $xhtml Replace & by & for XML compliance.
* @param integer $ssl Secure state for the resolved URI.
* 0: (default) No change, use the protocol currently used in the request
* 1: Make URI secure using global secure site URI.
* 2: Make URI unsecure using the global unsecure site URI.
* @param string $url Absolute or Relative URI to Joomla resource.
* @param boolean $xhtml Replace & by & for XML compliance.
* @param integer $ssl Secure state for the resolved URI.
* 0: (default) No change, use the protocol currently used in the request
* 1: Make URI secure using global secure site URI.
* 2: Make URI unsecure using the global unsecure site URI.
* @param string $client The client name for which to build the link. NULL to use active client.
*
* @return string The translated humanly readable URL.
* @return string The translated humanly readable URL.
*
* @since 11.1
*/
public static function _($url, $xhtml = true, $ssl = null)
public static function _($url, $xhtml = true, $ssl = null, $client = null)
{
if (!self::$_router)
// Get the target client name.
if (!isset($client))
{
// Get the router.
$app = JFactory::getApplication();
self::$_router = $app::getRouter();
$client = JFactory::getApplication()->getName();
}

// Make sure that we have our router
if (!self::$_router)
{
return;
}
// Get the router instance.
if (!isset(self::$_router[$client]))
{
self::$_router[$client] = JFactory::getApplication()->getRouter($client);
}

// Make sure that we have our router
if (!self::$_router[$client])
{
return null;
}

if (!is_array($url) && (strpos($url, '&') !== 0) && (strpos($url, 'index.php') !== 0))
Expand All @@ -59,8 +68,7 @@ public static function _($url, $xhtml = true, $ssl = null)
}

// Build route.
$uri = self::$_router->build($url);

$uri = self::$_router[$client]->build($url);
$scheme = array('path', 'query', 'fragment');

/*
Expand All @@ -76,7 +84,7 @@ public static function _($url, $xhtml = true, $ssl = null)

if (!is_array($host_port))
{
$uri2 = JUri::getInstance();
$uri2 = JUri::getInstance();
$host_port = array($uri2->getHost(), $uri2->getPort());
}

Expand All @@ -99,4 +107,29 @@ public static function _($url, $xhtml = true, $ssl = null)

return $url;
}

/**
* Magic method to provide short access to the clients' route functions "site" and "administrator".
* - To build a route for site: <var>JRoute::site($url)</var>.
* - To build a route for administrator: <var>JRoute::administrator($url)</var>
*
* @param string $name The called method name
* @param array $arguments The method arguments
*
* @return string
*
* @since __DEPLOY_VERSION__
*/
public static function __callStatic($name, $arguments)
{
if (count($arguments))
{
$xhtml = isset($arguments[1]) ? $arguments[1] : true;
$ssl = isset($arguments[2]) ? $arguments[2] : null;

return static::_($arguments[0], $xhtml, $ssl, $name);
}

return null;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a trigger_error() call because you in essence have a "call to undefined function" error here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we check for a valid client name here, the behavior would be different when called using undescore function. That function fails silently on invalid router and we need to keep it like that for B/C, imho.

And here we are only checking for missing first parameter coz we need to populate 4th parameter.

Another alternate way that comes to my mind is to not modify the undescore function signature and add another common function that accepts client name as first parameter.

We might then not need the magic function too. Your opinion please.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I hate the use of magic functions. They usually expose unwanted behavior and do not handle error conditions appropriately (in an error state they should do the same thing as the native PHP engine does for whatever that magic function is overloading). Even in the Framework's database repo, I removed __call() and just made "real" stub methods for the shorthand q(), qn(), and e() methods.

So personally I'd just add stub real methods and avoid the magic as a first option. If the magic method is really needed, then it needs to error trap/handle properly.

That function fails silently on invalid router and we need to keep it like that for B/C, imho.

If the system can't generate a route, it shouldn't silently fail over. There needs to be an error raised.

}
}
62 changes: 38 additions & 24 deletions plugins/system/languagefilter/languagefilter.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,14 +91,15 @@ public function __construct(&$subject, $config)

$this->app = JFactory::getApplication();

// Setup language data.
$this->mode_sef = $this->app->get('sef', 0);
$this->sefs = JLanguageHelper::getLanguages('sef');
$this->lang_codes = JLanguageHelper::getLanguages('lang_code');
$this->default_lang = JComponentHelper::getParams('com_languages')->get('site', 'en-GB');

// If language filter plugin is executed in a site page.
if ($this->app->isClient('site'))
{
// Setup language data.
$this->mode_sef = $this->app->get('sef', 0);
$this->sefs = JLanguageHelper::getLanguages('sef');
$this->lang_codes = JLanguageHelper::getLanguages('lang_code');
$this->default_lang = JComponentHelper::getParams('com_languages')->get('site', 'en-GB');

$levels = JFactory::getUser()->getAuthorisedViewLevels();

foreach ($this->sefs as $sef => $language)
Expand All @@ -112,6 +113,21 @@ public function __construct(&$subject, $config)
}
}
}
// If language filter plugin is executed in a admin page (ex: JRoute site).
else
{
// Set current language to default site language, fallback to en-GB if there is no content language for the default site language.
$this->current_lang = isset($this->lang_codes[$this->default_lang]) ? $this->default_lang : 'en-GB';

foreach ($this->sefs as $sef => $language)
{
if (!array_key_exists($language->lang_code, JLanguageHelper::getInstalledLanguages(0)))
{
unset($this->lang_codes[$language->lang_code]);
unset($this->sefs[$language->sef]);
}
}
}
}

/**
Expand All @@ -125,26 +141,24 @@ public function onAfterInitialise()
{
$this->app->item_associations = $this->params->get('item_associations', 0);

if ($this->app->isClient('site'))
{
$router = $this->app->getRouter();

// Attach build rules for language SEF.
$router->attachBuildRule(array($this, 'preprocessBuildRule'), JRouter::PROCESS_BEFORE);
$router->attachBuildRule(array($this, 'buildRule'), JRouter::PROCESS_DURING);
// We need to make sure we are always using the site router, even if the language plugin is executed in admin app.
$router = $this->app->isClient('site') ? $this->app->getRouter() : JApplicationCms::getInstance('site')->getRouter('site');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just have this always call JApplicationCms::getInstance('site')->getRouter('site'), the singleton instance in JApplicationCms will be the same as the one in JFactory::$application if you're on the frontend anyway.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure.


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

// Attach parse rules for language SEF.
$router->attachParseRule(array($this, 'parseRule'), JRouter::PROCESS_DURING);
if ($this->mode_sef)
{
$router->attachBuildRule(array($this, 'postprocessSEFBuildRule'), JRouter::PROCESS_AFTER);
}
else
{
$router->attachBuildRule(array($this, 'postprocessNonSEFBuildRule'), JRouter::PROCESS_AFTER);
}

// Attach parse rules for language SEF.
$router->attachParseRule(array($this, 'parseRule'), JRouter::PROCESS_DURING);
}

/**
Expand All @@ -157,7 +171,7 @@ public function onAfterInitialise()
public function onAfterRoute()
{
// Add custom site name.
if (isset($this->lang_codes[$this->current_lang]) && $this->lang_codes[$this->current_lang]->sitename)
if ($this->app->isClient('site') && isset($this->lang_codes[$this->current_lang]) && $this->lang_codes[$this->current_lang]->sitename)
{
$this->app->set('sitename', $this->lang_codes[$this->current_lang]->sitename);
}
Expand Down