Permalink
Browse files

Added show_error on Controller not found - loading always returns res…

…ult, and removed unit test coverage comments for better Travis analysis

Signed-off-by: dchill42 <dchill42@gmail.com>
  • Loading branch information...
1 parent 1eeebdb commit 68343f8a5813ab49130319bd3508015183294626 @dchill42 committed Sep 30, 2012
@@ -491,65 +491,54 @@ public function is_callable($class, $method)
* @param string Method
* @param array Optional arguments
* @param string Optional object name
- * @param bool TRUE to return call result (or NULL if failed)
- * @return bool TRUE or call result on success, otherwise FALSE or NULL (if $return == TRUE)
+ * @param bool TRUE to return call success/failure
+ * @return mixed Called method return value
*/
- public function call_controller($class, $method, array $args = array(), $name = '', $return = FALSE)
+ public function call_controller($class, $method, array $args = array(), $name = '', $status = FALSE)
{
// Default name if not provided
if (empty($name))
{
$name = strtolower($class);
}
- // Track whether it ran and what it returned
- $ran = FALSE;
- $result = NULL;
-
// Class must be loaded, and method cannot start with underscore, nor be a member of the base class
+ $result = NULL;
if (isset($this->$name) && strncmp($method, '_', 1) != 0 && ! $this->is_callable('CI_Controller', $method))
{
// Check for _remap
if ($this->is_callable($class, '_remap'))
{
// Call _remap
$result = $this->$name->_remap($method, $args);
- $ran = TRUE;
}
else if ($this->is_callable($class, $method))
{
// Call method
$result = call_user_func_array(array(&$this->$name, $method), $args);
- $ran = TRUE;
+ }
+ else
+ {
+ // Not callable - check for status return
+ if ($status)
+ {
+ return FALSE;
+ }
+ show_error($method.' is not a callable method of '.$class);
}
}
+ else
+ {
+ // Not valid - check for status return
+ if ($status)
+ {
+ return FALSE;
+ }
+ show_error($class.'->'.$method.' is not a valid controller method to call');
+ }
- // Return result or success status
- return $return ? $result : $ran;
- }
-
- /**
- * Call a controller method and get its output
- *
- * Requires that controller already be loaded, validates method name, and calls
- * _remap if available.
- *
- * @param string Reference to output string
- * @param string Class name
- * @param string Method
- * @param array Optional arguments
- * @param string Optional object name
- * @return bool TRUE on success, otherwise FALSE
- */
- public function get_controller_output(&$out, $class, $method, array $args = array(), $name = '')
- {
- // Capture output and call controller
- $this->output->stack_push();
- $result = $this->call_controller($class, $method, $args, $name);
- $out = $this->output->stack_pop();
-
- // Return success or failure
- return $result;
+ // Return success status or result
+ return $status ? TRUE : $result;
}
/**
@@ -722,7 +711,7 @@ protected function _run_controller()
// Call post_controller_constructor hook
$this->hooks->call_hook('post_controller_constructor');
- if ($this->call_controller($class, $method, $args) == FALSE)
+ if ($this->call_controller($class, $method, $args, '', TRUE) === FALSE)
{
// Both _remap and $method failed - go to 404
show_404($class.'/'.$method);
View
@@ -188,7 +188,7 @@ public function __construct()
// Validate each path and add to list
foreach ( (array) $paths as $path)
{
- // Groom and resolve path against includes
+ // Groom and resolve path against includes
// (get root object class for unit test override)
$ciclass = get_class($this->CI);
$path = $ciclass::resolve_path($path);
@@ -276,10 +276,10 @@ public function library($library = '', $params = NULL, $object_name = NULL)
*
* @param mixed Route to controller/method
* @param string Optional controller object name
- * @param bool TRUE to return method result, FALSE to skip calling method
- * @return bool TRUE or call result on success, otherwise FALSE or NULL (if $call == TRUE)
+ * @param bool FALSE to skip calling method
+ * @return mixed Called method return value or NULL if not called
*/
- public function controller($route, $name = NULL, $call = NULL)
+ public function controller($route, $name = NULL, $call = TRUE)
{
// Set output flag to be passed
$out = FALSE;
@@ -295,15 +295,15 @@ public function controller($route, $name = NULL, $call = NULL)
* @param string Reference to output string
* @param mixed Route to controller/method
* @param string Optional controller object name
- * @param bool TRUE to return method result, FALSE to skip calling method
- * @return bool TRUE or call result on success, otherwise FALSE or NULL (if $call == TRUE)
+ * @param bool FALSE to skip calling method
+ * @return mixed Called method return value or NULL if not called
*/
- public function controller_output(&$out, $route, $name = NULL, $call = NULL)
+ public function controller_output(&$out, $route, $name = NULL, $call = TRUE)
{
// Check for missing class
if (empty($route))
{
- return $call === TRUE ? NULL : FALSE;
+ return NULL;
}
// Get instance and establish segment stack
@@ -312,7 +312,7 @@ public function controller_output(&$out, $route, $name = NULL, $call = NULL)
// Assume segments have been pre-parsed by CI_Router::validate_route() - make sure there's 4
if (count($route) <= CI_Router::SEG_METHOD)
{
- return $call === TRUE ? NULL : FALSE;
+ show_error('Invalid route stack provided');
}
}
else
@@ -321,7 +321,7 @@ public function controller_output(&$out, $route, $name = NULL, $call = NULL)
$route = $this->CI->router->validate_route(explode('/', $route));
if ($route === FALSE)
{
- return $call === TRUE ? NULL : FALSE;
+ show_error('The route "'.$route.'" does not resolve to a valid controller');
}
}
@@ -369,22 +369,30 @@ public function controller_output(&$out, $route, $name = NULL, $call = NULL)
$this->_ci_controllers[] = $name;
}
- // Check call and output flags
+ // Check call flag
if ($call === FALSE)
{
- // Call disabled - return success
- return TRUE;
+ // Call disabled - we're done here
+ return NULL;
}
- else if ($out === FALSE)
+
+ // Check for output
+ if ($out !== FALSE)
{
- // No output - just return result or success status
- return $this->CI->call_controller($class, $method, $route, $name, (bool)$call);
+ // Push a new stack level to capture output
+ $this->CI->output->stack_push();
}
- else
+
+ // Call method and get return value
+ $result = $this->CI->call_controller($class, $method, $route, $name);
+
+ if ($out !== FALSE)
{
- // Get output and return success status
- return $this->CI->get_controller_output($out, $class, $method, $route, $name);
+ // Capture output from stack level
+ $out = $this->CI->output->stack_pop();
}
+
+ return $result;
}
// --------------------------------------------------------------------
@@ -257,7 +257,7 @@ public function test_call_controller()
$CI->$name = new $class();
// Call method
- $this->assertTrue($CI->call_controller($class, $method, array(), $name));
+ $this->assertNull($CI->call_controller($class, $method, array(), $name));
$this->assertEquals($msg, $CI->$name->message);
}
@@ -38,7 +38,7 @@ public function set_up()
/**
* Test constructor
*
- * @covers CI_Config::__construct
+ * covers CI_Config::__construct
*/
public function test_ctor()
{
@@ -59,7 +59,7 @@ public function test_ctor()
/**
* Test item retrieval
*
- * @covers CI_Config::item
+ * covers CI_Config::item
*/
public function test_item()
{
@@ -86,7 +86,7 @@ public function test_item()
/**
* Test set item
*
- * @covers CI_Config::set_item
+ * covers CI_Config::set_item
*/
public function test_set_item()
{
@@ -105,7 +105,7 @@ public function test_set_item()
/**
* Test slash item
*
- * @covers CI_Config::slash_item
+ * covers CI_Config::slash_item
*/
public function test_slash_item()
{
@@ -122,7 +122,7 @@ public function test_slash_item()
/**
* Test site url
*
- * @covers CI_Config::site_url
+ * covers CI_Config::site_url
*/
public function test_site_url()
{
@@ -160,7 +160,7 @@ public function test_site_url()
/**
* Test system url
*
- * @covers CI_Config::system_url
+ * covers CI_Config::system_url
*/
public function test_system_url()
{
@@ -171,7 +171,7 @@ public function test_system_url()
/**
* Test loading a config file
*
- * @covers CI_Config::load
+ * covers CI_Config::load
*/
public function test_load()
{
@@ -214,7 +214,7 @@ public function test_load()
/**
* Test getting config file contents
*
- * @covers CI_Config::get
+ * covers CI_Config::get
*/
public function test_get()
{
@@ -278,7 +278,7 @@ public function test_get()
/**
* Test getting config file contents with extras
*
- * @covers CI_Config::get_ext
+ * covers CI_Config::get_ext
*/
public function test_get_ext()
{
@@ -15,7 +15,7 @@ public function set_up()
*
* Make sure $this->core_object gets objects attached to the root instance
*
- * @covers CI_Controller::__get
+ * covers CI_Controller::__get
*/
public function test_get()
{
@@ -45,7 +45,7 @@ public function test_get()
/**
* Test __isset magic method
*
- * @covers CI_Controller::__isset
+ * covers CI_Controller::__isset
*/
public function test_isset()
{
@@ -64,7 +64,7 @@ public function test_isset()
/**
* Test instance method
*
- * @covers CI_Controller::instance
+ * covers CI_Controller::instance
*/
public function test_instance()
{
Oops, something went wrong.

0 comments on commit 68343f8

Please sign in to comment.