BoardService - Code Metrics - Inspection of "Merge pull request #535 from nextcloud/feature/83/..." - nextcloud/deck - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Push — master ( a3a70f...f040df )

by Julius

created 2018-08-15 19:21 UTC

BoardService F

↳ Parent: Project

Complexity

Total Complexity

Size/Duplication

Total Lines	453
Duplicated Lines	10.15 %

Coupling/Cohesion

Components	1
Dependencies	15

Importance

Changes

Metric	Value
wmc	65
lcom	1
cbo	15
dl	46
loc	453
rs	3.2
c	0
b	0
f	0

14 Methods

Rating	Name	Duplication	Size	Complexity
A	findAll()	0	26	5
A	find()	0	26	4
A	getBoardPrerequisites()	0	9	1
A	isArchived()	0	20	5
B	isDeleted()	0	24	7
B	create()	0	47	8
A	delete()	12	12	2
A	deleteUndo()	11	11	2
B	update()	0	26	7
C	addAcl()	0	42	12
A	updateAcl()	0	27	5
A	deleteAcl()	0	18	4
A	__construct()	23	23	1
A	deleteForce()	0	9	2

How to fix Duplicated Code Complexity

Duplicated Code

Duplicate code is one of the most pungent code smells. A rule that is often used is to re-structure code once it is duplicated in three or more places.

Common duplication problems, and corresponding solutions are:

If you have the same expression in different places: Extract expression to a method
If you have the same method in different sub-classes: Extract method, and pull up field to the parent class
If you have the same code in unrelated classes: Consider extracting the code to a new class, and injecting that class

Complex Class

Tip: Before tackling complexity, make sure that you eliminate any duplication first. This often can reduce the size of classes significantly.

Complex classes like BoardService often do a lot of different things. To break such a class down, we need to identify a cohesive component within that class. A common approach to find such a component is to look for fields/methods that share the same prefixes, or suffixes. You can also have a look at the cohesion graph to spot any un-connected, or weakly-connected components.

Once you have determined the fields that belong together, you can apply the Extract Class refactoring. If the component makes sense as a sub-class, Extract Subclass is also a candidate, and is often faster.

While breaking up the class, it is a good idea to analyze how other classes use BoardService, and based on these observations, apply Extract Interface, too.

