GroupsService - Code Metrics - Inspection of "Merge pull request #310 from nextcloud/bugfix/noid..." - nextcloud/circles - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( 962e80...7b87aa )

by Maxence

created 2019-06-24 15:25 UTC

GroupsService A

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	252
Duplicated Lines	13.1 %

Coupling/Cohesion

Components	1
Dependencies	10

Importance

Changes

Metric	Value
wmc	21
lcom	1
cbo	10
dl	33
loc	252
rs	10
c	0
b	0
f	0

7 Methods

Rating	Name	Duplication	Size	Complexity
A	__construct()	16	16	1
B	linkGroup()	0	43	6
A	getFreshNewMember()	0	21	4
A	levelGroup()	0	32	5
A	editGroupLevel()	17	17	2
A	unlinkGroup()	0	19	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;
use OCP\IUserManager;

class GroupsService {

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

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

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

	/** @var IUserManager */
	private $userManager;

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

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

	/** @var CirclesService */
	private $circlesService;

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

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

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

		$userId, IL10N $l10n, IGroupManager $groupManager, IUserManager $userManager,
		CirclesRequest $circlesRequest,
		MembersRequest $membersRequest, CirclesService $circlesService,
		EventsService $eventsService, MiscService $miscService
	) {
		$this->userId = $userId;
		$this->l10n = $l10n;
		$this->groupManager = $groupManager;
		$this->userManager = $userManager;
		$this->circlesRequest = $circlesRequest;
		$this->membersRequest = $membersRequest;
		$this->circlesService = $circlesService;
		$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);
			$this->circlesService->hasToBeAdmin($circle->getHigherViewer());

			$allMembers =
				$this->membersRequest->forceGetMembers($circleUniqueId, Member::LEVEL_MEMBER, true);

			$group = $this->groupManager->get($groupId);
			$count = $group->count();

			foreach ($allMembers as $member) {
				if ($member->getType() !== Member::TYPE_USER) {
					continue;
				}

				$user = $this->userManager->get($member->getUserId());
				if ($group->inGroup($user)) {
					continue;
				}

				$count++;
			}

			if ($count > $circle->getSetting('members_limit')) {
				throw new \Exception('Group contains too many members');
			}

			$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();
			$this->circlesService->hasToBeAdmin($isMod);
			$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();
			$this->circlesService->hasToBeAdmin($higherViewer);

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

			$this->membersRequest->unlinkAllFromGroup($groupId);

			$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		use OCP\IUserManager;
