RequestSharedSecret::run() - Code Metrics - Inspection of "Merge pull request #4329 from nextcloud/move-out-s..." - nextcloud/server - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( dccb89...31024b )

by Morris

created 2017-04-12 16:44 UTC

RequestSharedSecret::run() B

↳ Parent: RequestSharedSecret

Complexity

Conditions	8
Paths	56

Size

Total Lines	54
Code Lines	32

Duplication

Lines	5
Ratio	9.26 %

Importance

Changes

Metric	Value
cc	8
eloc	32
nc	56
nop	1
dl	5
loc	54
rs	7.4119
c	0
b	0
f	0

How to fix Long Method

<?php
/**
 * @copyright Copyright (c) 2016, ownCloud, Inc.
 *
 * @author Björn Schießle <[email protected]>
 * @author Joas Schilling <[email protected]>
 * @author Robin Appelman <[email protected]>
 * @author Thomas Müller <[email protected]>
 *
 * @license AGPL-3.0
 *
 * This code is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License, version 3,
 * as published by the Free Software Foundation.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
 * GNU Affero General Public License for more details.
 *
 * You should have received a copy of the GNU Affero General Public License, version 3,
 * along with this program.  If not, see <http://www.gnu.org/licenses/>
 *
 */


namespace OCA\Federation\BackgroundJob;


use GuzzleHttp\Exception\ClientException;
use OC\BackgroundJob\JobList;
use OC\BackgroundJob\Job;
use OCA\Federation\DbHandler;
use OCA\Federation\TrustedServers;
use OCP\AppFramework\Http;
use OCP\BackgroundJob\IJobList;
use OCP\Http\Client\IClient;
use OCP\ILogger;
use OCP\IURLGenerator;
use OCP\OCS\IDiscoveryService;

/**
 * Class RequestSharedSecret
 *
 * Ask remote Nextcloud to request a sharedSecret from this server
 *
 * @package OCA\Federation\Backgroundjob
 */
class RequestSharedSecret extends Job {

	/** @var IClient */
	private $httpClient;

	/** @var IJobList */
	private $jobList;

	/** @var IURLGenerator */
	private $urlGenerator;

	/** @var DbHandler */
	private $dbHandler;

	/** @var TrustedServers */
	private $trustedServers;

	/** @var IDiscoveryService  */
	private $ocsDiscoveryService;

	/** @var ILogger */
	private $logger;

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

	private $format = '?format=json';

	private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret';

	/**
	 * RequestSharedSecret constructor.
	 *
	 * @param IClient $httpClient
	 * @param IURLGenerator $urlGenerator
	 * @param IJobList $jobList
	 * @param TrustedServers $trustedServers
	 * @param DbHandler $dbHandler
	 * @param IDiscoveryService $ocsDiscoveryService
	 */
	public function __construct(
		IClient $httpClient = null,
		IURLGenerator $urlGenerator = null,
		IJobList $jobList = null,
		TrustedServers $trustedServers = null,
		DbHandler $dbHandler = null,
		IDiscoveryService $ocsDiscoveryService = null
	) {
		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
		$this->logger = \OC::$server->getLogger();
		$this->ocsDiscoveryService = $ocsDiscoveryService ? $ocsDiscoveryService : \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
		if ($trustedServers) {
			$this->trustedServers = $trustedServers;
		} else {
			$this->trustedServers = new TrustedServers(
				$this->dbHandler,
				\OC::$server->getHTTPClientService(),
				$this->logger,
				$this->jobList,
				\OC::$server->getSecureRandom(),
				\OC::$server->getConfig(),
				\OC::$server->getEventDispatcher()
			);
		}
	}


	/**
	 * run the job, then remove it from the joblist
	 *
	 * @param JobList $jobList
	 * @param ILogger $logger
	 */
	public function execute($jobList, ILogger $logger = null) {

		$target = $this->argument['url'];
		// only execute if target is still in the list of trusted domains
		if ($this->trustedServers->isTrustedServer($target)) {
			$this->parentExecute($jobList, $logger);
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->retainJob) {
			$jobList->remove($this, $this->argument);
		}
	}

	/**
	 * call execute() method of parent
	 *
	 * @param JobList $jobList
	 * @param ILogger $logger
	 */
	protected function parentExecute($jobList, $logger) {
		parent::execute($jobList, $logger);
class Daddy
{
    protected function getFirstName()
    {
        return "Eidur";
    }

    protected function getSurName()
    {
        return "Gudjohnsen";
    }
}

class Son
{
    public function getFirstName()
    {
        return parent::getSurname();
    }
}
	}

