RootURLController - Code Metrics - Inspection of "Fixes relating to Controller cleanup" - silverstripe/silverstripe-cms - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#1169)

by Daniel

created 2016-01-18 22:42 UTC

RootURLController A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	149
Duplicated Lines	12.75 %

Coupling/Cohesion

Components	1
Dependencies	11

Metric	Value
wmc	28
lcom	1
cbo	11
dl	19
loc	149
rs	10

7 Methods

Rating	Name	Duplication	Size	Complexity
A	set_default_homepage_link()	0	4	1
A	get_default_homepage_link()	0	4	1
A	reset()	0	3	1
D	get_homepage_link()	0	31	10
B	should_be_on_root()	0	9	6
A	beforeHandleRequest()	15	15	4
B	handleRequest()	4	23	5

How to fix Duplicated Code

<?php
/**
 * @package cms
 * @subpackage control
 */
class RootURLController extends Controller {

	/**
	 * @var bool
	 */
	protected static $is_at_root = false;

	/**
	 * @config
	 * @var string
	 */
	private static $default_homepage_link = 'home';

	/**
	 * @var string
	 */
	protected static $cached_homepage_link;

	/**
	 * Get the full form (e.g. /home/) relative link to the home page for the current HTTP_HOST value. Note that the
	 * link is trimmed of leading and trailing slashes before returning to ensure consistency.
	 *
	 * @return string
	 */
	static public function get_homepage_link() {
		if(!self::$cached_homepage_link) {
			// @todo Move to 'homepagefordomain' module
			if(class_exists('HomepageForDomainExtension')) {
				$host       = str_replace('www.', null, $_SERVER['HTTP_HOST']);
				$candidates = SiteTree::get()->where(array(
					'"SiteTree"."HomepageForDomain" LIKE ?' => "%$host%"
				));
				if($candidates) foreach($candidates as $candidate) {
					if(preg_match('/(,|^) *' . preg_quote($host) . ' *(,|$)/', $candidate->HomepageForDomain)) {
						self::$cached_homepage_link = trim($candidate->RelativeLink(true), '/');
					}
				}
			}

			if(!self::$cached_homepage_link) {
				// TODO Move to 'translatable' module
				if (
					class_exists('Translatable')
					&& SiteTree::has_extension('Translatable')
					&& $link = Translatable::get_homepage_link_by_locale(Translatable::get_current_locale())
				) {
					self::$cached_homepage_link = $link;
				} else {
					self::$cached_homepage_link = Config::inst()->get('RootURLController', 'default_homepage_link');
				}
			}
		}

		return self::$cached_homepage_link;
	}

	/**
	 * Set the URL Segment used for your homepage when it is created by dev/build.
	 * This allows you to use home page URLs other than the default "home".
	 *
	 * @deprecated 4.0 Use the "RootURLController.default_homepage_link" config setting instead
	 * @param string $urlsegment the URL segment for your home page
	 */
	static public function set_default_homepage_link($urlsegment = "home") {
		Deprecation::notice('4.0', 'Use the "RootURLController.default_homepage_link" config setting instead');
		Config::inst()->update('RootURLController', 'default_homepage_link', $urlsegment);
	}

	/**
	 * Gets the link that denotes the homepage if there is not one explicitly defined for this HTTP_HOST value.
	 *
	 * @deprecated 4.0 Use the "RootURLController.default_homepage_link" config setting instead
	 * @return string
	 */
	static public function get_default_homepage_link() {
		Deprecation::notice('4.0', 'Use the "RootURLController.default_homepage_link" config setting instead');
		return Config::inst()->get('RootURLController', 'default_homepage_link');
	}