42
43		class GroupsService {
44
45		/** @var string */
46		private $userId;
47
48		/** @var IL10N */
49		private $l10n;
50
51		/** @var IGroupManager */
52		private $groupManager;
53
54		/** @var IUserManager */
55		private $userManager;
56
57		/** @var CirclesRequest */
58		private $circlesRequest;
59
60		/** @var MembersRequest */
61		private $membersRequest;
62
63		/** @var CirclesService */
64		private $circlesService;
65
66		/** @var EventsService */
67		private $eventsService;
68
69		/** @var MiscService */
70		private $miscService;
71
72		/**
73		* GroupsService constructor.
74		*
75		* @param string $userId
76		* @param IL10N $l10n
77		* @param IGroupManager $groupManager
78		* @param IUserManager $userManager
79		* @param CirclesRequest $circlesRequest
80		* @param MembersRequest $membersRequest
81		* @param CirclesService $circlesService
82		* @param EventsService $eventsService
83		* @param MiscService $miscService
84		*/
85	View Code Duplication	public function __construct(
		0 ignored issues – show Duplication introduced 2019-06-24 15:27 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...
86		$userId, IL10N $l10n, IGroupManager $groupManager, IUserManager $userManager,
87		CirclesRequest $circlesRequest,
88		MembersRequest $membersRequest, CirclesService $circlesService,
89		EventsService $eventsService, MiscService $miscService
90		) {
91		$this->userId = $userId;
92		$this->l10n = $l10n;
93		$this->groupManager = $groupManager;
94		$this->userManager = $userManager;
95		$this->circlesRequest = $circlesRequest;
96		$this->membersRequest = $membersRequest;
97		$this->circlesService = $circlesService;
98		$this->eventsService = $eventsService;
99		$this->miscService = $miscService;
100		}
101
102
103		/**
104		* @param string $circleUniqueId
105		* @param string $groupId
106		*
107		* @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...
108		* @throws \Exception
109		*/
110		public function linkGroup($circleUniqueId, $groupId) {
111
112		try {
113		$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
114		$this->circlesService->hasToBeAdmin($circle->getHigherViewer());
115
116		$allMembers =
117		$this->membersRequest->forceGetMembers($circleUniqueId, Member::LEVEL_MEMBER, true);
118
119		$group = $this->groupManager->get($groupId);
120		$count = $group->count();
121
122		foreach ($allMembers as $member) {
123		if ($member->getType() !== Member::TYPE_USER) {
124		continue;
125		}
126
127		$user = $this->userManager->get($member->getUserId());
128		if ($group->inGroup($user)) {
129		continue;
130		}
131
132		$count++;
133		}
134
135		if ($count > $circle->getSetting('members_limit')) {
136		throw new \Exception('Group contains too many members');
137		}
138
139		$group = $this->getFreshNewMember($circleUniqueId, $groupId);
140		} catch (\Exception $e) {
141		throw $e;
142		}
143
144		$group->setLevel(Member::LEVEL_MEMBER);
145		$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 `139` 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...
146
147		$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 `139` 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...
148
149		return $this->membersRequest->getGroupsFromCircle(
150		$circleUniqueId, $circle->getHigherViewer()
151		);
152		}
153
154
155		/**
156		* Check if a fresh member can be generated (by linkGroup)
157		*
158		* @param $circleId
159		* @param $groupId
160		*
161		* @return null\|Member
162		* @throws MemberAlreadyExistsException
163		* @throws GroupDoesNotExistException
164		*/
165		private function getFreshNewMember($circleId, $groupId) {
166
167		if (!$this->groupManager->groupExists($groupId)) {
168		throw new GroupDoesNotExistException($this->l10n->t("This group does not exist"));
169		}
170
171		try {
172		$member = $this->membersRequest->forceGetGroup($circleId, $groupId);
173		} catch (MemberDoesNotExistException $e) {
174		$member = new Member($groupId, Member::TYPE_GROUP, $circleId);
175		$this->membersRequest->insertGroup($member);
176		}
177
178		if ($member->getLevel() > Member::LEVEL_NONE) {
179		throw new MemberAlreadyExistsException(
180		$this->l10n->t('This group is already linked to the circle')
181		);
182		}
183
184		return $member;
185		}
186
187
188		/**
189		* @param string $circleUniqueId
190		* @param string $groupId
191		* @param int $level
192		*
193		* @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...
194		* @throws \Exception
195		*/
196		public function levelGroup($circleUniqueId, $groupId, $level) {
197
198		$level = (int)$level;
199		try {
200		$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
201		if ($circle->getType() === Circle::CIRCLES_PERSONAL) {
202		throw new CircleTypeNotValidException(
203		$this->l10n->t('You cannot edit level in a personal circle')
204		);
205		}
206
207		$group = $this->membersRequest->forceGetGroup($circle->getUniqueId(), $groupId);
208		if ($group->getLevel() !== $level) {
209		if ($level === Member::LEVEL_OWNER) {
210		throw new GroupCannotBeOwnerException(
211		$this->l10n->t('Group cannot be set as owner of a circle')
212		);
213		} else {
214		$this->editGroupLevel($circle, $group, $level);
215		}
216
217		$this->eventsService->onGroupLevel($circle, $group);
218		}
219
220		return $this->membersRequest->getGroupsFromCircle(
221		$circle->getUniqueId(), $circle->getHigherViewer()
222		);
223		} catch (\Exception $e) {
224		throw $e;
225		}
226
227		}
228
229
230		/**
231		* @param Circle $circle
232		* @param Member $group
233		* @param $level
234		*
235		* @throws \Exception
236		*/
237	View Code Duplication	private function editGroupLevel(Circle $circle, Member &$group, $level) {
		0 ignored issues – show Duplication introduced 2017-08-07 15: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...
238		try {
239		$isMod = $circle->getHigherViewer();
240		$this->circlesService->hasToBeAdmin($isMod);
241		$isMod->hasToBeHigherLevel($level);
242
243		$group->hasToBeMember();
244		$group->cantBeOwner();
245		$isMod->hasToBeHigherLevel($group->getLevel());
246
247		$group->setLevel($level);
248		$this->membersRequest->updateGroup($group);
249
250		} catch (\Exception $e) {
251		throw $e;
252		}
253		}
254
255
256		/**
257		* @param string $circleUniqueId
258		* @param string $groupId
259		*
260		* @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...
261		* @throws \Exception
262		*/
263		public function unlinkGroup($circleUniqueId, $groupId) {
264		try {
265		$circle = $this->circlesRequest->getCircle($circleUniqueId, $this->userId);
266		$higherViewer = $circle->getHigherViewer();
267		$this->circlesService->hasToBeAdmin($higherViewer);
268
269		$group = $this->membersRequest->forceGetGroup($circleUniqueId, $groupId);
270		$group->cantBeOwner();
271		$higherViewer->hasToBeHigherLevel($group->getLevel());
272
273		$this->membersRequest->unlinkAllFromGroup($groupId);
274
275		$this->eventsService->onGroupUnlink($circle, $group);
276
277		return $this->membersRequest->getGroupsFromCircle($circleUniqueId, $higherViewer);
278		} catch (\Exception $e) {
279		throw $e;
280		}
281		}
282
283
284		/**
285		* When a group is removed, remove it from all Circles
286		*
287		* @param string $groupId
288		*/
289		public function onGroupRemoved($groupId) {
290		$this->membersRequest->unlinkAllFromGroup($groupId);
291		}
292
293
294		}
295

nextcloud / circles

Push — master ( 962e80...7b87aa )

GroupsService A

Complexity

Size/Duplication

Coupling/Cohesion

Importance

7 Methods

How to fix Duplicated Code

Duplicated Code

Duplication Side-by-Side

Filter issues like