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

Routing: Implementing routing stages #5264

Closed
wants to merge 15 commits into from
Closed

Routing: Implementing routing stages #5264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
0e6ca11
Implementing routing stages pre-, main and post-stage
Hackwar Nov 30, 2014
1ba0176
Fixing codestyle, typo and unittests
Hackwar Dec 1, 2014
4e0234f
Checking $stage parameter when methods are called
Hackwar Dec 7, 2014
64bcedb
Codestyle
Hackwar Dec 7, 2014
889587c
Codestyle, second attempt
Hackwar Dec 7, 2014
4ab35bc
Fixing typo
Hackwar Dec 8, 2014
e3b2500
Adding consts
Hackwar Dec 12, 2014
2b9cad8
Merge branch 'staging' of https://github.com/joomla/joomla-cms into r…
Hackwar Dec 12, 2014
5c70150
Updating unittests for JRouter to accommodate the new routing stages
Hackwar Dec 29, 2014
a9da353
Merge branch 'staging' of https://github.com/joomla/joomla-cms into r…
Hackwar Dec 29, 2014
99e7dc6
Reaching 100% coverage for unittests with this
Hackwar Dec 30, 2014
ded1d0b
Fixing position of component router preprocess stage
Hackwar Jan 4, 2015
12c2aa5
Codestyle
Hackwar Jan 4, 2015
f72c423
Fixing order of method calls in JRouterSite
Hackwar Jan 5, 2015
7eefb40
Fixing unittests
Hackwar Jan 5, 2015
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
105 changes: 87 additions & 18 deletions libraries/cms/router/router.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@

/**
* Set the available masks for the routing mode
*
* @deprecated 4.0
*/
const JROUTER_MODE_RAW = 0;
const JROUTER_MODE_SEF = 1;
Expand All @@ -22,6 +24,13 @@
*/
class JRouter
{
/**
* Processing-stages for the router
*/
const PROCESS_BEFORE = 'preprocess';
const PROCESS_DURING = '';
const PROCESS_AFTER = 'postprocess';

/**
* The rewrite mode
*
Expand Down Expand Up @@ -63,8 +72,12 @@ class JRouter
* @since 1.5
*/
protected $rules = array(
'buildpreprocess' => array(),
'build' => array(),
'parse' => array()
'buildpostprocess' => array(),
'parsepreprocess' => array(),
'parse' => array(),
'parsepostprocess' => array()
);

/**
Expand All @@ -75,8 +88,12 @@ class JRouter
* @deprecated 4.0 Will convert to $rules
*/
protected $_rules = array(
'buildpreprocess' => array(),
'build' => array(),
'parse' => array()
'buildpostprocess' => array(),
'parsepreprocess' => array(),
'parse' => array(),
'parsepostprocess' => array()
);

/**
Expand Down Expand Up @@ -175,8 +192,12 @@ public static function getInstance($client, $options = array())
*/
public function parse(&$uri)
{
// Do the preprocess stage of the URL build process
$vars = $this->processParseRules($uri, self::PROCESS_BEFORE);

// Process the parsed variables based on custom defined rules
$vars = $this->_processParseRules($uri);
// This is the main parse stage
$vars += $this->_processParseRules($uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Hackwar For code readability sake it would change this to $vars += $this->_processParseRules($uri, self::PROCESS_DURING);


// Parse RAW URL
if ($this->_mode == JROUTER_MODE_RAW)
Expand All @@ -190,6 +211,9 @@ public function parse(&$uri)
$vars += $this->_parseSefRoute($uri);
}

// Do the postprocess stage of the URL build process
$vars += $this->processParseRules($uri, self::PROCESS_AFTER);

return array_merge($this->getVars(), $vars);
}

Expand All @@ -214,7 +238,11 @@ public function build($url)
// Create the URI object
$uri = $this->createURI($url);

// Process the uri information based on custom defined rules
// Do the preprocess stage of the URL build process
$this->processBuildRules($uri, self::PROCESS_BEFORE);

// Process the uri information based on custom defined rules.
// This is the main build stage
$this->_processBuildRules($uri);
Copy link
Contributor

Choose a reason for hiding this comment

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

@Hackwar For code readability sake it would change this to $vars += $this->_processBuildRules($uri, self::PROCESS_DURING);


// Build RAW URL
Expand All @@ -229,6 +257,9 @@ public function build($url)
$this->_buildSefRoute($uri);
}

// Do the postprocess stage of the URL build process
$this->processBuildRules($uri, self::PROCESS_AFTER);

$this->cache[$key] = clone $uri;

return $uri;
Expand Down Expand Up @@ -338,28 +369,46 @@ public function getVars()
* Attach a build rule
*
* @param callback $callback The function to be called
* @param string $stage The stage of the build process that
* this should be added to. Possible values:
* 'preprocess', '' for the main build process,
* 'postprocess'
*
* @return void
*
* @since 1.5
*/
public function attachBuildRule($callback)
public function attachBuildRule($callback, $stage = self::PROCESS_DURING)
{
$this->_rules['build'][] = $callback;
if (!array_key_exists('build' . $stage, $this->_rules))
{
throw new InvalidArgumentException(sprintf('The %s stage is not registered. (%s)', $stage, __METHOD__));
}

$this->_rules['build' . $stage][] = $callback;
}

/**
* Attach a parse rule
*
* @param callback $callback The function to be called.
* @param string $stage The stage of the parse process that
* this should be added to. Possible values:
* 'preprocess', '' for the main parse process,
* 'postprocess'
*
* @return void
*
* @since 1.5
*/
public function attachParseRule($callback)
public function attachParseRule($callback, $stage = self::PROCESS_DURING)
{
$this->_rules['parse'][] = $callback;
if (!array_key_exists('parse' . $stage, $this->_rules))
{
throw new InvalidArgumentException(sprintf('The %s stage is not registered. (%s)', $stage, __METHOD__));
}

$this->_rules['parse' . $stage][] = $callback;
}