<?php
/**
 * @copyright Copyright (c) 2016 Julius Härtl <[email protected]>
 *
 * @author Julius Härtl <[email protected]>
 *
 * @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\Deck\Service;

use OCA\Deck\Db\Acl;
use OCA\Deck\Db\AclMapper;
use OCA\Deck\Db\AssignedUsersMapper;
use OCA\Deck\Db\IPermissionMapper;
use OCA\Deck\Db\Label;
use OCA\Deck\Notification\NotificationHelper;
use OCP\AppFramework\Db\DoesNotExistException;
use OCP\IGroupManager;
use OCP\IL10N;
use OCA\Deck\Db\Board;
use OCA\Deck\Db\BoardMapper;
use OCA\Deck\Db\LabelMapper;
use OCP\IUserManager;
use OCA\Deck\BadRequestException;


class BoardService {

	private $boardMapper;
	private $labelMapper;
	private $aclMapper;
	private $l10n;
	private $permissionService;
	private $notificationHelper;
	private $assignedUsersMapper;
	private $userManager;
	private $groupManager;
	private $userId;

	public function __construct(

		BoardMapper $boardMapper,
		IL10N $l10n,
		LabelMapper $labelMapper,
		AclMapper $aclMapper,
		PermissionService $permissionService,
		NotificationHelper $notificationHelper,
		AssignedUsersMapper $assignedUsersMapper,
		IUserManager $userManager,
		IGroupManager $groupManager,
		$userId
	) {
		$this->boardMapper = $boardMapper;
		$this->labelMapper = $labelMapper;
		$this->aclMapper = $aclMapper;
		$this->l10n = $l10n;
		$this->permissionService = $permissionService;
		$this->notificationHelper = $notificationHelper;
		$this->assignedUsersMapper = $assignedUsersMapper;
		$this->userManager = $userManager;
		$this->groupManager = $groupManager;
		$this->userId = $userId;
	}

	/**
	 * @return array
	 */
	public function findAll() {
		$userInfo = $this->getBoardPrerequisites();
		$userBoards = $this->boardMapper->findAllByUser($userInfo['user']);
		$groupBoards = $this->boardMapper->findAllByGroups($userInfo['user'], $userInfo['groups']);
		$complete = array_merge($userBoards, $groupBoards);
		$result = [];
		foreach ($complete as &$item) {
			if (!array_key_exists($item->getId(), $result)) {
				$this->boardMapper->mapOwner($item);
				if ($item->getAcl() !== null) {
					foreach ($item->getAcl() as &$acl) {
foreach ($array as &$itemValue) { }
						$this->boardMapper->mapAcl($acl);
					}
				}
				$permissions = $this->permissionService->matchPermissions($item);
				$item->setPermissions([
					'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ],
					'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT],
					'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE],
					'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE]
				]);
				$result[$item->getId()] = $item;
			}
		}
		return array_values($result);
	}

	/**
	 * @param $boardId
	 * @return Board
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function find($boardId) {

		if ( is_numeric($boardId) === false ) {
			throw new BadRequestException('board id must be a number');
		}

		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
		/** @var Board $board */
		$board = $this->boardMapper->find($boardId, true, true);
		$this->boardMapper->mapOwner($board);
		foreach ($board->getAcl() as &$acl) {
foreach ($array as &$itemValue) { }
			if ($acl !== null) {
				$this->boardMapper->mapAcl($acl);
			}
		}
		$permissions = $this->permissionService->matchPermissions($board);
		$board->setPermissions([
			'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ],
			'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT],
			'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE],
			'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE]
		]);
		$boardUsers = $this->permissionService->findUsers($boardId);
		$board->setUsers(array_values($boardUsers));
		return $board;
	}

	/**
	 * @return array
	 */
	private function getBoardPrerequisites() {
		$groups = $this->groupManager->getUserGroupIds(
			$this->userManager->get($this->userId)
/** @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 [
			'user' => $this->userId,
			'groups' => $groups
		];
	}

	/**
	 * @param $mapper
	 * @param $id
	 * @return bool
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function isArchived($mapper, $id) {

		if (is_numeric($id) === false)  {
			throw new BadRequestException('id must be a number');
		}

		try {
			$boardId = $id;
			if ($mapper instanceof IPermissionMapper) {
				$boardId = $mapper->findBoardId($id);
			}
			if ($boardId === null) {
				return false;
			}
		} catch (DoesNotExistException $exception) {
			return false;
		}
		$board = $this->find($boardId);
		return $board->getArchived();
	}

	/**
	 * @param $mapper
	 * @param $id
	 * @return bool
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function isDeleted($mapper, $id) {

		if ($mapper === false || $mapper === null) {
			throw new BadRequestException('mapper must be provided');
		}

		if (is_numeric($id) === false)  {
			throw new BadRequestException('id must be a number');
		}

		try {
			$boardId = $id;
			if ($mapper instanceof IPermissionMapper) {
				$boardId = $mapper->findBoardId($id);
			}
			if ($boardId === null) {
				return false;
			}
		} catch (DoesNotExistException $exception) {
			return false;
		}
		$board = $this->find($boardId);
		return $board->getDeletedAt() > 0;
	}


	/**
	 * @param $title
	 * @param $userId
	 * @param $color
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws BadRequestException
	 */
	public function create($title, $userId, $color) {

		if ($title === false || $title === null) {
			throw new BadRequestException('title must be provided');
		}

		if ($userId === false || $userId === null) {
			throw new BadRequestException('userId must be provided');
		}

		if ($color === false || $color === null) {
			throw new BadRequestException('color must be provided');
		}

		$board = new Board();
		$board->setTitle($title);
		$board->setOwner($userId);
		$board->setColor($color);
		$new_board = $this->boardMapper->insert($board);		

		// create new labels
		$default_labels = [
			'31CC7C' => $this->l10n->t('Finished'),
			'317CCC' => $this->l10n->t('To review'),
			'FF7A66' => $this->l10n->t('Action needed'),
			'F1DB50' => $this->l10n->t('Later')
		];
		$labels = [];
		foreach ($default_labels as $labelColor => $labelTitle) {
			$label = new Label();
			$label->setColor($labelColor);
			$label->setTitle($labelTitle);
			$label->setBoardId($new_board->getId());
			$labels[] = $this->labelMapper->insert($label);
		}
		$new_board->setLabels($labels);
		$this->boardMapper->mapOwner($new_board);
		$permissions = $this->permissionService->matchPermissions($new_board);
		$new_board->setPermissions([
			'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ],
			'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT],
			'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE],
			'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE]
		]);
		return $new_board;

	}

	/**
	 * @param $id
	 * @return Board
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function delete($id) {


		if (is_numeric($id) === false) {
			throw new BadRequestException('board id must be a number');
		}

		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
		$board = $this->find($id);
		$board->setDeletedAt(time());
		$this->boardMapper->update($board);
		return $board;
	}

	/**
	 * @param $id
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 */
	public function deleteUndo($id) {


		if (is_numeric($id) === false) {
			throw new BadRequestException('board id must be a number');
		}

		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
		$board = $this->find($id);
		$board->setDeletedAt(0);
		return $this->boardMapper->update($board);
	}

	/**
	 * @param $id
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function deleteForce($id) {		
		if (is_numeric($id) === false)  {
			throw new BadRequestException('id must be a number');
		}

		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
		$board = $this->find($id);
		return $this->boardMapper->delete($board);
	}

	/**
	 * @param $id
	 * @param $title
	 * @param $color
	 * @param $archived
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function update($id, $title, $color, $archived) {

		if (is_numeric($id) === false) {
			throw new BadRequestException('board id must be a number');
		}

		if ($title === false || $title === null) {
			throw new BadRequestException('color must be provided');
		}

		if ($color === false || $color === null) {
			throw new BadRequestException('color must be provided');
		}

		if ( is_bool($archived) === false ) {
			throw new BadRequestException('archived must be a boolean');
		}

		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE);
		$board = $this->find($id);
		$board->setTitle($title);
		$board->setColor($color);
		$board->setArchived($archived);
		$this->boardMapper->mapOwner($board);
		return $this->boardMapper->update($board);
	}


	/**
	 * @param $boardId
	 * @param $type
	 * @param $participant
	 * @param $edit
	 * @param $share
	 * @param $manage
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws \OCA\Deck\
	 * @throws BadRequestException
	 */
	public function addAcl($boardId, $type, $participant, $edit, $share, $manage) {

		if (is_numeric($boardId) === false) {
			throw new BadRequestException('board id must be a number');
		}

		if ($type === false || $type === null) {
			throw new BadRequestException('type must be provided');
		}

		if ($participant === false || $participant === null) {
			throw new BadRequestException('participant must be provided');
		}

		if ($edit === false || $edit === null) {
			throw new BadRequestException('edit must be provided');
		}

		if ($share === false || $share === null) {
			throw new BadRequestException('share must be provided');
		}

		if ($manage === false || $manage === null) {
			throw new BadRequestException('manage must be provided');
		}

		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_SHARE);
		$acl = new Acl();
		$acl->setBoardId($boardId);
		$acl->setType($type);
		$acl->setParticipant($participant);
		$acl->setPermissionEdit($edit);
		$acl->setPermissionShare($share);
		$acl->setPermissionManage($manage);

		/* Notify users about the shared board */
		$this->notificationHelper->sendBoardShared($boardId, $acl);

		$newAcl = $this->aclMapper->insert($acl);
		$this->boardMapper->mapAcl($newAcl);
		return $newAcl;
	}

	/**
	 * @param $id
	 * @param $edit
	 * @param $share
	 * @param $manage
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function updateAcl($id, $edit, $share, $manage) {

		if (is_numeric($id) === false) {
			throw new BadRequestException('id must be a number');
		}

		if ($edit === null) {
			throw new BadRequestException('edit must be provided');
		}

		if ($share === null) {
			throw new BadRequestException('share must be provided');
		}

		if ($manage === null) {
			throw new BadRequestException('manage must be provided');
		}

		$this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE);
		/** @var Acl $acl */
		$acl = $this->aclMapper->find($id);
		$acl->setPermissionEdit($edit);
		$acl->setPermissionShare($share);
		$acl->setPermissionManage($manage);
		$this->boardMapper->mapAcl($acl);
		return $this->aclMapper->update($acl);
	}

	/**
	 * @param $id
	 * @return \OCP\AppFramework\Db\Entity
	 * @throws DoesNotExistException
	 * @throws \OCA\Deck\NoPermissionException
	 * @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
	 * @throws BadRequestException
	 */
	public function deleteAcl($id) {

		if (is_numeric($id) === false) {
			throw new BadRequestException('id must be a number');
		}

		$this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE);
		/** @var Acl $acl */
		$acl = $this->aclMapper->find($id);
		$this->boardMapper->mapAcl($acl);
		if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
			$assignements = $this->assignedUsersMapper->findByUserId($acl->getParticipant());
			foreach ($assignements as $assignement) {
				$this->assignedUsersMapper->delete($assignement);
			}
		}
		return $this->aclMapper->delete($acl);
	}

}