	protected function run($argument) {

		$target = $argument['url'];
		$source = $this->urlGenerator->getAbsoluteURL('/');
		$source = rtrim($source, '/');
		$token = $argument['token'];

		$endPoints = $this->ocsDiscoveryService->discover($target, 'FEDERATED_SHARING');
		$endPoint = isset($endPoints['shared-secret']) ? $endPoints['shared-secret'] : $this->defaultEndPoint;

		// make sure that we have a well formated url
		$url = rtrim($target, '/') . '/' . trim($endPoint, '/') . $this->format;

		try {
			$result = $this->httpClient->post(
				$url,
				[
					'body' => [
						'url' => $source,
						'token' => $token,
					],
					'timeout' => 3,
					'connect_timeout' => 3,
				]
			);

			$status = $result->getStatusCode();

		} catch (ClientException $e) {

			$status = $e->getCode();
			if ($status === Http::STATUS_FORBIDDEN) {
				$this->logger->info($target . ' refused to ask for a shared secret.', ['app' => 'federation']);
			} else {
				$this->logger->logException($e, ['app' => 'federation']);
			}
		} catch (\Exception $e) {
			$status = Http::STATUS_INTERNAL_SERVER_ERROR;
			$this->logger->logException($e, ['app' => 'federation']);
		}

		// if we received a unexpected response we try again later
		if (
			$status !== Http::STATUS_OK
			&& $status !== Http::STATUS_FORBIDDEN
		) {
			$this->retainJob = true;
		}

		if ($status === Http::STATUS_FORBIDDEN) {
			// clear token if remote server refuses to ask for shared secret
			$this->dbHandler->addToken($target, '');
		}

	}
}