/**
Expand All @@ -370,7 +419,7 @@ public function attachParseRule($callback)
* @return boolean
*
* @since 1.5
* @deprecated 4.0 Use parseRawRoute() instead
* @deprecated 4.0 Attach your logic as rule to the main parse stage
*/
protected function _parseRawRoute(&$uri)
{
Expand All @@ -385,6 +434,7 @@ protected function _parseRawRoute(&$uri)
* @return array Array of variables
*
* @since 3.2
* @deprecated 4.0 Attach your logic as rule to the main parse stage
*/
protected function parseRawRoute(&$uri)
{
Expand All @@ -399,7 +449,7 @@ protected function parseRawRoute(&$uri)
* @return string Internal URI
*
* @since 1.5
* @deprecated 4.0 Use parseSefRoute() instead
* @deprecated 4.0 Attach your logic as rule to the main parse stage
*/
protected function _parseSefRoute(&$uri)
{
Expand All @@ -414,6 +464,7 @@ protected function _parseSefRoute(&$uri)
* @return array Array of variables
*
* @since 3.2
* @deprecated 4.0 Attach your logic as rule to the main parse stage
*/
protected function parseSefRoute(&$uri)
{
Expand All @@ -428,7 +479,7 @@ protected function parseSefRoute(&$uri)
* @return string Raw Route
*
* @since 1.5
* @deprecated 4.0 Use buildRawRoute() instead
* @deprecated 4.0 Attach your logic as rule to the main build stage
*/
protected function _buildRawRoute(&$uri)
{
Expand All @@ -443,6 +494,7 @@ protected function _buildRawRoute(&$uri)
* @return string Raw Route
*
* @since 3.2
* @deprecated 4.0 Attach your logic as rule to the main build stage
*/
protected function buildRawRoute(&$uri)
{
Expand All @@ -456,7 +508,7 @@ protected function buildRawRoute(&$uri)
* @return string The SEF route
*
* @since 1.5
* @deprecated 4.0 Use buildSefRoute() instead
* @deprecated 4.0 Attach your logic as rule to the main build stage
*/
protected function _buildSefRoute(&$uri)
{
Expand All @@ -471,6 +523,7 @@ protected function _buildSefRoute(&$uri)
* @return string The SEF route
*
* @since 3.2
* @deprecated 4.0 Attach your logic as rule to the main build stage
*/
protected function buildSefRoute(&$uri)
{
Expand All @@ -494,17 +547,25 @@ protected function _processParseRules(&$uri)
/**
* Process the parsed router variables based on custom defined rules
*
* @param JUri &$uri The URI to parse
* @param JUri &$uri The URI to parse
* @param string $stage The stage that should be processed.
* Possible values: 'preprocess', 'postprocess'
* and '' for the main parse stage
Copy link
Contributor

Choose a reason for hiding this comment

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

I know we state possible values here in the doc block. But in an attempt to stop stupid people from doing things here, can we add an explicit check for these 3 variables and call an InvalidArgumentException otherwise please?

Copy link
Member Author

Choose a reason for hiding this comment

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

I decided against that, since you can basically extend the current router and have more than those 3 stages if your application needs it. Basically, you could define your own stage or rule group here. If you still feel like this should be checked against, I will put it in.

Copy link
Contributor

Choose a reason for hiding this comment

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

Would there be a way to evaluate the $stage against the rules array? That would accomplish catching an error condition and still allowing extra stages.

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the chance of that being genuinely useful vs. people abusing it to hack stuff together?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right, lets simply check if there is an array that is named in such a way and otherwise throw an Exception. How should that exception look? Anything special?

Copy link
Contributor

Choose a reason for hiding this comment

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

if (!array_key_exists('parse' . $stage, $this->_rules)) {
    throw new InvalidArgumentException(sprintf('The %s stage is not registered.', $stage));
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Like that?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me, just need to get the code style right.

*
* @return array The array of processed URI variables
*
* @since 3.2
*/
protected function processParseRules(&$uri)
protected function processParseRules(&$uri, $stage = self::PROCESS_DURING)
{
if (!array_key_exists('parse' . $stage, $this->_rules))
{
throw new InvalidArgumentException(sprintf('The %s stage is not registered. (%s)', $stage, __METHOD__));
}

$vars = array();

foreach ($this->_rules['parse'] as $rule)
foreach ($this->_rules['parse' . $stage] as $rule)
{
$vars += call_user_func_array($rule, array(&$this, &$uri));
}
Expand All @@ -530,15 +591,23 @@ protected function _processBuildRules(&$uri)
/**
* Process the build uri query data based on custom defined rules
*
* @param JUri &$uri The URI
* @param JUri &$uri The URI
* @param string $stage The stage that should be processed.
* Possible values: 'preprocess', 'postprocess'
* and '' for the main build stage
*
* @return void
*
* @since 3.2
*/
protected function processBuildRules(&$uri)
protected function processBuildRules(&$uri, $stage = self::PROCESS_DURING)
{
foreach ($this->_rules['build'] as $rule)
if (!array_key_exists('build' . $stage, $this->_rules))
{
throw new InvalidArgumentException(sprintf('The %s stage is not registered. (%s)', $stage, __METHOD__));
}

foreach ($this->_rules['build' . $stage] as $rule)
{
call_user_func_array($rule, array(&$this, &$uri));
}
Expand Down
Loading