Push — master ( a3a70f...f040df )

BoardService F

Complexity

Size/Duplication

Coupling/Cohesion

Importance

14 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

1. Do not assign by reference

2. Assign to a local variable first

3. Return a reference

Available Fixes

1. Do not assign by reference

2. Assign to a local variable first

3. Return a reference

1		<?php
2		/**
3		* @copyright Copyright (c) 2016 Julius Härtl <[email protected]>
4		*
5		* @author Julius Härtl <[email protected]>
6		*
7		* @license GNU AGPL version 3 or any later version
8		*
9		* This program is free software: you can redistribute it and/or modify
10		* it under the terms of the GNU Affero General Public License as
11		* published by the Free Software Foundation, either version 3 of the
12		* License, or (at your option) any later version.
13		*
14		* This program is distributed in the hope that it will be useful,
15		* but WITHOUT ANY WARRANTY; without even the implied warranty of
16		* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
17		* GNU Affero General Public License for more details.
18		*
19		* You should have received a copy of the GNU Affero General Public License
20		* along with this program. If not, see <http://www.gnu.org/licenses/>.
21		*
22		*/
23
24		namespace OCA\Deck\Service;
25
26		use OCA\Deck\Db\Acl;
27		use OCA\Deck\Db\AclMapper;
28		use OCA\Deck\Db\AssignedUsersMapper;
29		use OCA\Deck\Db\IPermissionMapper;
30		use OCA\Deck\Db\Label;
31		use OCA\Deck\Notification\NotificationHelper;
32		use OCP\AppFramework\Db\DoesNotExistException;
33		use OCP\IGroupManager;
34		use OCP\IL10N;
35		use OCA\Deck\Db\Board;
36		use OCA\Deck\Db\BoardMapper;
37		use OCA\Deck\Db\LabelMapper;
38		use OCP\IUserManager;
39		use OCA\Deck\BadRequestException;
40
41
42		class BoardService {
43
44		private $boardMapper;
45		private $labelMapper;
46		private $aclMapper;
47		private $l10n;
48		private $permissionService;
49		private $notificationHelper;
50		private $assignedUsersMapper;
51		private $userManager;
52		private $groupManager;
53		private $userId;
54
55	View Code Duplication	public function __construct(
		0 ignored issues – show Duplication introduced 2018-08-15 19:05 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...
56		BoardMapper $boardMapper,
57		IL10N $l10n,
58		LabelMapper $labelMapper,
59		AclMapper $aclMapper,
60		PermissionService $permissionService,
61		NotificationHelper $notificationHelper,
62		AssignedUsersMapper $assignedUsersMapper,
63		IUserManager $userManager,
64		IGroupManager $groupManager,
65		$userId
66		) {
67		$this->boardMapper = $boardMapper;
68		$this->labelMapper = $labelMapper;
69		$this->aclMapper = $aclMapper;
70		$this->l10n = $l10n;
71		$this->permissionService = $permissionService;
72		$this->notificationHelper = $notificationHelper;
73		$this->assignedUsersMapper = $assignedUsersMapper;
74		$this->userManager = $userManager;
75		$this->groupManager = $groupManager;
76		$this->userId = $userId;
77		}
78
79		/**
80		* @return array
81		*/
82		public function findAll() {
83		$userInfo = $this->getBoardPrerequisites();
84		$userBoards = $this->boardMapper->findAllByUser($userInfo['user']);
85		$groupBoards = $this->boardMapper->findAllByGroups($userInfo['user'], $userInfo['groups']);
86		$complete = array_merge($userBoards, $groupBoards);
87		$result = [];
88		foreach ($complete as &$item) {
89		if (!array_key_exists($item->getId(), $result)) {
90		$this->boardMapper->mapOwner($item);
91		if ($item->getAcl() !== null) {
92		foreach ($item->getAcl() as &$acl) {
		0 ignored issues – show Bug introduced 2017-02-28 12:55 UTC by Report Bug Copy Issue Report The expression `$item->getAcl()` cannot be used as a reference. Let?s assume that you have the following `foreach` statement: foreach ($array as &$itemValue) { } `$itemValue` is assigned by reference. This is possible because the expression (in the example `$array`) can be used as a reference target. However, if we were to replace `$array` with something different like the result of a function call as in foreach (getArray() as &$itemValue) { } then assigning by reference is not possible anymore as there is no target that could be modified. Available Fixes 1. Do not assign by reference foreach (getArray() as $itemValue) { } 2. Assign to a local variable first $array = getArray(); foreach ($array as &$itemValue) {} 3. Return a reference function &getArray() { $array = array(); return $array; } foreach (getArray() as &$itemValue) { } Loading history...
93		$this->boardMapper->mapAcl($acl);
94		}
95		}
96		$permissions = $this->permissionService->matchPermissions($item);
97		$item->setPermissions([
98		'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ],
99		'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT],
100		'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE],
101		'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE]
102		]);
103		$result[$item->getId()] = $item;
104		}
105		}
106		return array_values($result);
107		}
108
109		/**
110		* @param $boardId
111		* @return Board
112		* @throws DoesNotExistException
113		* @throws \OCA\Deck\NoPermissionException
114		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
115		* @throws BadRequestException
116		*/
117		public function find($boardId) {
118
119		if ( is_numeric($boardId) === false ) {
120		throw new BadRequestException('board id must be a number');
121		}
122
123		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_READ);
124		/** @var Board $board */
125		$board = $this->boardMapper->find($boardId, true, true);
126		$this->boardMapper->mapOwner($board);
127		foreach ($board->getAcl() as &$acl) {
		0 ignored issues – show Bug introduced 2017-02-28 12:55 UTC by Report Bug Copy Issue Report The expression `$board->getAcl()` cannot be used as a reference. Let?s assume that you have the following `foreach` statement: foreach ($array as &$itemValue) { } `$itemValue` is assigned by reference. This is possible because the expression (in the example `$array`) can be used as a reference target. However, if we were to replace `$array` with something different like the result of a function call as in foreach (getArray() as &$itemValue) { } then assigning by reference is not possible anymore as there is no target that could be modified. Available Fixes 1. Do not assign by reference foreach (getArray() as $itemValue) { } 2. Assign to a local variable first $array = getArray(); foreach ($array as &$itemValue) {} 3. Return a reference function &getArray() { $array = array(); return $array; } foreach (getArray() as &$itemValue) { } Loading history...
128		if ($acl !== null) {
129		$this->boardMapper->mapAcl($acl);
130		}
131		}
132		$permissions = $this->permissionService->matchPermissions($board);
133		$board->setPermissions([
134		'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ],
135		'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT],
136		'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE],
137		'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE]
138		]);
139		$boardUsers = $this->permissionService->findUsers($boardId);
140		$board->setUsers(array_values($boardUsers));
141		return $board;
142		}
143
144		/**
145		* @return array
146		*/
147		private function getBoardPrerequisites() {
148		$groups = $this->groupManager->getUserGroupIds(
149		$this->userManager->get($this->userId)
		0 ignored issues – show Bug introduced 2017-10-03 17:09 UTC by Report Bug Copy Issue Report It seems like `$this->userManager->get($this->userId)` can be `null`; however, `getUserGroupIds()` 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...
150		);
151		return [
152		'user' => $this->userId,
153		'groups' => $groups
154		];
155		}
156
157		/**
158		* @param $mapper
159		* @param $id
160		* @return bool
161		* @throws DoesNotExistException
162		* @throws \OCA\Deck\NoPermissionException
163		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
164		* @throws BadRequestException
165		*/
166		public function isArchived($mapper, $id) {
167
168		if (is_numeric($id) === false) {
169		throw new BadRequestException('id must be a number');
170		}
171
172		try {
173		$boardId = $id;
174		if ($mapper instanceof IPermissionMapper) {
175		$boardId = $mapper->findBoardId($id);
176		}
177		if ($boardId === null) {
178		return false;
179		}
180		} catch (DoesNotExistException $exception) {
181		return false;
182		}
183		$board = $this->find($boardId);
184		return $board->getArchived();
185		}
186
187		/**
188		* @param $mapper
189		* @param $id
190		* @return bool
191		* @throws DoesNotExistException
192		* @throws \OCA\Deck\NoPermissionException
193		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
194		* @throws BadRequestException
195		*/
196		public function isDeleted($mapper, $id) {
197
198		if ($mapper === false \|\| $mapper === null) {
199		throw new BadRequestException('mapper must be provided');
200		}
201
202		if (is_numeric($id) === false) {
203		throw new BadRequestException('id must be a number');
204		}
205
206		try {
207		$boardId = $id;
208		if ($mapper instanceof IPermissionMapper) {
209		$boardId = $mapper->findBoardId($id);
210		}
211		if ($boardId === null) {
212		return false;
213		}
214		} catch (DoesNotExistException $exception) {
215		return false;
216		}
217		$board = $this->find($boardId);
218		return $board->getDeletedAt() > 0;
219		}
220
221
222		/**
223		* @param $title
224		* @param $userId
225		* @param $color
226		* @return \OCP\AppFramework\Db\Entity
227		* @throws BadRequestException
228		*/
229		public function create($title, $userId, $color) {
230
231		if ($title === false \|\| $title === null) {
232		throw new BadRequestException('title must be provided');
233		}
234
235		if ($userId === false \|\| $userId === null) {
236		throw new BadRequestException('userId must be provided');
237		}
238
239		if ($color === false \|\| $color === null) {
240		throw new BadRequestException('color must be provided');
241		}
242
243		$board = new Board();
244		$board->setTitle($title);
245		$board->setOwner($userId);
246		$board->setColor($color);
247		$new_board = $this->boardMapper->insert($board);
248
249		// create new labels
250		$default_labels = [
251		'31CC7C' => $this->l10n->t('Finished'),
252		'317CCC' => $this->l10n->t('To review'),
253		'FF7A66' => $this->l10n->t('Action needed'),
254		'F1DB50' => $this->l10n->t('Later')
255		];
256		$labels = [];
257		foreach ($default_labels as $labelColor => $labelTitle) {
258		$label = new Label();
259		$label->setColor($labelColor);
260		$label->setTitle($labelTitle);
261		$label->setBoardId($new_board->getId());
262		$labels[] = $this->labelMapper->insert($label);
263		}
264		$new_board->setLabels($labels);
265		$this->boardMapper->mapOwner($new_board);
266		$permissions = $this->permissionService->matchPermissions($new_board);
267		$new_board->setPermissions([
268		'PERMISSION_READ' => $permissions[Acl::PERMISSION_READ],
269		'PERMISSION_EDIT' => $permissions[Acl::PERMISSION_EDIT],
270		'PERMISSION_MANAGE' => $permissions[Acl::PERMISSION_MANAGE],
271		'PERMISSION_SHARE' => $permissions[Acl::PERMISSION_SHARE]
272		]);
273		return $new_board;
274
275		}
276
277		/**
278		* @param $id
279		* @return Board
280		* @throws DoesNotExistException
281		* @throws \OCA\Deck\NoPermissionException
282		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
283		* @throws BadRequestException
284		*/
285	View Code Duplication	public function delete($id) {
		0 ignored issues – show Duplication introduced 2018-07-20 15:20 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...
286
287		if (is_numeric($id) === false) {
288		throw new BadRequestException('board id must be a number');
289		}
290
291		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
292		$board = $this->find($id);
293		$board->setDeletedAt(time());
294		$this->boardMapper->update($board);
295		return $board;
296		}
297
298		/**
299		* @param $id
300		* @return \OCP\AppFramework\Db\Entity
301		* @throws DoesNotExistException
302		* @throws \OCA\Deck\NoPermissionException
303		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
304		*/
305	View Code Duplication	public function deleteUndo($id) {
		0 ignored issues – show Duplication introduced 2018-07-16 13:57 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...
306
307		if (is_numeric($id) === false) {
308		throw new BadRequestException('board id must be a number');
309		}
310
311		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
312		$board = $this->find($id);
313		$board->setDeletedAt(0);
314		return $this->boardMapper->update($board);
315		}
316
317		/**
318		* @param $id
319		* @return \OCP\AppFramework\Db\Entity
320		* @throws DoesNotExistException
321		* @throws \OCA\Deck\NoPermissionException
322		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
323		* @throws BadRequestException
324		*/
325		public function deleteForce($id) {
326		if (is_numeric($id) === false) {
327		throw new BadRequestException('id must be a number');
328		}
329
330		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_READ);
331		$board = $this->find($id);
332		return $this->boardMapper->delete($board);
333		}
334
335		/**
336		* @param $id
337		* @param $title
338		* @param $color
339		* @param $archived
340		* @return \OCP\AppFramework\Db\Entity
341		* @throws DoesNotExistException
342		* @throws \OCA\Deck\NoPermissionException
343		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
344		* @throws BadRequestException
345		*/
346		public function update($id, $title, $color, $archived) {
347
348		if (is_numeric($id) === false) {
349		throw new BadRequestException('board id must be a number');
350		}
351
352		if ($title === false \|\| $title === null) {
353		throw new BadRequestException('color must be provided');
354		}
355
356		if ($color === false \|\| $color === null) {
357		throw new BadRequestException('color must be provided');
358		}
359
360		if ( is_bool($archived) === false ) {
361		throw new BadRequestException('archived must be a boolean');
362		}
363
364		$this->permissionService->checkPermission($this->boardMapper, $id, Acl::PERMISSION_MANAGE);
365		$board = $this->find($id);
366		$board->setTitle($title);
367		$board->setColor($color);
368		$board->setArchived($archived);
369		$this->boardMapper->mapOwner($board);
370		return $this->boardMapper->update($board);
371		}
372
373
374		/**
375		* @param $boardId
376		* @param $type
377		* @param $participant
378		* @param $edit
379		* @param $share
380		* @param $manage
381		* @return \OCP\AppFramework\Db\Entity
382		* @throws \OCA\Deck\
383		* @throws BadRequestException
384		*/
385		public function addAcl($boardId, $type, $participant, $edit, $share, $manage) {
386
387		if (is_numeric($boardId) === false) {
388		throw new BadRequestException('board id must be a number');
389		}
390
391		if ($type === false \|\| $type === null) {
392		throw new BadRequestException('type must be provided');
393		}
394
395		if ($participant === false \|\| $participant === null) {
396		throw new BadRequestException('participant must be provided');
397		}
398
399		if ($edit === false \|\| $edit === null) {
400		throw new BadRequestException('edit must be provided');
401		}
402
403		if ($share === false \|\| $share === null) {
404		throw new BadRequestException('share must be provided');
405		}
406
407		if ($manage === false \|\| $manage === null) {
408		throw new BadRequestException('manage must be provided');
409		}
410
411		$this->permissionService->checkPermission($this->boardMapper, $boardId, Acl::PERMISSION_SHARE);
412		$acl = new Acl();
413		$acl->setBoardId($boardId);
414		$acl->setType($type);
415		$acl->setParticipant($participant);
416		$acl->setPermissionEdit($edit);
417		$acl->setPermissionShare($share);
418		$acl->setPermissionManage($manage);
419
420		/* Notify users about the shared board */
421		$this->notificationHelper->sendBoardShared($boardId, $acl);
422
423		$newAcl = $this->aclMapper->insert($acl);
424		$this->boardMapper->mapAcl($newAcl);
425		return $newAcl;
426		}
427
428		/**
429		* @param $id
430		* @param $edit
431		* @param $share
432		* @param $manage
433		* @return \OCP\AppFramework\Db\Entity
434		* @throws DoesNotExistException
435		* @throws \OCA\Deck\NoPermissionException
436		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
437		* @throws BadRequestException
438		*/
439		public function updateAcl($id, $edit, $share, $manage) {
440
441		if (is_numeric($id) === false) {
442		throw new BadRequestException('id must be a number');
443		}
444
445		if ($edit === null) {
446		throw new BadRequestException('edit must be provided');
447		}
448
449		if ($share === null) {
450		throw new BadRequestException('share must be provided');
451		}
452
453		if ($manage === null) {
454		throw new BadRequestException('manage must be provided');
455		}
456
457		$this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE);
458		/** @var Acl $acl */
459		$acl = $this->aclMapper->find($id);
460		$acl->setPermissionEdit($edit);
461		$acl->setPermissionShare($share);
462		$acl->setPermissionManage($manage);
463		$this->boardMapper->mapAcl($acl);
464		return $this->aclMapper->update($acl);
465		}
466
467		/**
468		* @param $id
469		* @return \OCP\AppFramework\Db\Entity
470		* @throws DoesNotExistException
471		* @throws \OCA\Deck\NoPermissionException
472		* @throws \OCP\AppFramework\Db\MultipleObjectsReturnedException
473		* @throws BadRequestException
474		*/
475		public function deleteAcl($id) {
476
477		if (is_numeric($id) === false) {
478		throw new BadRequestException('id must be a number');
479		}
480
481		$this->permissionService->checkPermission($this->aclMapper, $id, Acl::PERMISSION_SHARE);
482		/** @var Acl $acl */
483		$acl = $this->aclMapper->find($id);
484		$this->boardMapper->mapAcl($acl);
485		if ($acl->getType() === Acl::PERMISSION_TYPE_USER) {
486		$assignements = $this->assignedUsersMapper->findByUserId($acl->getParticipant());
487		foreach ($assignements as $assignement) {
488		$this->assignedUsersMapper->delete($assignement);
489		}
490		}
491		return $this->aclMapper->delete($acl);
492		}
493
494		}
495

nextcloud / deck

Push — master ( a3a70f...f040df )

BoardService F

Complexity

Size/Duplication

Coupling/Cohesion

Importance

14 Methods

How to fix Duplicated Code Complexity

Duplicated Code

Complex Class

Available Fixes

1. Do not assign by reference

2. Assign to a local variable first

3. Return a reference

Available Fixes

1. Do not assign by reference

2. Assign to a local variable first

3. Return a reference

Duplication Side-by-Side

Filter issues like