GroupsService - Code Metrics - Inspection of "cleaning code" - nextcloud/circles - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 96b2c0...c8832b )

by Maxence

created 2017-08-30 15:46 UTC

GroupsService A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	219
Duplicated Lines	9.59 %

Coupling/Cohesion

Components	1
Dependencies	9

Importance

Changes	2
Bugs	0	Features	0

Metric	Value
wmc	17
lcom	1
cbo	9
dl	21
loc	219
rs	10
c	2
b	0
f	0

7 Methods

Rating	Name	Duplication	Size	Complexity
A	linkGroup()	21	21	2
A	getFreshNewMember()	0	21	4
A	__construct()	0	12	1
B	levelGroup()	0	32	5
A	editGroupLevel()	0	17	2
A	unlinkGroup()	0	20	2
A	onGroupRemoved()	0	3	1

How to fix Duplicated Code

<?php
/**
 * Circles - bring cloud-users closer
 *
 * 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\Service;


use OCA\Circles\Db\CirclesRequest;
use OCA\Circles\Db\MembersRequest;
use OCA\Circles\Exceptions\CircleTypeNotValidException;
use OCA\Circles\Exceptions\GroupCannotBeOwnerException;
use OCA\Circles\Exceptions\GroupDoesNotExistException;
use OCA\Circles\Exceptions\MemberAlreadyExistsException;
use OCA\Circles\Exceptions\MemberDoesNotExistException;
use OCA\Circles\Model\Circle;
use OCA\Circles\Model\Member;
use OCP\IGroupManager;
use OCP\IL10N;

class GroupsService {

	/** @var string */
	private $userId;

	/** @var IL10N */
	private $l10n;

	/** @var IGroupManager */
	private $groupManager;

	/** @var CirclesRequest */
	private $circlesRequest;

	/** @var MembersRequest */
	private $membersRequest;

	/** @var EventsService */
	private $eventsService;

	/** @var MiscService */
	private $miscService;

	/**
	 * GroupsService constructor.
	 *
	 * @param string $UserId
	 * @param IL10N $l10n
	 * @param IGroupManager $groupManager
	 * @param CirclesRequest $circlesRequest
	 * @param MembersRequest $membersRequest
	 * @param EventsService $eventsService
	 * @param MiscService $miscService
	 */
	public function __construct(

		$UserId, IL10N $l10n, IGroupManager $groupManager, CirclesRequest $circlesRequest,
		MembersRequest $membersRequest, EventsService $eventsService, MiscService $miscService
	) {
		$this->userId = $UserId;
		$this->l10n = $l10n;
		$this->groupManager = $groupManager;
		$this->circlesRequest = $circlesRequest;
		$this->membersRequest = $membersRequest;
		$this->eventsService = $eventsService;
		$this->miscService = $miscService;
	}


	/**
	 * @param string $circleUniqueId
	 * @param string $groupId
	 *
	 * @return array

	 * @throws \Exception
	 */
	public function linkGroup($circleUniqueId, $groupId) {


		try {
			$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
			$circle->getHigherViewer()
				   ->hasToBeAdmin();

			$group = $this->getFreshNewMember($circleUniqueId, $groupId);
		} catch (\Exception $e) {
			throw $e;
		}

		$group->setLevel(Member::LEVEL_MEMBER);
		$this->membersRequest->updateGroup($group);
/** @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);
    }
}

		$this->eventsService->onGroupLink($circle, $group);
/** @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);
    }
}

		return $this->membersRequest->getGroupsFromCircle(
			$circleUniqueId, $circle->getHigherViewer()
		);
	}


	/**
	 * Check if a fresh member can be generated (by linkGroup)
	 *
	 * @param $circleId
	 * @param $groupId
	 *
	 * @return null|Member
	 * @throws MemberAlreadyExistsException
	 * @throws GroupDoesNotExistException
	 */
	private function getFreshNewMember($circleId, $groupId) {

		if (!$this->groupManager->groupExists($groupId)) {
			throw new GroupDoesNotExistException($this->l10n->t("This group does not exist"));
		}

		try {
			$member = $this->membersRequest->forceGetGroup($circleId, $groupId);
		} catch (MemberDoesNotExistException $e) {
			$member = new Member($groupId, Member::TYPE_GROUP, $circleId);
			$this->membersRequest->insertGroup($member);
		}

		if ($member->getLevel() > Member::LEVEL_NONE) {
			throw new MemberAlreadyExistsException(
				$this->l10n->t('This group is already linked to the circle')
			);
		}

		return $member;
	}


	/**
	 * @param string $circleUniqueId
	 * @param string $groupId
	 * @param int $level
	 *
	 * @return array

	 * @throws \Exception
	 */
	public function levelGroup($circleUniqueId, $groupId, $level) {

		$level = (int)$level;
		try {
			$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
			if ($circle->getType() === Circle::CIRCLES_PERSONAL) {
				throw new CircleTypeNotValidException(
					$this->l10n->t('You cannot edit level in a personal circle')
				);
			}

			$group = $this->membersRequest->forceGetGroup($circle->getUniqueId(), $groupId);
			if ($group->getLevel() !== $level) {
				if ($level === Member::LEVEL_OWNER) {
					throw new GroupCannotBeOwnerException(
						$this->l10n->t('Group cannot be set as owner of a circle')
					);
				} else {
					$this->editGroupLevel($circle, $group, $level);
				}

				$this->eventsService->onGroupLevel($circle, $group);
			}

			return $this->membersRequest->getGroupsFromCircle(
				$circle->getUniqueId(), $circle->getHigherViewer()
			);
		} catch (\Exception $e) {
			throw $e;
		}

	}


	/**
	 * @param Circle $circle
	 * @param Member $group
	 * @param $level
	 *
	 * @throws \Exception
	 */
	private function editGroupLevel(Circle $circle, Member &$group, $level) {
		try {
			$isMod = $circle->getHigherViewer();
			$isMod->hasToBeAdmin();
			$isMod->hasToBeHigherLevel($level);

			$group->hasToBeMember();
			$group->cantBeOwner();
			$isMod->hasToBeHigherLevel($group->getLevel());

			$group->setLevel($level);
			$this->membersRequest->updateGroup($group);

		} catch (\Exception $e) {
			throw $e;
		}
	}


	/**
	 * @param string $circleUniqueId
	 * @param string $groupId
	 *
	 * @return array

	 * @throws \Exception
	 */
	public function unlinkGroup($circleUniqueId, $groupId) {
		try {
			$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
			$higherViewer = $circle->getHigherViewer();
			$higherViewer->hasToBeAdmin();

			$group = $this->membersRequest->forceGetGroup($circleUniqueId, $groupId);
			$group->cantBeOwner();
			$higherViewer->hasToBeHigherLevel($group->getLevel());

			$group->setLevel(Member::LEVEL_NONE);
			$this->membersRequest->updateGroup($group);

			$this->eventsService->onGroupUnlink($circle, $group);

			return $this->membersRequest->getGroupsFromCircle($circleUniqueId, $higherViewer);
		} catch (\Exception $e) {
			throw $e;
		}
	}


	/**
	 * When a group is removed, remove it from all Circles
	 *
	 * @param string $groupId
	 */
	public function onGroupRemoved($groupId) {
		$this->membersRequest->unlinkAllFromGroup($groupId);
	}


}

