BaseRestController::handleAction() - Code Metrics - Inspection of "New release 1.0.0" - notthatbad/silverstripe-rest-api - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#38)

by Christian

created 2016-08-19 23:17 UTC

BaseRestController::handleAction() D

↳ Parent: BaseRestController

Complexity

Conditions	13
Paths	132

Size

Total Lines	91
Code Lines	61

Duplication

Lines	14
Ratio	15.38 %

Importance

Changes	3
Bugs	1	Features	2

Metric	Value
c	3
b	1
f	2
dl	14
loc	91
rs	4.6605
cc	13
eloc	61
nc	132
nop	2

How to fix Long Method Complexity

<?php

namespace Ntb\RestAPI;

use Director;
use Exception;
use HTMLText;
use Member;
use SS_HTTPRequest;
use SS_HTTPResponse;
use SS_Log;

/**
 * Base class for the rest resource controllers.
 * @author Christian Blank <[email protected]>
 */
abstract class BaseRestController extends \Controller {

    private static $allowed_actions = array (

        'options' => true,
        'head' => true
    );

    /**
     * Configuration option.
     * If set to true, only https connections will be processed.
     * @var bool
     */
    private static $https_only = true;


    /**
     *
     */
    public function init() {
        parent::init();
        // check for https
        if($this->config()->https_only && !Director::is_https()) {
            $response = $this->getResponse();
            $response->setStatusCode('403', 'http request not allowed');
            $response->setBody("Request over HTTP is not allowed. Please switch to https.");
            $response->output();
            exit;
        }
        // check for CORS options request
        if ($this->request->httpMethod() === 'OPTIONS' ) {
            // create direct response without requesting any controller
            $response = $this->getResponse();
            // set CORS header from config
            $response = $this->addCORSHeaders($response);
            $response->output();
            exit;
        }
    }

    /**
     * @param SS_HTTPRequest $request
     * @return null

     * @throws RestUserException
     */
    public function head(SS_HTTPRequest $request) {
        if(method_exists($this, 'get')) {
            $result = $this->get($request);
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 { }
            if($result instanceof SS_HTTPResponse) {
                $result->setBody(null);
                return $result;
            }
            return null;
        }
        throw new RestUserException("Endpoint doesn't have a GET implementation", 404);
    }

    /**
     * handleAction implementation for rest controllers. This handles the requested action differently then the standard
     * implementation.
     *
     * @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;
        }
        // set the action to the request method / for developing we could use an additional parameter to choose another method
        $action = $this->getMethodName($request);
        $this->action = $action;
        $this->requestParams = $request->requestVars();
function aContainsB(array $needle = null, array  $haystack) {
    if (!$needle) {
        return false;
    }

    return array_intersect($haystack, $needle) == $haystack;
}
        $className = $this->class;
        // create serializer
        $serializer = SerializerFactory::create_from_request($request);
        $response = $this->getResponse();
        // perform action
        try {
            if(!$this->hasAction($action)) {
                // method couldn't found on controller
                throw new RestUserException("Action '$action' isn't available on class $className.", 404);
            }
            if(!$this->checkAccessAction($action)) {
                throw new RestUserException("Action '$action' isn't allowed on class $className.", 404, 401);
            }
            $actionResult = null;
            if(method_exists($this, 'beforeCallActionHandler')) {
                // call before action hook
                $actionResult = $this->beforeCallActionHandler($request, $action);
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 { }
            }
            // if action hook contains data it will be used as result, otherwise the action handler will be called
            if(!$actionResult) {
                // perform action
                $actionResult = $this->$action($request);
            }
            $body = $actionResult;
        } catch(RestUserException $ex) {
            // a user exception was caught
            $response->setStatusCode($ex->getHttpStatusCode());
            $body = [
                'message' => $ex->getMessage(),
                'code' => $ex->getCode()
            ];
            // log all data
            SS_Log::log(
                json_encode(array_merge($body, ['file' => $ex->getFile(), 'line' => $ex->getLine()])),
                SS_Log::INFO);
        } catch(RestSystemException $ex) {
            // a system exception was caught
            $response->addHeader('Content-Type', $serializer->contentType());
            $response->setStatusCode($ex->getHttpStatusCode());
            $body = [
                'message' => $ex->getMessage(),
                'code' => $ex->getCode()
            ];
            if(Director::isDev()) {

                $body = array_merge($body, [
                    'file' => $ex->getFile(),
                    'line' => $ex->getLine(),
                    'trace' => $ex->getTrace()
                ]);
            }
            // log all data
            SS_Log::log(
                json_encode(array_merge($body, ['file' => $ex->getFile(), 'line' => $ex->getLine()])),
                SS_Log::WARN);
        } catch(Exception $ex) {
            // an unexpected exception was caught
            $response->addHeader('Content-Type', $serializer->contentType());
            $response->setStatusCode("500");
            $body = [
                'message' => $ex->getMessage(),
                'code' => $ex->getCode()
            ];
            if(Director::isDev()) {

                $body = array_merge($body, [
                    'file' => $ex->getFile(),
                    'line' => $ex->getLine(),
                    'trace' => $ex->getTrace()
                ]);
            }
            // log all data and the trace to get a better understanding of the exception
            SS_Log::log(
                json_encode(array_merge(
                    $body, ['file' => $ex->getFile(), 'line' => $ex->getLine(),'trace' => $ex->getTrace()])),
                SS_Log::ERR);
        }
        // serialize content and set body of response
        $response->addHeader('Content-Type', $serializer->contentType());
        // TODO: body could be an exception; check it before the response is generated
        $response->setBody($serializer->serialize($body));
        // set CORS header from config
        $response = $this->addCORSHeaders($response);
        return $response;
    }

    /**
     * Returns the http method for this request. If the current environment is a development env, the method can be
     * changed with a `method` variable.
     *
     * @param \SS_HTTPRequest $request the current request
     * @return string the used http method as string
     */
    private function getMethodName($request) {
        $method = '';
        if(Director::isDev() && ($varMethod = $request->getVar('method'))) {
            if(in_array(strtoupper($varMethod), ['GET','POST','PUT','DELETE','HEAD', 'PATCH'])) {
                $method = $varMethod;
            }
        } else {
            $method = $request->httpMethod();
        }
        return strtolower($method);
    }

