SharesController - Code Metrics - Inspection of "removing broadcaster argument from broadcastFrame(..." - nextcloud/circles - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 41fdc1...6665fc )

by Maxence

created 2017-08-21 15:06 UTC

SharesController A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	91
Duplicated Lines	0 %

Coupling/Cohesion

Components	1
Dependencies	7

Importance

Changes

Metric	Value
wmc	5
lcom	1
cbo	7
dl	0
loc	91
rs	10
c	0
b	0
f	0

2 Methods

Rating	Name	Duplication	Size	Complexity
B	create()	0	28	2
A	initShareDelivery()	0	21	3

<?php
/**
 * Circles - Bring cloud-users closer together.
 *
 * This file is licensed under the Affero General Public License version 3 or
 * later. See the COPYING file.
 *
 * @author Maxence Lange <[email protected]>
 * @copyright 2017
 * @license GNU AGPL version 3 or any later version
 *
 * This program is free software: you can redistribute it and/or modify
 * it under the terms of the GNU Affero General Public License as
 * published by the Free Software Foundation, either version 3 of the
 * License, or (at your option) any later version.
 *
 * 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
 * along with this program.  If not, see <http://www.gnu.org/licenses/>.
 *
 */

namespace OCA\Circles\Controller;

use Exception;
use OCA\Circles\Model\SharingFrame;
use OCP\AppFramework\Http\DataResponse;

class SharesController extends BaseController {


	/**
	 * Called by the JavaScript API when creating a new Share item that will be
	 * broadcasted to the circle itself, and any other circle linked to it.
	 *
	 * @NoAdminRequired
	 * @NoSubAdminRequired
	 *
	 * @param string $circleUniqueId
	 * @param string $source
	 * @param string $type
	 * @param array $payload
	 *
	 * @return DataResponse
	 */
	public function create($circleUniqueId, $source, $type, $payload) {

		try {
			$share = new SharingFrame($source, $type);
			$share->setPayload($payload);

			$this->sharesService->createFrame($circleUniqueId, $share);
		} catch (\Exception $e) {
			return $this->fail(
				[
					'circle_id' => $circleUniqueId,
					'source'    => $source,
					'type'      => $type,
					'payload'   => $payload,
					'error'     => $e->getMessage()
				]
			);
		}

		return $this->success(
			[
				'circle_id' => $circleUniqueId,
				'source'    => $source,
				'type'      => $type,
				'payload'   => $payload
			]
		);
	}


	/**
	 * initShareDelivery()
	 *
	 * Note: this function will close the request mid-run from the client but will still
	 * running its process.
	 *
	 * Called by locally, the function will get the SharingFrame by its uniqueId from the database.
	 * After closing the socket, will broadcast the Frame locally and - if Federated Shares are
	 * enable - will deliver it to each remote circles linked to the circle the Payload belongs to.
	 *
	 * A status response is sent to free the client process before starting to broadcast the item
	 * to other federated links.
	 *
	 * @PublicPage
	 * @NoCSRFRequired
	 *
	 * @param string $circleId
	 * @param string $frameId
	 *
	 * @return DataResponse

	 */
	public function initShareDelivery($circleId, $frameId) {

		try {
			$frame = $this->sharesService->getFrameFromUniqueId($circleId, $frameId);
		} catch (Exception $e) {
			return $this->fail($e->getMessage());
		}

		// We don't want to keep the connection up
		$this->miscService->asyncAndLeaveClientOutOfThis('done');

		$this->broadcastService->broadcastFrame($frame);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}

		// TODO - do not update cloudId to avoid duplicate, use it's own field and keep cloudId
		$this->federatedService->updateFrameWithCloudId($frame);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
		if ($this->configService->isFederatedCirclesAllowed()) {
			$this->federatedService->sendRemoteShare($frame);
/** @return stdClass|null */
function mayReturnNull() { }

function doesNotAcceptNull(stdClass $x) { }

// With potential error.
function withoutCheck() {
    $x = mayReturnNull();
    doesNotAcceptNull($x); // Potential error here.
}

// Safe - Alternative 1
function withCheck1() {
    $x = mayReturnNull();
    if ( ! $x instanceof stdClass) {
        throw new \LogicException('$x must be defined.');
    }
    doesNotAcceptNull($x);
}

// Safe - Alternative 2
function withCheck2() {
    $x = mayReturnNull();
    if ($x instanceof stdClass) {
        doesNotAcceptNull($x);
    }
}
		}

		exit();

	}

}



