Controller - Code Metrics - Inspection of "MINOR: Fix docblocks." - sminnee/silverstripe-framework - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — namespace-model ( 476d0e...5d15d8 )

by Sam

created 2016-05-03 10:25 UTC

Controller D

↳ Parent: Project

Complexity

Total Complexity

104

Size/Duplication

Total Lines	670
Duplicated Lines	2.39 %

Coupling/Cohesion

Components	1
Dependencies	11

Importance

Changes	2
Bugs	2	Features	0

Metric	Value
wmc	104
c	2
b	2
f	0
lcom	1
cbo	11
dl	16
loc	670
rs	4.1641

30 Methods

Rating	Name	Duplication	Size	Complexity
A	init()	0	6	2
A	Link()	0	3	1
B	handleRequest()	4	56	9
C	handleAction()	0	34	8
A	setURLParams()	0	3	1
A	getURLParams()	0	3	1
A	getResponse()	0	6	2
A	setResponse()	0	4	1
A	getFormOwner()	0	14	3
A	defaultAction()	0	3	1
A	getAction()	0	3	1
D	getViewer()	8	36	10
A	hasAction()	0	3	2
A	removeAction()	0	10	3
A	definingClassForAction()	0	12	4
A	hasActionTemplate()	4	13	3
A	render()	0	10	3
A	disableBasicAuth()	0	3	1
A	curr()	0	7	2
A	has_curr()	0	3	2
A	can()	0	12	4
A	pushCurrent()	0	11	3
A	popCurrent()	0	8	2
B	redirect()	0	15	8
C	redirectBack()	0	30	8
A	redirectedTo()	0	3	2
A	getSession()	0	3	1
A	setSession()	0	3	1
C	join_links()	0	30	14
A	get_template_global_variables()	0	5	1

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like Controller often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use Controller, and based on these observations, apply Extract Interface, too.

<?php

namespace SilverStripe\Control;

/**
 * Controllers are the cornerstone of all site functionality in SilverStripe. The {@link Director}
 * selects a controller to pass control to, and then calls {@link run()}. This method will execute
 * the appropriate action - either by calling the action method, or displaying the action's template.
 *
 * See {@link getTemplate()} for information on how the template is chosen.
 *
 * @package framework
 *
 * @subpackage control
 */

use SilverStripe\Model\DataModel;
use BasicAuth;


use Debug;
use Object;
use SSViewer;
use Member;
use Injector;
use TemplateGlobalProvider;
use SilverStripe\Control\HTTPRequest;
use SilverStripe\Control\HTTPResponse;


class Controller extends RequestHandler implements TemplateGlobalProvider {

	/**
	 * An array of arguments extracted from the URL.
	 *
	 * @var array
	 */
	protected $urlParams;

	/**
	 * Contains all GET and POST parameters passed to the current {@link SS_HTTPRequest}.
	 *
	 * @var array
	 */
	protected $requestParams;

	/**
	 * The URL part matched on the current controller as determined by the "$Action" part of the
	 * {@link $url_handlers} definition. Should correlate to a public method on this controller.
	 *
	 * Used in {@link render()} and {@link getViewer()} to determine action-specific templates.
	 *
	 * @var string
	 */
	protected $action;

	/**
	 * The {@link Session} object for this controller.
	 *
	 * @var Session
	 */
	protected $session;

	/**
	 * Stack of current controllers. Controller::$controller_stack[0] is the current controller.
	 *
	 * @var array
	 */
	protected static $controller_stack = array();

	/**
	 * @var bool
	 */
	protected $basicAuthEnabled = true;

	/**
	 * The response object that the controller returns.
	 *
	 * Set in {@link handleRequest()}.
	 *
	 * @var SS_HTTPResponse
	 */
	protected $response;

	/**
	 * Default URL handlers.
	 *
	 * @var array
	 */
	private static $url_handlers = array(

		'$Action//$ID/$OtherID' => 'handleAction',
	);

	/**
	 * @var array
	 */
	private static $allowed_actions = array(

		'handleAction',
		'handleIndex',
	);

	/**
	 * Initialisation function that is run before any action on the controller is called.
	 *
	 * @uses BasicAuth::requireLogin()
	 */
	public function init() {
		if($this->basicAuthEnabled) BasicAuth::protect_site_if_necessary();

		// This is used to test that subordinate controllers are actually calling parent::init() - a common bug
		$this->baseInitCalled = true;
	}

	/**
	 * Returns a link to this controller. Overload with your own Link rules if they exist.
	 *
	 * @return string
	 */
	public function Link() {
		return get_class($this) .'/';
	}