1		<?php
2		/**
3		* @copyright Copyright (c) 2016, ownCloud, Inc.
4		*
5		* @author Björn Schießle <[email protected]>
6		* @author Joas Schilling <[email protected]>
7		* @author Robin Appelman <[email protected]>
8		* @author Thomas Müller <[email protected]>
9		*
10		* @license AGPL-3.0
11		*
12		* This code is free software: you can redistribute it and/or modify
13		* it under the terms of the GNU Affero General Public License, version 3,
14		* as published by the Free Software Foundation.
15		*
16		* This program is distributed in the hope that it will be useful,
17		* but WITHOUT ANY WARRANTY; without even the implied warranty of
18		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
19		* GNU Affero General Public License for more details.
20		*
21		* You should have received a copy of the GNU Affero General Public License, version 3,
22		* along with this program. If not, see <http://www.gnu.org/licenses/>
23		*
24		*/
25
26
27		namespace OCA\Federation\BackgroundJob;
28
29
30		use GuzzleHttp\Exception\ClientException;
31		use OC\BackgroundJob\JobList;
32		use OC\BackgroundJob\Job;
33		use OCA\Federation\DbHandler;
34		use OCA\Federation\TrustedServers;
35		use OCP\AppFramework\Http;
36		use OCP\BackgroundJob\IJobList;
37		use OCP\Http\Client\IClient;
38		use OCP\ILogger;
39		use OCP\IURLGenerator;
40		use OCP\OCS\IDiscoveryService;
41
42		/**
43		* Class RequestSharedSecret
44		*
45		* Ask remote Nextcloud to request a sharedSecret from this server
46		*
47		* @package OCA\Federation\Backgroundjob
48		*/
49		class RequestSharedSecret extends Job {
50
51		/** @var IClient */
52		private $httpClient;
53
54		/** @var IJobList */
55		private $jobList;
56
57		/** @var IURLGenerator */
58		private $urlGenerator;
59
60		/** @var DbHandler */
61		private $dbHandler;
62
63		/** @var TrustedServers */
64		private $trustedServers;
65
66		/** @var IDiscoveryService */
67		private $ocsDiscoveryService;
68
69		/** @var ILogger */
70		private $logger;
71
72		/** @var bool */
73		protected $retainJob = false;
74
75		private $format = '?format=json';
76
77		private $defaultEndPoint = '/ocs/v2.php/apps/federation/api/v1/request-shared-secret';
78
79		/**
80		* RequestSharedSecret constructor.
81		*
82		* @param IClient $httpClient
83		* @param IURLGenerator $urlGenerator
84		* @param IJobList $jobList
85		* @param TrustedServers $trustedServers
86		* @param DbHandler $dbHandler
87		* @param IDiscoveryService $ocsDiscoveryService
88		*/
89		public function __construct(
90		IClient $httpClient = null,
91		IURLGenerator $urlGenerator = null,
92		IJobList $jobList = null,
93		TrustedServers $trustedServers = null,
94		DbHandler $dbHandler = null,
95		IDiscoveryService $ocsDiscoveryService = null
96		) {
97		$this->httpClient = $httpClient ? $httpClient : \OC::$server->getHTTPClientService()->newClient();
98		$this->jobList = $jobList ? $jobList : \OC::$server->getJobList();
99		$this->urlGenerator = $urlGenerator ? $urlGenerator : \OC::$server->getURLGenerator();
100		$this->dbHandler = $dbHandler ? $dbHandler : new DbHandler(\OC::$server->getDatabaseConnection(), \OC::$server->getL10N('federation'));
101		$this->logger = \OC::$server->getLogger();
102		$this->ocsDiscoveryService = $ocsDiscoveryService ? $ocsDiscoveryService : \OC::$server->query(\OCP\OCS\IDiscoveryService::class);
103	View Code Duplication	if ($trustedServers) {
104		$this->trustedServers = $trustedServers;
105		} else {
106		$this->trustedServers = new TrustedServers(
107		$this->dbHandler,
108		\OC::$server->getHTTPClientService(),
109		$this->logger,
110		$this->jobList,
111		\OC::$server->getSecureRandom(),
112		\OC::$server->getConfig(),
113		\OC::$server->getEventDispatcher()
114		);
115		}
116		}
117
118
119		/**
120		* run the job, then remove it from the joblist
121		*
122		* @param JobList $jobList
123		* @param ILogger $logger
124		*/
125	View Code Duplication	public function execute($jobList, ILogger $logger = null) {
		0 ignored issues – show Duplication introduced 2016-06-09 12:21 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...
126		$target = $this->argument['url'];
127		// only execute if target is still in the list of trusted domains
128		if ($this->trustedServers->isTrustedServer($target)) {
129		$this->parentExecute($jobList, $logger);
		0 ignored issues – show Bug introduced 2016-06-09 12:21 UTC by Report Bug Copy Issue Report It seems like `$logger` defined by parameter `$logger` on line `125` can be `null`; however, `OCA\Federation\Backgroun...Secret::parentExecute()` 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...
130		}
131
132		if (!$this->retainJob) {
133		$jobList->remove($this, $this->argument);
134		}
135		}
136
137		/**
138		* call execute() method of parent
139		*
140		* @param JobList $jobList
141		* @param ILogger $logger
142		*/
143		protected function parentExecute($jobList, $logger) {
144		parent::execute($jobList, $logger);
		0 ignored issues – show Comprehensibility Bug introduced 2016-06-09 12:21 UTC by Report Bug Copy Issue Report It seems like you call parent on a different method (`execute()` instead of `parentExecute()`). Are you sure this is correct? If so, you might want to change this to `$this->execute()`. This check looks for a call to a parent method whose name is different than the method from which it is called. Consider the following code: class Daddy { protected function getFirstName() { return "Eidur"; } protected function getSurName() { return "Gudjohnsen"; } } class Son { public function getFirstName() { return parent::getSurname(); } } The `getFirstName()` method in the `Son` calls the wrong method in the parent class. Loading history...
145		}
146
147		protected function run($argument) {
148
149		$target = $argument['url'];
150		$source = $this->urlGenerator->getAbsoluteURL('/');
151		$source = rtrim($source, '/');
152		$token = $argument['token'];
153
154		$endPoints = $this->ocsDiscoveryService->discover($target, 'FEDERATED_SHARING');
155		$endPoint = isset($endPoints['shared-secret']) ? $endPoints['shared-secret'] : $this->defaultEndPoint;
156
157		// make sure that we have a well formated url
158		$url = rtrim($target, '/') . '/' . trim($endPoint, '/') . $this->format;
159
160		try {
161		$result = $this->httpClient->post(
162		$url,
163		[
164		'body' => [
165		'url' => $source,
166		'token' => $token,
167		],
168		'timeout' => 3,
169		'connect_timeout' => 3,
170		]
171		);
172
173		$status = $result->getStatusCode();
174
175		} catch (ClientException $e) {
		0 ignored issues – show Bug introduced 2016-06-09 12:21 UTC by Report Bug Copy Issue Report The class `GuzzleHttp\Exception\ClientException` does not exist. Did you forget a USE statement, or did you not list all dependencies? Scrutinizer analyzes your `composer.json`/`composer.lock` file if available to determine the classes, and functions that are defined by your dependencies. It seems like the listed class was neither found in your dependencies, nor was it found in the analyzed files in your repository. If you are using some other form of dependency management, you might want to disable this analysis. Loading history...
176		$status = $e->getCode();
177	View Code Duplication	if ($status === Http::STATUS_FORBIDDEN) {
178		$this->logger->info($target . ' refused to ask for a shared secret.', ['app' => 'federation']);
179		} else {
180		$this->logger->logException($e, ['app' => 'federation']);
181		}
182		} catch (\Exception $e) {
183		$status = Http::STATUS_INTERNAL_SERVER_ERROR;
184		$this->logger->logException($e, ['app' => 'federation']);
185		}
186
187		// if we received a unexpected response we try again later
188		if (
189		$status !== Http::STATUS_OK
190		&& $status !== Http::STATUS_FORBIDDEN
191		) {
192		$this->retainJob = true;
193		}
194
195		if ($status === Http::STATUS_FORBIDDEN) {
196		// clear token if remote server refuses to ask for shared secret
197		$this->dbHandler->addToken($target, '');
198		}
199
200		}
201		}
202

nextcloud / server

Push — master ( dccb89...31024b )

RequestSharedSecret::run() B

Complexity

Size

Duplication

Importance

How to fix Long Method

Long Method

Duplication Side-by-Side

Filter issues like