    /**
     * Check, if the request is authenticated.
     * @return bool
     * @throws RestSystemException
     */
    protected function isAuthenticated() {
        return $this->currentUser() ? true : false;
    }

    /**
     * Check if the user has admin privileges.
     *
     * @return bool
     * @throws RestSystemException
     */
    protected function isAdmin() {
        $member = $this->currentUser();
        return $member && \Injector::inst()->get('PermissionChecks')->isAdmin($member);
    }

    /**
     * @param \SS_HTTPResponse $response the current response object
     * @return \SS_HTTPResponse the response with CORS headers
     */
    protected function addCORSHeaders($response) {
        $response->addHeader('Access-Control-Allow-Origin', \Config::inst()->get('BaseRestController', 'CORSOrigin'));
        $response->addHeader('Access-Control-Allow-Methods', \Config::inst()->get('BaseRestController', 'CORSMethods'));
        $response->addHeader('Access-Control-Max-Age', \Config::inst()->get('BaseRestController', 'CORSMaxAge'));
        $response->addHeader('Access-Control-Allow-Headers', \Config::inst()->get('BaseRestController', 'CORSAllowHeaders'));
        return $response;
    }

    /**
     * Return the current user from the request.
     * @return \Member the current user
     */
    protected function currentUser() {
        return AuthFactory::createAuth()->current($this->request);
    }
}