	/**
	 * Executes this controller, and return an {@link SS_HTTPResponse} object with the result.
	 *
	 * This method first does a few set-up activities:
	 * - Push this controller ont to the controller stack - see {@link Controller::curr()} for
	 *   information about this.
	 * - Call {@link init()}
	 * - Defer to {@link RequestHandler->handleRequest()} to determine which action should be executed.
	 *
	 * Note: $requestParams['executeForm'] support was removed, make the following change in your URLs:
	 * "/?executeForm=FooBar" -> "/FooBar".
	 *
	 * Also make sure "FooBar" is in the $allowed_actions of your controller class.
	 *
	 * Note: You should rarely need to overload run() - this kind of change is only really appropriate
	 * for things like nested controllers - {@link ModelAsController} and {@link RootURLController}
	 * are two examples here. If you want to make more orthodox functionality, it's better to overload
	 * {@link init()} or {@link index()}.
	 *
	 * Important: If you are going to overload handleRequest, make sure that you start the method with
	 * $this->pushCurrent() and end the method with $this->popCurrent(). Failure to do this will create
	 * weird session errors.
	 *
	 * @param SS_HTTPRequest $request
	 * @param DataModel $model
	 *
	 * @return HTTPResponse
	 */
	public function handleRequest(HTTPRequest $request, DataModel $model) {
		if(!$request) user_error("Controller::handleRequest() not passed a request!", E_USER_ERROR);

		$this->pushCurrent();
		$this->urlParams = $request->allParams();
		$this->setRequest($request);
		$this->getResponse();
		$this->setDataModel($model);

		$this->extend('onBeforeInit');

		// Init
		$this->baseInitCalled = false;
		$this->init();
		if(!$this->baseInitCalled) {
			user_error("init() method on class '$this->class' doesn't call Controller::init()."
				. "Make sure that you have parent::init() included.", E_USER_WARNING);
		}

		$this->extend('onAfterInit');

		$response = $this->getResponse();

		// If we had a redirection or something, halt processing.
		if($response->isFinished()) {
			$this->popCurrent();
			return $response;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
		}

		$body = parent::handleRequest($request, $model);
		if($body instanceof HTTPResponse) {
			if(isset($_REQUEST['debug_request'])) {
				Debug::message("Request handler returned SS_HTTPResponse object to $this->class controller;"
					. "returning it without modification.");
			}
			$response = $body;
			$this->setResponse($response);

		} else {
			if($body instanceof Object && $body->hasMethod('getViewer')) {
				if(isset($_REQUEST['debug_request'])) {

					Debug::message("Request handler $body->class object to $this->class controller;"
						. "rendering with template returned by $body->class::getViewer()");
				}
				$body = $body->getViewer($this->getAction())->process($body);
			}

			$response->setBody($body);
		}


		ContentNegotiator::process($response);
		HTTP::add_cache_headers($response);
/**
 * @return array|string
 */
function returnsDifferentValues($x) {
    if ($x) {
        return 'foo';
    }

    return array();
}

$x = returnsDifferentValues($y);
if (is_array($x)) {
    // $x is an array.
}

		$this->popCurrent();
		return $response;
	}

	/**
	 * Controller's default action handler.  It will call the method named in "$Action", if that method
	 * exists. If "$Action" isn't given, it will use "index" as a default.
	 *
	 * @param SS_HTTPRequest $request
	 * @param string $action
	 *
	 * @return HTMLText|SS_HTTPResponse
	 */
	protected function handleAction($request, $action) {
		foreach($request->latestParams() as $k => $v) {
			if($v || !isset($this->urlParams[$k])) $this->urlParams[$k] = $v;
		}

		$this->action = $action;
		$this->requestParams = $request->requestVars();

		if($this->hasMethod($action)) {
			$result = parent::handleAction($request, $action);

			// If the action returns an array, customise with it before rendering the template.
			if(is_array($result)) {
				return $this->getViewer($action)->process($this->customise($result));
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
			} else {
				return $result;
class Author {
    private $name;

    public function __construct($name) {
        $this->name = $name;
    }

    public function getName() {
        return $this->name;
    }
}

abstract class Post {
    public function getAuthor() {
        return 'Johannes';
    }
}

class BlogPost extends Post {
    public function getAuthor() {
        return new Author('Johannes');
    }
}

class ForumPost extends Post { /* ... */ }

function my_function(Post $post) {
    echo strtoupper($post->getAuthor());
}
			}
		}

		// Fall back to index action with before/after handlers
		$beforeResult = $this->extend('beforeCallActionHandler', $request, $action);
		if ($beforeResult) {

			return reset($beforeResult);
		}

		$result = $this->getViewer($action)->process($this);

		$afterResult = $this->extend('afterCallActionHandler', $request, $action, $result);
		if($afterResult) {

			return reset($afterResult);
		}

		return $result;
	}