1		<?php
2		/**
3		* Circles - bring cloud-users closer
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\Service;
28
29
30		use OCA\Circles\Db\CirclesRequest;
31		use OCA\Circles\Db\MembersRequest;
32		use OCA\Circles\Exceptions\CircleTypeNotValidException;
33		use OCA\Circles\Exceptions\GroupCannotBeOwnerException;
34		use OCA\Circles\Exceptions\GroupDoesNotExistException;
35		use OCA\Circles\Exceptions\MemberAlreadyExistsException;
36		use OCA\Circles\Exceptions\MemberDoesNotExistException;
37		use OCA\Circles\Model\Circle;
38		use OCA\Circles\Model\Member;
39		use OCP\IGroupManager;
40		use OCP\IL10N;
41
42		class GroupsService {
43
44		/** @var string */
45		private $userId;
46
47		/** @var IL10N */
48		private $l10n;
49
50		/** @var IGroupManager */
51		private $groupManager;
52
53		/** @var CirclesRequest */
54		private $circlesRequest;
55
56		/** @var MembersRequest */
57		private $membersRequest;
58
59		/** @var EventsService */
60		private $eventsService;
61
62		/** @var MiscService */
63		private $miscService;
64
65		/**
66		* GroupsService constructor.
67		*
68		* @param string $UserId
69		* @param IL10N $l10n
70		* @param IGroupManager $groupManager
71		* @param CirclesRequest $circlesRequest
72		* @param MembersRequest $membersRequest
73		* @param EventsService $eventsService
74		* @param MiscService $miscService
75		*/
76		public function __construct(
		0 ignored issues – show Coding Style Naming introduced 2017-08-30 15:18 UTC by Report Bug Copy Issue Report The parameter $UserId is not named in camelCase. This check marks parameter names that have not been written in camelCase. In camelCase names are written without any punctuation, the start of each new word being marked by a capital letter. Thus the name database connection string becomes `databaseConnectionString`. Loading history...
77		$UserId, IL10N $l10n, IGroupManager $groupManager, CirclesRequest $circlesRequest,
78		MembersRequest $membersRequest, EventsService $eventsService, MiscService $miscService
79		) {
80		$this->userId = $UserId;
81		$this->l10n = $l10n;
82		$this->groupManager = $groupManager;
83		$this->circlesRequest = $circlesRequest;
84		$this->membersRequest = $membersRequest;
85		$this->eventsService = $eventsService;
86		$this->miscService = $miscService;
87		}
88
89
90		/**
91		* @param string $circleUniqueId
92		* @param string $groupId
93		*
94		* @return array
		0 ignored issues – show Documentation introduced 2017-07-14 16:23 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `Member[]`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
95		* @throws \Exception
96		*/
97	View Code Duplication	public function linkGroup($circleUniqueId, $groupId) {
		0 ignored issues – show Duplication introduced 2017-08-12 17:59 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...
98
99		try {
100		$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
101		$circle->getHigherViewer()
102		->hasToBeAdmin();
103
104		$group = $this->getFreshNewMember($circleUniqueId, $groupId);
105		} catch (\Exception $e) {
106		throw $e;
107		}
108
109		$group->setLevel(Member::LEVEL_MEMBER);
110		$this->membersRequest->updateGroup($group);
		0 ignored issues – show Bug introduced 2017-07-28 02:28 UTC by Report Bug Copy Issue Report It seems like `$group` defined by `$this->getFreshNewMember...rcleUniqueId, $groupId)` on line `104` can be `null`; however, `OCA\Circles\Db\MembersRequest::updateGroup()` 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...
111
112		$this->eventsService->onGroupLink($circle, $group);
		0 ignored issues – show Bug introduced 2017-07-28 17:30 UTC by Report Bug Copy Issue Report It seems like `$group` defined by `$this->getFreshNewMember...rcleUniqueId, $groupId)` on line `104` can be `null`; however, `OCA\Circles\Service\EventsService::onGroupLink()` 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		return $this->membersRequest->getGroupsFromCircle(
115		$circleUniqueId, $circle->getHigherViewer()
116		);
117		}
118
119
120		/**
121		* Check if a fresh member can be generated (by linkGroup)
122		*
123		* @param $circleId
124		* @param $groupId
125		*
126		* @return null\|Member
127		* @throws MemberAlreadyExistsException
128		* @throws GroupDoesNotExistException
129		*/
130		private function getFreshNewMember($circleId, $groupId) {
131
132		if (!$this->groupManager->groupExists($groupId)) {
133		throw new GroupDoesNotExistException($this->l10n->t("This group does not exist"));
134		}
135
136		try {
137		$member = $this->membersRequest->forceGetGroup($circleId, $groupId);
138		} catch (MemberDoesNotExistException $e) {
139		$member = new Member($groupId, Member::TYPE_GROUP, $circleId);
140		$this->membersRequest->insertGroup($member);
141		}
142
143		if ($member->getLevel() > Member::LEVEL_NONE) {
144		throw new MemberAlreadyExistsException(
145		$this->l10n->t('This group is already linked to the circle')
146		);
147		}
148
149		return $member;
150		}
151
152
153		/**
154		* @param string $circleUniqueId
155		* @param string $groupId
156		* @param int $level
157		*
158		* @return array
		0 ignored issues – show Documentation introduced 2017-07-05 22:18 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `Member[]`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
159		* @throws \Exception
160		*/
161		public function levelGroup($circleUniqueId, $groupId, $level) {
162
163		$level = (int)$level;
164		try {
165		$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
166		if ($circle->getType() === Circle::CIRCLES_PERSONAL) {
167		throw new CircleTypeNotValidException(
168		$this->l10n->t('You cannot edit level in a personal circle')
169		);
170		}
171
172		$group = $this->membersRequest->forceGetGroup($circle->getUniqueId(), $groupId);
173		if ($group->getLevel() !== $level) {
174		if ($level === Member::LEVEL_OWNER) {
175		throw new GroupCannotBeOwnerException(
176		$this->l10n->t('Group cannot be set as owner of a circle')
177		);
178		} else {
179		$this->editGroupLevel($circle, $group, $level);
180		}
181
182		$this->eventsService->onGroupLevel($circle, $group);
183		}
184
185		return $this->membersRequest->getGroupsFromCircle(
186		$circle->getUniqueId(), $circle->getHigherViewer()
187		);
188		} catch (\Exception $e) {
189		throw $e;
190		}
191
192		}
193
194
195		/**
196		* @param Circle $circle
197		* @param Member $group
198		* @param $level
199		*
200		* @throws \Exception
201		*/
202		private function editGroupLevel(Circle $circle, Member &$group, $level) {
203		try {
204		$isMod = $circle->getHigherViewer();
205		$isMod->hasToBeAdmin();
206		$isMod->hasToBeHigherLevel($level);
207
208		$group->hasToBeMember();
209		$group->cantBeOwner();
210		$isMod->hasToBeHigherLevel($group->getLevel());
211
212		$group->setLevel($level);
213		$this->membersRequest->updateGroup($group);
214
215		} catch (\Exception $e) {
216		throw $e;
217		}
218		}
219
220
221		/**
222		* @param string $circleUniqueId
223		* @param string $groupId
224		*
225		* @return array
		0 ignored issues – show Documentation introduced 2017-07-05 22:18 UTC by Report Bug Copy Issue Report Consider making the return type a bit more specific; maybe use `Member[]`. This check looks for the generic type `array` as a return type and suggests a more specific type. This type is inferred from the actual code. Loading history...
226		* @throws \Exception
227		*/
228		public function unlinkGroup($circleUniqueId, $groupId) {
229		try {
230		$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
231		$higherViewer = $circle->getHigherViewer();
232		$higherViewer->hasToBeAdmin();
233
234		$group = $this->membersRequest->forceGetGroup($circleUniqueId, $groupId);
235		$group->cantBeOwner();
236		$higherViewer->hasToBeHigherLevel($group->getLevel());
237
238		$group->setLevel(Member::LEVEL_NONE);
239		$this->membersRequest->updateGroup($group);
240
241		$this->eventsService->onGroupUnlink($circle, $group);
242
243		return $this->membersRequest->getGroupsFromCircle($circleUniqueId, $higherViewer);
244		} catch (\Exception $e) {
245		throw $e;
246		}
247		}
248
249
250		/**
251		* When a group is removed, remove it from all Circles
252		*
253		* @param string $groupId
254		*/
255		public function onGroupRemoved($groupId) {
256		$this->membersRequest->unlinkAllFromGroup($groupId);
257		}
258
259
260		}

nextcloud / circles

Push — master ( 96b2c0...c8832b )

GroupsService A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

7 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like