1		<?php
2
3		namespace Ntb\RestAPI;
4
5		use Director;
6		use Exception;
7		use HTMLText;
8		use Member;
9		use SS_HTTPRequest;
10		use SS_HTTPResponse;
11		use SS_Log;
12
13		/**
14		* Base class for the rest resource controllers.
15		* @author Christian Blank <[email protected]>
16		*/
17		abstract class BaseRestController extends \Controller {
18
19		private static $allowed_actions = array (
		0 ignored issues – show Comprehensibility introduced 2016-08-19 22:57 UTC by Report Bug Copy Issue Report Consider using a different property name as you override a private property of the parent class. Loading history... Unused Code introduced 2016-08-19 22:57 UTC by Report Bug Copy Issue Report The property `$allowed_actions` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
20		'options' => true,
21		'head' => true
22		);
23
24		/**
25		* Configuration option.
26		* If set to true, only https connections will be processed.
27		* @var bool
28		*/
29		private static $https_only = true;
		0 ignored issues – show Unused Code introduced 2016-05-18 07:01 UTC by Report Bug Copy Issue Report The property `$https_only` is not used and could be removed. This check marks private properties in classes that are never used. Those properties can be removed. Loading history...
30
31		/**
32		*
33		*/
34		public function init() {
35		parent::init();
36		// check for https
37		if($this->config()->https_only && !Director::is_https()) {
38		$response = $this->getResponse();
39		$response->setStatusCode('403', 'http request not allowed');
40		$response->setBody("Request over HTTP is not allowed. Please switch to https.");
41		$response->output();
42		exit;
43		}
44		// check for CORS options request
45		if ($this->request->httpMethod() === 'OPTIONS' ) {
46		// create direct response without requesting any controller
47		$response = $this->getResponse();
48		// set CORS header from config
49		$response = $this->addCORSHeaders($response);
50		$response->output();
51		exit;
52		}
53		}
54
55		/**
56		* @param SS_HTTPRequest $request
57		* @return null
		0 ignored issues – show Documentation introduced 2016-08-19 22:57 UTC by Report Bug Copy Issue Report Should the return type not be `SS_HTTPResponse\|null`? This check compares the return type specified in the `@return` annotation of a function or method doc comment with the types returned by the function and raises an issue if they mismatch. Loading history...
58		* @throws RestUserException
59		*/
60		public function head(SS_HTTPRequest $request) {
61		if(method_exists($this, 'get')) {
62		$result = $this->get($request);
		0 ignored issues – show Documentation Bug introduced 2016-08-19 22:57 UTC by Report Bug Copy Issue Report The method `get` does not exist on `object<Ntb\RestAPI\BaseRestController>`? 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...
63		if($result instanceof SS_HTTPResponse) {
64		$result->setBody(null);
65		return $result;
66		}
67		return null;
68		}
69		throw new RestUserException("Endpoint doesn't have a GET implementation", 404);
70		}
71
72		/**
73		* handleAction implementation for rest controllers. This handles the requested action differently then the standard
74		* implementation.
75		*
76		* @param SS_HTTPRequest $request
77		* @param string $action
78		* @return HTMLText\|SS_HTTPResponse
79		*/
80		protected function handleAction($request, $action) {
81		foreach($request->latestParams() as $k => $v) {
82		if($v \|\| !isset($this->urlParams[$k])) $this->urlParams[$k] = $v;
83		}
84		// set the action to the request method / for developing we could use an additional parameter to choose another method
85		$action = $this->getMethodName($request);
86		$this->action = $action;
87		$this->requestParams = $request->requestVars();
		0 ignored issues – show Documentation Bug introduced 2016-05-18 07:01 UTC by Report Bug Copy Issue Report It seems like `$request->requestVars()` can be `null`. However, the property `$requestParams` is declared as `array`. Maybe change the type of the property to `array\|null` or add a type check? Our type inference engine has found an assignment of a scalar value (like a string, an integer or null) to a property which is an array. Either this assignment is in error or the assigned type should be added to the documentation/type hint for that property. To type hint that a parameter can be either an array or null, you can set a type hint of array and a default value of null. The PHP interpreter will then accept both an array or null for that parameter. function aContainsB(array $needle = null, array $haystack) { if (!$needle) { return false; } return array_intersect($haystack, $needle) == $haystack; } The function can be called with either null or an array for the parameter `$needle` but will only accept an array as `$haystack`. Loading history...
88		$className = $this->class;
89		// create serializer
90		$serializer = SerializerFactory::create_from_request($request);
91		$response = $this->getResponse();
92		// perform action
93		try {
94		if(!$this->hasAction($action)) {
95		// method couldn't found on controller
96		throw new RestUserException("Action '$action' isn't available on class $className.", 404);
97		}
98		if(!$this->checkAccessAction($action)) {
99		throw new RestUserException("Action '$action' isn't allowed on class $className.", 404, 401);
100		}
101		$actionResult = null;
102		if(method_exists($this, 'beforeCallActionHandler')) {
103		// call before action hook
104		$actionResult = $this->beforeCallActionHandler($request, $action);
		0 ignored issues – show Documentation Bug introduced 2016-08-19 22:57 UTC by Report Bug Copy Issue Report The method `beforeCallActionHandler` does not exist on `object<Ntb\RestAPI\BaseRestController>`? 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...
105		}
106		// if action hook contains data it will be used as result, otherwise the action handler will be called
107		if(!$actionResult) {
108		// perform action
109		$actionResult = $this->$action($request);
110		}
111		$body = $actionResult;
112		} catch(RestUserException $ex) {
113		// a user exception was caught
114		$response->setStatusCode($ex->getHttpStatusCode());
115		$body = [
116		'message' => $ex->getMessage(),
117		'code' => $ex->getCode()
118		];
119		// log all data
120		SS_Log::log(
121		json_encode(array_merge($body, ['file' => $ex->getFile(), 'line' => $ex->getLine()])),
122		SS_Log::INFO);
123		} catch(RestSystemException $ex) {
124		// a system exception was caught
125		$response->addHeader('Content-Type', $serializer->contentType());
126		$response->setStatusCode($ex->getHttpStatusCode());
127		$body = [
128		'message' => $ex->getMessage(),
129		'code' => $ex->getCode()
130		];
131	View Code Duplication	if(Director::isDev()) {
		0 ignored issues – show Duplication introduced 2016-05-18 07:01 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...
132		$body = array_merge($body, [
133		'file' => $ex->getFile(),
134		'line' => $ex->getLine(),
135		'trace' => $ex->getTrace()
136		]);
137		}
138		// log all data
139		SS_Log::log(
140		json_encode(array_merge($body, ['file' => $ex->getFile(), 'line' => $ex->getLine()])),
141		SS_Log::WARN);
142		} catch(Exception $ex) {
143		// an unexpected exception was caught
144		$response->addHeader('Content-Type', $serializer->contentType());
145		$response->setStatusCode("500");
146		$body = [
147		'message' => $ex->getMessage(),
148		'code' => $ex->getCode()
149		];
150	View Code Duplication	if(Director::isDev()) {
		0 ignored issues – show Duplication introduced 2016-05-18 07:01 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...
151		$body = array_merge($body, [
152		'file' => $ex->getFile(),
153		'line' => $ex->getLine(),
154		'trace' => $ex->getTrace()
155		]);
156		}
157		// log all data and the trace to get a better understanding of the exception
158		SS_Log::log(
159		json_encode(array_merge(
160		$body, ['file' => $ex->getFile(), 'line' => $ex->getLine(),'trace' => $ex->getTrace()])),
161		SS_Log::ERR);
162		}
163		// serialize content and set body of response
164		$response->addHeader('Content-Type', $serializer->contentType());
165		// TODO: body could be an exception; check it before the response is generated
166		$response->setBody($serializer->serialize($body));
167		// set CORS header from config
168		$response = $this->addCORSHeaders($response);
169		return $response;
170		}
171
172		/**
173		* Returns the http method for this request. If the current environment is a development env, the method can be
174		* changed with a `method` variable.
175		*
176		* @param \SS_HTTPRequest $request the current request
177		* @return string the used http method as string
178		*/
179		private function getMethodName($request) {
180		$method = '';
181		if(Director::isDev() && ($varMethod = $request->getVar('method'))) {
182		if(in_array(strtoupper($varMethod), ['GET','POST','PUT','DELETE','HEAD', 'PATCH'])) {
183		$method = $varMethod;
184		}
185		} else {
186		$method = $request->httpMethod();
187		}
188		return strtolower($method);
189		}
190
191		/**
192		* Check, if the request is authenticated.
193		* @return bool
194		* @throws RestSystemException
195		*/
196		protected function isAuthenticated() {
197		return $this->currentUser() ? true : false;
198		}
199
200		/**
201		* Check if the user has admin privileges.
202		*
203		* @return bool
204		* @throws RestSystemException
205		*/
206		protected function isAdmin() {
207		$member = $this->currentUser();
208		return $member && \Injector::inst()->get('PermissionChecks')->isAdmin($member);
209		}
210
211		/**
212		* @param \SS_HTTPResponse $response the current response object
213		* @return \SS_HTTPResponse the response with CORS headers
214		*/
215		protected function addCORSHeaders($response) {
216		$response->addHeader('Access-Control-Allow-Origin', \Config::inst()->get('BaseRestController', 'CORSOrigin'));
217		$response->addHeader('Access-Control-Allow-Methods', \Config::inst()->get('BaseRestController', 'CORSMethods'));
218		$response->addHeader('Access-Control-Max-Age', \Config::inst()->get('BaseRestController', 'CORSMaxAge'));
219		$response->addHeader('Access-Control-Allow-Headers', \Config::inst()->get('BaseRestController', 'CORSAllowHeaders'));
220		return $response;
221		}
222
223		/**
224		* Return the current user from the request.
225		* @return \Member the current user
226		*/
227		protected function currentUser() {
228		return AuthFactory::createAuth()->current($this->request);
229		}
230		}
231

notthatbad / silverstripe-rest-api

Pull Request — master (#38)

BaseRestController::handleAction() D

Complexity

Size

Duplication

Importance

How to fix Long Method Complexity

Long Method

Duplication Side-by-Side

Filter issues like