	/**
	 * Returns TRUE if a request to a certain page should be redirected to the site root (i.e. if the page acts as the
	 * home page).
	 *
	 * @param SiteTree $page
	 * @return bool
	 */
	static public function should_be_on_root(SiteTree $page) {
		if(!self::$is_at_root && self::get_homepage_link() == trim($page->RelativeLink(true), '/')) {
function acceptsInteger($int) { }

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

// Instead of
acceptsInteger($x);

// we recommend to use
acceptsInteger((integer) $x);
			return !(
				class_exists('Translatable') && $page->hasExtension('Translatable') && $page->Locale && $page->Locale != Translatable::default_locale()
<?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.");
        }
    }

}
			);
		}

		return false;
	}

	/**
	 * Resets the cached homepage link value - useful for testing.
	 */
	static public function reset() {
		self::$cached_homepage_link = null;
	}

	protected function beforeHandleRequest(SS_HTTPRequest $request, DataModel $model) {

		parent::beforeHandleRequest($request, $model);


		self::$is_at_root = true;

		if(!DB::is_active() || !ClassInfo::hasTable('SiteTree')) {
			$this->getResponse()->redirect(Controller::join_links(
				Director::absoluteBaseURL(),
				'dev/build',
				'?' . array(
					'returnURL' => isset($_GET['url']) ? $_GET['url'] : null,
				)
			));
		}
	}

	/**
	 * @param SS_HTTPRequest $request
	 * @param DataModel|null $model
	 * @return SS_HTTPResponse
	 */
	public function handleRequest(SS_HTTPRequest $request, DataModel $model = null) {
		self::$is_at_root = true;
		$this->beforeHandleRequest($request, $model);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}

		if (!$this->getResponse()->isFinished()) {
			if (!DB::is_active() || !ClassInfo::hasTable('SiteTree')) {

				$this->getResponse()->redirect(Director::absoluteBaseURL() . 'dev/build?returnURL=' . (isset($_GET['url']) ? urlencode($_GET['url']) : null));
				return $this->getResponse();
			}

			$request->setUrl(self::get_homepage_link() . '/');
			$request->match('$URLSegment//$Action', true);
			$controller = new ModelAsController();

			$response = $controller->handleRequest($request, $model);
function notNullable(stdClass $x) { }

// Unsafe
function withoutCheck(stdClass $x = null) {
    notNullable($x);
}

// Safe - Alternative 1: Adding Additional Type-Check
function withCheck(stdClass $x = null) {
    if ($x instanceof stdClass) {
        notNullable($x);
    }
}

// Safe - Alternative 2: Changing Parameter
function withNonNullableParam(stdClass $x) {
    notNullable($x);
}

			$this->prepareResponse($response);
class ParentClass {
    private $data = array();

    public function __call($method, array $args) {
        if (0 === strpos($method, 'get')) {
            return $this->data[strtolower(substr($method, 3))];
        }

        throw new \LogicException(sprintf('Unsupported method: %s', $method));
    }
}

/**
 * If this class knows which fields exist, you can specify the methods here:
 *
 * @method string getName()
 */
class SomeClass extends ParentClass { }
		}

		$this->afterHandleRequest();


		return $this->getResponse();
	}

}