1			<?php
2			/**
3			* Circles - Bring cloud-users closer together.
4			*
5			* This file is licensed under the Affero General Public License version 3 or
6			* later. See the COPYING file.
7			*
8			* @author Maxence Lange <[email protected]>
9			* @copyright 2017
10			* @license GNU AGPL version 3 or any later version
11			*
12			* This program is free software: you can redistribute it and/or modify
13			* it under the terms of the GNU Affero General Public License as
14			* published by the Free Software Foundation, either version 3 of the
15			* License, or (at your option) any later version.
16			*
17			* This program is distributed in the hope that it will be useful,
18			* but WITHOUT ANY WARRANTY; without even the implied warranty of
19			* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
20			* GNU Affero General Public License for more details.
21			*
22			* You should have received a copy of the GNU Affero General Public License
23			* along with this program. If not, see <http://www.gnu.org/licenses/>.
24			*
25			*/
26
27			namespace OCA\Circles\Controller;
28
29			use Exception;
30			use OCA\Circles\Model\SharingFrame;
31			use OCP\AppFramework\Http\DataResponse;
32
33			class SharesController extends BaseController {
34
35
36			/**
37			* Called by the JavaScript API when creating a new Share item that will be
38			* broadcasted to the circle itself, and any other circle linked to it.
39			*
40			* @NoAdminRequired
41			* @NoSubAdminRequired
42			*
43			* @param string $circleUniqueId
44			* @param string $source
45			* @param string $type
46			* @param array $payload
47			*
48			* @return DataResponse
49			*/
50			public function create($circleUniqueId, $source, $type, $payload) {
51
52			try {
53			$share = new SharingFrame($source, $type);
54			$share->setPayload($payload);
55
56			$this->sharesService->createFrame($circleUniqueId, $share);
57			} catch (\Exception $e) {
58			return $this->fail(
59			[
60			'circle_id' => $circleUniqueId,
61			'source' => $source,
62			'type' => $type,
63			'payload' => $payload,
64			'error' => $e->getMessage()
65			]
66			);
67			}
68
69			return $this->success(
70			[
71			'circle_id' => $circleUniqueId,
72			'source' => $source,
73			'type' => $type,
74			'payload' => $payload
75			]
76			);
77			}
78
79
80			/**
81			* initShareDelivery()
82			*
83			* Note: this function will close the request mid-run from the client but will still
84			* running its process.
85			*
86			* Called by locally, the function will get the SharingFrame by its uniqueId from the database.
87			* After closing the socket, will broadcast the Frame locally and - if Federated Shares are
88			* enable - will deliver it to each remote circles linked to the circle the Payload belongs to.
89			*
90			* A status response is sent to free the client process before starting to broadcast the item
91			* to other federated links.
92			*
93			* @PublicPage
94			* @NoCSRFRequired
95			*
96			* @param string $circleId
97			* @param string $frameId
98			*
99			* @return DataResponse
			0 ignored issues – show Documentation introduced 2017-08-21 15:09 UTC by Report Bug Copy Issue Report Should the return type not be `DataResponse\|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...
100			*/
101			public function initShareDelivery($circleId, $frameId) {
102
103			try {
104			$frame = $this->sharesService->getFrameFromUniqueId($circleId, $frameId);
105			} catch (Exception $e) {
106			return $this->fail($e->getMessage());
107			}
108
109			// We don't want to keep the connection up
110			$this->miscService->asyncAndLeaveClientOutOfThis('done');
111
112			$this->broadcastService->broadcastFrame($frame);
			0 ignored issues – show Bug introduced 2017-08-21 15:09 UTC by Report Bug Copy Issue Report It seems like `$frame` defined by `$this->sharesService->ge...Id($circleId, $frameId)` on line `104` can be `null`; however, `OCA\Circles\Service\Broa...rvice::broadcastFrame()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
113
114			// TODO - do not update cloudId to avoid duplicate, use it's own field and keep cloudId
115			$this->federatedService->updateFrameWithCloudId($frame);
			0 ignored issues – show Bug introduced 2017-07-28 02:28 UTC by Report Bug Copy Issue Report It seems like `$frame` defined by `$this->sharesService->ge...Id($circleId, $frameId)` on line `104` can be `null`; however, `OCA\Circles\Service\Fede...pdateFrameWithCloudId()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
116			if ($this->configService->isFederatedCirclesAllowed()) {
117			$this->federatedService->sendRemoteShare($frame);
			0 ignored issues – show Bug introduced 2017-07-28 02:28 UTC by Report Bug Copy Issue Report It seems like `$frame` defined by `$this->sharesService->ge...Id($circleId, $frameId)` on line `104` can be `null`; however, `OCA\Circles\Service\Fede...vice::sendRemoteShare()` does not accept `null`, maybe add an additional type check? Unless you are absolutely sure that the expression can never be null because of other conditions, we strongly recommend to add an additional type check to your code: /** @return stdClass\|null */ function mayReturnNull() { } function doesNotAcceptNull(stdClass $x) { } // With potential error. function withoutCheck() { $x = mayReturnNull(); doesNotAcceptNull($x); // Potential error here. } // Safe - Alternative 1 function withCheck1() { $x = mayReturnNull(); if ( ! $x instanceof stdClass) { throw new \LogicException('$x must be defined.'); } doesNotAcceptNull($x); } // Safe - Alternative 2 function withCheck2() { $x = mayReturnNull(); if ($x instanceof stdClass) { doesNotAcceptNull($x); } } Loading history...
118			}
119
120			exit();
			0 ignored issues – show Coding Style Compatibility introduced 2017-08-21 15:09 UTC by Report Bug Copy Issue Report The method `initShareDelivery()` contains an exit expression. An exit expression should only be used in rare cases. For example, if you write a short command line script. In most cases however, using an `exit` expression makes the code untestable and often causes incompatibilities with other libraries. Thus, unless you are absolutely sure it is required here, we recommend to refactor your code to avoid its usage. Loading history...
121			}
122
123			}
124
125

nextcloud / circles

Push — master ( 41fdc1...6665fc )

SharesController A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

2 Methods

Duplication Side-by-Side

Filter issues like