	/**
	 * @param array $urlParams
	 */
	public function setURLParams($urlParams) {
		$this->urlParams = $urlParams;
	}

	/**
	 * Returns the parameters extracted from the URL by the {@link Director}.
	 *
	 * @return array
	 */
	public function getURLParams() {
		return $this->urlParams;
	}

	/**
	 * Returns the SS_HTTPResponse object that this controller is building up. Can be used to set the
	 * status code and headers.
	 *
	 * @return HTTPResponse
	 */
	public function getResponse() {
		if (!$this->response) {
			$this->setResponse(new HTTPResponse());
		}
		return $this->response;
	}

	/**
	 * Sets the SS_HTTPResponse object that this controller is building up.
	 *
	 * @param SS_HTTPResponse $response
	 *
	 * @return $this
	 */
	public function setResponse(HTTPResponse $response) {
		$this->response = $response;

		return $this;
	}

	/**
	 * @var bool
	 */
	protected $baseInitCalled = false;

	/**
	 * Return the object that is going to own a form that's being processed, and handle its execution.
	 * Note that the result need not be an actual controller object.
	 *
	 * @return mixed
	 */
	public function getFormOwner() {
		// Get the appropriate controller: sometimes we want to get a form from another controller
		if(isset($this->requestParams['formController'])) {
			$formController = Director::getControllerForURL($this->requestParams['formController']);

			while(is_a($formController, 'NestedController')) {
				$formController = $formController->getNestedController();
			}
			return $formController;

		} else {
			return $this;
		}
	}

	/**
	 * This is the default action handler used if a method doesn't exist. It will process the
	 * controller object with the template returned by {@link getViewer()}.
	 *
	 * @param string $action
	 *
	 * @return HTMLText
	 */
	public function defaultAction($action) {
		return $this->getViewer($action)->process($this);
	}

	/**
	 * Returns the action that is being executed on this controller.
	 *
	 * @return string
	 */
	public function getAction() {
		return $this->action;
	}

	/**
	 * Return the viewer identified being the default handler for this Controller/Action combination.
	 *
	 * @param string $action
	 *
	 * @return SSViewer
	 */
	public function getViewer($action) {
		// Hard-coded templates
		if(isset($this->templates[$action]) && $this->templates[$action]) {
			$templates = $this->templates[$action];

		}	else if(isset($this->templates['index']) && $this->templates['index']) {
			$templates = $this->templates['index'];

		}	else if($this->template) {
			$templates = $this->template;
<?php

/**
 * @property int $x
 * @property int $y
 * @property string $text
 */
class MyLabel
{
    private $properties;

    private $allowedProperties = array('x', 'y', 'text');

    public function __get($name)
    {
        if (isset($properties[$name]) && in_array($name, $this->allowedProperties)) {
            return $properties[$name];
        } else {
            return null;
        }
    }

    public function __set($name, $value)
    {
        if (in_array($name, $this->allowedProperties)) {
            $properties[$name] = $value;
        } else {
            throw new \LogicException("Property $name is not defined.");
        }
    }

}
		} else {
			// Add action-specific templates for inheritance chain
			$templates = array();
			$parentClass = $this->class;
$myVar = 'Value';
$higher = false;

if (rand(1, 6) > 3) {
    $higher = true;
} else {
    $higher = false;
}
			if($action && $action != 'index') {
				$parentClass = $this->class;
				while($parentClass != 'SilverStripe\Control\Controller') {

					$templates[] = strtok($parentClass,'_') . '_' . $action;
					$parentClass = get_parent_class($parentClass);
				}
			}
			// Add controller templates for inheritance chain
			$parentClass = $this->class;
			while($parentClass != 'SilverStripe\Control\Controller') {

				$templates[] = strtok($parentClass,'_');
				$parentClass = get_parent_class($parentClass);
			}

			$templates[] = 'SilverStripe\Control\Controller';

			// remove duplicates
			$templates = array_unique($templates);
		}

		return new SSViewer($templates);
	}

	/**
	 * @param string $action
	 *
	 * @return bool
	 */
	public function hasAction($action) {
		return parent::hasAction($action) || $this->hasActionTemplate($action);
	}