1		<?php
2		/**
3		* @package cms
4		* @subpackage control
5		*/
6		class RootURLController extends Controller {
7
8		/**
9		* @var bool
10		*/
11		protected static $is_at_root = false;
12
13		/**
14		* @config
15		* @var string
16		*/
17		private static $default_homepage_link = 'home';
18
19		/**
20		* @var string
21		*/
22		protected static $cached_homepage_link;
23
24		/**
25		* Get the full form (e.g. /home/) relative link to the home page for the current HTTP_HOST value. Note that the
26		* link is trimmed of leading and trailing slashes before returning to ensure consistency.
27		*
28		* @return string
29		*/
30		static public function get_homepage_link() {
31		if(!self::$cached_homepage_link) {
32		// @todo Move to 'homepagefordomain' module
33		if(class_exists('HomepageForDomainExtension')) {
34		$host = str_replace('www.', null, $_SERVER['HTTP_HOST']);
35		$candidates = SiteTree::get()->where(array(
36		'"SiteTree"."HomepageForDomain" LIKE ?' => "%$host%"
37		));
38		if($candidates) foreach($candidates as $candidate) {
39		if(preg_match('/(,\|^) ' . preg_quote($host) . ' (,\|$)/', $candidate->HomepageForDomain)) {
40		self::$cached_homepage_link = trim($candidate->RelativeLink(true), '/');
41		}
42		}
43		}
44
45		if(!self::$cached_homepage_link) {
46		// TODO Move to 'translatable' module
47		if (
48		class_exists('Translatable')
49		&& SiteTree::has_extension('Translatable')
50		&& $link = Translatable::get_homepage_link_by_locale(Translatable::get_current_locale())
51		) {
52		self::$cached_homepage_link = $link;
53		} else {
54		self::$cached_homepage_link = Config::inst()->get('RootURLController', 'default_homepage_link');
55		}
56		}
57		}
58
59		return self::$cached_homepage_link;
60		}
61
62		/**
63		* Set the URL Segment used for your homepage when it is created by dev/build.
64		* This allows you to use home page URLs other than the default "home".
65		*
66		* @deprecated 4.0 Use the "RootURLController.default_homepage_link" config setting instead
67		* @param string $urlsegment the URL segment for your home page
68		*/
69		static public function set_default_homepage_link($urlsegment = "home") {
70		Deprecation::notice('4.0', 'Use the "RootURLController.default_homepage_link" config setting instead');
71		Config::inst()->update('RootURLController', 'default_homepage_link', $urlsegment);
72		}
73
74		/**
75		* Gets the link that denotes the homepage if there is not one explicitly defined for this HTTP_HOST value.
76		*
77		* @deprecated 4.0 Use the "RootURLController.default_homepage_link" config setting instead
78		* @return string
79		*/
80		static public function get_default_homepage_link() {
81		Deprecation::notice('4.0', 'Use the "RootURLController.default_homepage_link" config setting instead');
82		return Config::inst()->get('RootURLController', 'default_homepage_link');
83		}
84
85		/**
86		* Returns TRUE if a request to a certain page should be redirected to the site root (i.e. if the page acts as the
87		* home page).
88		*
89		* @param SiteTree $page
90		* @return bool
91		*/
92		static public function should_be_on_root(SiteTree $page) {
93		if(!self::$is_at_root && self::get_homepage_link() == trim($page->RelativeLink(true), '/')) {
		0 ignored issues – show Documentation introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report `true` is of type `boolean`, but the function expects a `string\|null`. It seems like the type of the argument is not accepted by the function/method which you are calling. In some cases, in particular if PHP’s automatic type-juggling kicks in this might be fine. In other cases, however this might be a bug. We suggest to add an explicit type cast like in the following example: function acceptsInteger($int) { } $x = '123'; // string "123" // Instead of acceptsInteger($x); // we recommend to use acceptsInteger((integer) $x); Loading history...
94		return !(
95		class_exists('Translatable') && $page->hasExtension('Translatable') && $page->Locale && $page->Locale != Translatable::default_locale()
		0 ignored issues – show Documentation introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report The property `Locale` does not exist on `object<SiteTree>`. Since you implemented `__get`, maybe consider adding a @property annotation. Since your code implements the magic getter `_get`, this function will be called for any read access on an undefined variable. You can add the `@property` annotation to your class or interface to document the existence of this variable. <?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."); } } } If the property has read access only, you can use the @property-read annotation instead. Of course, you may also just have mistyped another name, in which case you should fix the error. See also the PhpDoc documentation for @property. Loading history...
96		);
97		}
98
99		return false;
100		}
101
102		/**
103		* Resets the cached homepage link value - useful for testing.
104		*/
105		static public function reset() {
106		self::$cached_homepage_link = null;
107		}
108
109	View Code Duplication	protected function beforeHandleRequest(SS_HTTPRequest $request, DataModel $model) {
		0 ignored issues – show Duplication introduced 2016-01-18 22:45 UTC by Report Bug Copy Issue Report This method seems to be duplicated in your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
110		parent::beforeHandleRequest($request, $model);
		0 ignored issues – show Bug introduced 2016-01-18 22:45 UTC by Report Bug Copy Issue Report The method `beforeHandleRequest()` does not exist on `Controller`. Did you maybe mean `handleRequest()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history...
111
112		self::$is_at_root = true;
113
114		if(!DB::is_active() \|\| !ClassInfo::hasTable('SiteTree')) {
115		$this->getResponse()->redirect(Controller::join_links(
116		Director::absoluteBaseURL(),
117		'dev/build',
118		'?' . array(
119		'returnURL' => isset($_GET['url']) ? $_GET['url'] : null,
120		)
121		));
122		}
123		}
124
125		/**
126		* @param SS_HTTPRequest $request
127		* @param DataModel\|null $model
128		* @return SS_HTTPResponse
129		*/
130		public function handleRequest(SS_HTTPRequest $request, DataModel $model = null) {
131		self::$is_at_root = true;
132		$this->beforeHandleRequest($request, $model);
		0 ignored issues – show Bug introduced 2016-01-18 22:45 UTC by Report Bug Copy Issue Report It seems like `$model` defined by parameter `$model` on line `130` can be `null`; however, `RootURLController::beforeHandleRequest()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
133
134		if (!$this->getResponse()->isFinished()) {
135	View Code Duplication	if (!DB::is_active() \|\| !ClassInfo::hasTable('SiteTree')) {
		0 ignored issues – show Duplication introduced 2015-11-26 02:50 UTC by Report Bug Copy Issue Report This code seems to be duplicated across your project. Duplicated code is one of the most pungent code smells. If you need to duplicate the same code in three or more different places, we strongly encourage you to look into extracting the code into a single class or operation. You can also find more detailed suggestions in the “Code” section of your repository. Loading history...
136		$this->getResponse()->redirect(Director::absoluteBaseURL() . 'dev/build?returnURL=' . (isset($_GET['url']) ? urlencode($_GET['url']) : null));
137		return $this->getResponse();
138		}
139
140		$request->setUrl(self::get_homepage_link() . '/');
141		$request->match('$URLSegment//$Action', true);
142		$controller = new ModelAsController();
143
144		$response = $controller->handleRequest($request, $model);
		0 ignored issues – show Bug introduced 2016-01-18 22:45 UTC by Report Bug Copy Issue Report It seems like `$model` defined by parameter `$model` on line `130` can be `null`; however, `ModelAsController::handleRequest()` does not accept `null`, maybe add an additional type check? It seems like you allow that null is being passed for a parameter, however the function which is called does not seem to accept null. We recommend to add an additional type check (or disallow null for the parameter): function notNullable(stdClass $x) { } // Unsafe function withoutCheck(stdClass $x = null) { notNullable($x); } // Safe - Alternative 1: Adding Additional Type-Check function withCheck(stdClass $x = null) { if ($x instanceof stdClass) { notNullable($x); } } // Safe - Alternative 2: Changing Parameter function withNonNullableParam(stdClass $x) { notNullable($x); } Loading history...
145
146		$this->prepareResponse($response);
		0 ignored issues – show Documentation Bug introduced 2016-01-18 22:45 UTC by Report Bug Copy Issue Report The method `prepareResponse` does not exist on `object<RootURLController>`? Since you implemented `__call`, maybe consider adding a @method annotation. If you implement `__call` and you know which methods are available, you can improve IDE auto-completion and static analysis by adding a @method annotation to the class. This is often the case, when `__call` is implemented by a parent class and only the child class knows which methods exist: class ParentClass { private $data = array(); public function __call($method, array $args) { if (0 === strpos($method, 'get')) { return $this->data[strtolower(substr($method, 3))]; } throw new \LogicException(sprintf('Unsupported method: %s', $method)); } } /** * If this class knows which fields exist, you can specify the methods here: * * @method string getName() */ class SomeClass extends ParentClass { } Loading history...
147		}
148
149		$this->afterHandleRequest();
		0 ignored issues – show Bug introduced 2016-01-18 22:45 UTC by Report Bug Copy Issue Report The method `afterHandleRequest()` does not exist on `RootURLController`. Did you maybe mean `handleRequest()`? This check marks calls to methods that do not seem to exist on an object. This is most likely the result of a method being renamed without all references to it being renamed likewise. Loading history...
150
151		return $this->getResponse();
152		}
153
154		}
155

silverstripe / silverstripe-cms

Pull Request — master (#1169)

RootURLController A

Complexity

Size/Duplication

Coupling/Cohesion

7 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like