	/**
	 * Removes all the "action" part of the current URL and returns the result. If no action parameter
	 * is present, returns the full URL.
	 *
	 * @param string $fullURL
	 * @param null|string $action
	 *
	 * @return string
	 */
	public function removeAction($fullURL, $action = null) {
		if (!$action) $action = $this->getAction();    //default to current action
''   == false // true
''   == null  // true
'ab' == false // false
'ab' == null  // false

// It is often better to use strict comparison
'' === false // false
'' === null  // false
		$returnURL = $fullURL;

		if (($pos = strpos($fullURL, $action)) !== false) {
			$returnURL = substr($fullURL,0,$pos);
		}

		return $returnURL;
	}

	/**
	 * Return the class that defines the given action, so that we know where to check allowed_actions.
	 * Overrides RequestHandler to also look at defined templates.
	 *
	 * @param string $action
	 *
	 * @return string
	 */
	protected function definingClassForAction($action) {
		$definingClass = parent::definingClassForAction($action);
		if($definingClass) return $definingClass;

		$class = get_class($this);
		while($class != 'SilverStripe\Control\RequestHandler') {
			$templateName = strtok($class, '_') . '_' . $action;
			if(SSViewer::hasTemplate($templateName)) return $class;
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);

			$class = get_parent_class($class);
		}
	}

	/**
	 * Returns TRUE if this controller has a template that is specifically designed to handle a
	 * specific action.
	 *
	 * @param string $action
	 *
	 * @return bool
	 */
	public function hasActionTemplate($action) {
		if(isset($this->templates[$action])) return true;

		$parentClass = $this->class;
		$templates   = array();

		while($parentClass != 'SilverStripe\Control\Controller') {

			$templates[] = strtok($parentClass, '_') . '_' . $action;
			$parentClass = get_parent_class($parentClass);
		}

		return SSViewer::hasTemplate($templates);
	}

	/**
	 * Render the current controller with the templates determined by {@link getViewer()}.
	 *
	 * @param array $params
	 *
	 * @return string
	 */
	public function render($params = null) {
		$template = $this->getViewer($this->getAction());

		// if the object is already customised (e.g. through Controller->run()), use it
		$obj = ($this->customisedObj) ? $this->customisedObj : $this;


		if($params) $obj = $this->customise($params);

		return $template->process($obj);
	}

	/**
	 * Call this to disable site-wide basic authentication for a specific controller. This must be
	 * called before Controller::init(). That is, you must call it in your controller's init method
	 * before it calls parent::init().
	 */
	public function disableBasicAuth() {
		$this->basicAuthEnabled = false;
	}

	/**
	 * Returns the current controller.
	 *
	 * @return Controller
	 */
	public static function curr() {
		if(Controller::$controller_stack) {

			return Controller::$controller_stack[0];
		} else {
			user_error("No current controller available", E_USER_WARNING);
		}
	}

	/**
	 * Tests whether we have a currently active controller or not. True if there is at least 1
	 * controller in the stack.
	 *
	 * @return bool
	 */
	public static function has_curr() {
		return Controller::$controller_stack ? true : false;
	}

	/**
	 * Returns true if the member is allowed to do the given action. Defaults to the currently logged
	 * in user.
	 *
	 * @param string $perm
	 * @param null|member $member
	 *
	 * @return bool
	 */
	public function can($perm, $member = null) {
		if(!$member) $member = Member::currentUser();
		if(is_array($perm)) {
			$perm = array_map(array($this, 'can'), $perm, array_fill(0, count($perm), $member));
			return min($perm);
		}
		if($this->hasMethod($methodName = 'can' . $perm)) {
			return $this->$methodName($member);
		} else {
			return true;
		}
	}

	/**
	 * Pushes this controller onto the stack of current controllers. This means that any redirection,
	 * session setting, or other things that rely on Controller::curr() will now write to this
	 * controller object.
	 */
	public function pushCurrent() {
		array_unshift(self::$controller_stack, $this);
		// Create a new session object
		if(!$this->session) {
			if(isset(self::$controller_stack[1])) {
				$this->session = self::$controller_stack[1]->getSession();
			} else {
				$this->session = Injector::inst()->create('SilverStripe\Control\Session', array());
			}
		}
	}

	/**
	 * Pop this controller off the top of the stack.
	 */
	public function popCurrent() {
		if($this === self::$controller_stack[0]) {
			array_shift(self::$controller_stack);
		} else {
			user_error("popCurrent called on $this->class controller, but it wasn't at the top of the stack",
				E_USER_WARNING);
		}
	}

	/**
	 * Redirect to the given URL.
	 *
	 * @param string $url
	 * @param int $code
	 *
	 * @return HTTPResponse
	 */
	public function redirect($url, $code=302) {

		if($this->getResponse()->getHeader('Location') && $this->getResponse()->getHeader('Location') != $url) {
			user_error("Already directed to " . $this->getResponse()->getHeader('Location')
				. "; now trying to direct to $url", E_USER_WARNING);
			return;
		}

		// Attach site-root to relative links, if they have a slash in them
		if($url=="" || $url[0]=='?' || (substr($url,0,4) != "http" && $url[0] != "/" && strpos($url,'/') !== false)) {
			$url = Director::baseURL() . $url;
		}

		return $this->getResponse()->redirect($url, $code);
	}

	/**
	 * Redirect back. Uses either the HTTP-Referer or a manually set request-variable called "BackURL".
	 * This variable is needed in scenarios where HTTP-Referer is not sent (e.g when calling a page by
	 * location.href in IE). If none of the two variables is available, it will redirect to the base
	 * URL (see {@link Director::baseURL()}).
	 *
	 * @uses redirect()
	 *
	 * @return bool|SS_HTTPResponse
	 */
	public function redirectBack() {
		// Don't cache the redirect back ever
		HTTP::set_cache_age(0);

		$url = null;

		// In edge-cases, this will be called outside of a handleRequest() context; in that case,
		// redirect to the homepage - don't break into the global state at this stage because we'll
		// be calling from a test context or something else where the global state is inappropraite
		if($this->getRequest()) {
			if($this->getRequest()->requestVar('BackURL')) {
				$url = $this->getRequest()->requestVar('BackURL');
			} else if($this->getRequest()->isAjax() && $this->getRequest()->getHeader('X-Backurl')) {
				$url = $this->getRequest()->getHeader('X-Backurl');
			} else if($this->getRequest()->getHeader('Referer')) {
				$url = $this->getRequest()->getHeader('Referer');
			}
		}

		if(!$url) $url = Director::baseURL();

		// absolute redirection URLs not located on this site may cause phishing
		if(Director::is_site_url($url)) {
			$url = Director::absoluteURL($url, true);
function acceptsInteger($int) { }

$x = '123'; // string "123"

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
			return $this->redirect($url);
<?php

function getDate($date)
{
    if ($date !== null) {
        return new DateTime($date);
    }

    return false;
}
		} else {
			return false;
		}

	}

	/**
	 * Tests whether a redirection has been requested. If redirect() has been called, it will return
	 * the URL redirected to. Otherwise, it will return null.
	 *
	 * @return null|string
	 */
	public function redirectedTo() {
		return $this->getResponse() && $this->getResponse()->getHeader('Location');
	}

	/**
	 * Get the Session object representing this Controller's session.
	 *
	 * @return Session
	 */
	public function getSession() {
		return $this->session;
	}

	/**
	 * Set the Session object.
	 *
	 * @param Session $session
	 */
	public function setSession(Session $session) {
		$this->session = $session;
	}

	/**
	 * Joins two or more link segments together, putting a slash between them if necessary. Use this
	 * for building the results of {@link Link()} methods. If either of the links have query strings,
	 * then they will be combined and put at the end of the resulting url.
	 *
	 * Caution: All parameters are expected to be URI-encoded already.
	 *
	 * @param string
	 *
	 * @return string
	 */
	public static function join_links() {
		$args = func_get_args();
		$result = "";
		$queryargs = array();
		$fragmentIdentifier = null;

		foreach($args as $arg) {
			// Find fragment identifier - keep the last one
			if(strpos($arg,'#') !== false) {
				list($arg, $fragmentIdentifier) = explode('#',$arg,2);
			}
			// Find querystrings
			if(strpos($arg,'?') !== false) {
				list($arg, $suffix) = explode('?',$arg,2);
				parse_str($suffix, $localargs);
				$queryargs = array_merge($queryargs, $localargs);
			}
			if((is_string($arg) && $arg) || is_numeric($arg)) {
				$arg = (string) $arg;
				if($result && substr($result,-1) != '/' && $arg[0] != '/') $result .= "/$arg";
				else $result .= (substr($result, -1) == '/' && $arg[0] == '/') ? ltrim($arg, '/') : $arg;
			}
		}

		if($queryargs) $result .= '?' . http_build_query($queryargs);


		if($fragmentIdentifier) $result .= "#$fragmentIdentifier";

		return $result;
	}

	/**
	 * @return array
	 */
	public static function get_template_global_variables() {
		return array(
			'CurrentPage' => 'curr',
		);
	}
}




sminnee / silverstripe-framework