Issues in Applicable.php - Fixed Issues - Inspection of "commit transaction after select query" - owncloud/core - Measure and Improve Code Quality continuously with Scrutinizer

Completed

Pull Request — master (#26700)

by Philipp

created 2016-11-23 11:35 UTC

apps/files_external/lib/Command/Applicable.php (1 issue)

Labels

Bug 1

Severity

Minor 1

Upgrade to new PHP Analysis Engine

These results are based on our legacy PHP analysis, consider migrating to our new PHP analysis engine instead. Learn more

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

namespace OCA\Files_External\Command;

use OC\Core\Command\Base;
use OCA\Files_External\Lib\StorageConfig;
use OCA\Files_External\NotFoundException;
use OCA\Files_External\Service\GlobalStoragesService;
use OCP\IGroupManager;
use OCP\IUserManager;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
use Symfony\Component\Console\Output\OutputInterface;

class Applicable extends Base {
	/**
	 * @var GlobalStoragesService
	 */
	protected $globalService;

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

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

	function __construct(
		GlobalStoragesService $globalService,
		IUserManager $userManager,
		IGroupManager $groupManager
	) {
		parent::__construct();
		$this->globalService = $globalService;
		$this->userManager = $userManager;
		$this->groupManager = $groupManager;
	}

	protected function configure() {
		$this
			->setName('files_external:applicable')
			->setDescription('Manage applicable users and groups for a mount')
			->addArgument(
				'mount_id',
				InputArgument::REQUIRED,
				'The id of the mount to edit'
			)->addOption(
				'add-user',
				null,
				InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED,
				'user to add as applicable'
			)->addOption(
				'remove-user',
				null,
				InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED,
				'user to remove as applicable'
			)->addOption(
				'add-group',
				null,
				InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED,
				'group to add as applicable'
			)->addOption(
				'remove-group',
				null,
				InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED,
				'group to remove as applicable'
			)->addOption(
				'remove-all',
				null,
				InputOption::VALUE_NONE,
				'Set the mount to be globally applicable'
			);
		parent::configure();
	}

	protected function execute(InputInterface $input, OutputInterface $output) {
		$mountId = $input->getArgument('mount_id');
		try {
			$mount = $this->globalService->getStorage($mountId);
		} catch (NotFoundException $e) {
			$output->writeln('<error>Mount with id "' . $mountId . ' not found, check "occ files_external:list" to get available mounts</error>');
			return 404;
		}

		if ($mount->getType() === StorageConfig::MOUNT_TYPE_PERSONAl) {
			$output->writeln('<error>Can\'t change applicables on personal mounts</error>');
			return 1;
		}

		$addUsers = $input->getOption('add-user');
		$removeUsers = $input->getOption('remove-user');
		$addGroups = $input->getOption('add-group');
		$removeGroups = $input->getOption('remove-group');

		$applicableUsers = $mount->getApplicableUsers();
		$applicableGroups = $mount->getApplicableGroups();

		if ((count($addUsers) + count($removeUsers) + count($addGroups) + count($removeGroups) > 0) || $input->getOption('remove-all')) {
			foreach ($addUsers as $addUser) {
				if (!$this->userManager->userExists($addUser)) {
					$output->writeln('<error>User "' . $addUser . '" not found</error>');
					return 404;
				}
			}
			foreach ($addGroups as $addGroup) {
				if (!$this->groupManager->groupExists($addGroup)) {
					$output->writeln('<error>Group "' . $addGroup . '" not found</error>');
					return 404;
				}
			}

			if ($input->getOption('remove-all')) {
				$applicableUsers = [];
				$applicableGroups = [];
			} else {
				$applicableUsers = array_unique(array_merge($applicableUsers, $addUsers));
				$applicableUsers = array_values(array_diff($applicableUsers, $removeUsers));
				$applicableGroups = array_unique(array_merge($applicableGroups, $addGroups));
				$applicableGroups = array_values(array_diff($applicableGroups, $removeGroups));
			}
			$mount->setApplicableUsers($applicableUsers);
			$mount->setApplicableGroups($applicableGroups);
			$this->globalService->updateStorage($mount);
/** @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->writeArrayInOutputFormat($input, $output, [
			'users' => $applicableUsers,
			'groups' => $applicableGroups
		]);
	}
}


1			<?php
2			/**
3			* @author Joas Schilling <[email protected]>
4			* @author Robin Appelman <[email protected]>
5			*
6			* @copyright Copyright (c) 2016, ownCloud GmbH.
7			* @license AGPL-3.0
8			*
9			* This code is free software: you can redistribute it and/or modify
10			* it under the terms of the GNU Affero General Public License, version 3,
11			* as published by the Free Software Foundation.
12			*
13			* This program is distributed in the hope that it will be useful,
14			* but WITHOUT ANY WARRANTY; without even the implied warranty of
15			* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
16			* GNU Affero General Public License for more details.
17			*
18			* You should have received a copy of the GNU Affero General Public License, version 3,
19			* along with this program. If not, see <http://www.gnu.org/licenses/>
20			*
21			*/
22
23			namespace OCA\Files_External\Command;
24
25			use OC\Core\Command\Base;
26			use OCA\Files_External\Lib\StorageConfig;
27			use OCA\Files_External\NotFoundException;
28			use OCA\Files_External\Service\GlobalStoragesService;
29			use OCP\IGroupManager;
30			use OCP\IUserManager;
31			use Symfony\Component\Console\Input\InputArgument;
32			use Symfony\Component\Console\Input\InputInterface;
33			use Symfony\Component\Console\Input\InputOption;
34			use Symfony\Component\Console\Output\OutputInterface;
35
36			class Applicable extends Base {
37			/**
38			* @var GlobalStoragesService
39			*/
40			protected $globalService;
41
42			/**
43			* @var IUserManager
44			*/
45			private $userManager;
46
47			/**
48			* @var IGroupManager
49			*/
50			private $groupManager;
51
52			function __construct(
53			GlobalStoragesService $globalService,
54			IUserManager $userManager,
55			IGroupManager $groupManager
56			) {
57			parent::__construct();
58			$this->globalService = $globalService;
59			$this->userManager = $userManager;
60			$this->groupManager = $groupManager;
61			}
62
63			protected function configure() {
64			$this
65			->setName('files_external:applicable')
66			->setDescription('Manage applicable users and groups for a mount')
67			->addArgument(
68			'mount_id',
69			InputArgument::REQUIRED,
70			'The id of the mount to edit'
71			)->addOption(
72			'add-user',
73			null,
74			InputOption::VALUE_IS_ARRAY \| InputOption::VALUE_REQUIRED,
75			'user to add as applicable'
76			)->addOption(
77			'remove-user',
78			null,
79			InputOption::VALUE_IS_ARRAY \| InputOption::VALUE_REQUIRED,
80			'user to remove as applicable'
81			)->addOption(
82			'add-group',
83			null,
84			InputOption::VALUE_IS_ARRAY \| InputOption::VALUE_REQUIRED,
85			'group to add as applicable'
86			)->addOption(
87			'remove-group',
88			null,
89			InputOption::VALUE_IS_ARRAY \| InputOption::VALUE_REQUIRED,
90			'group to remove as applicable'
91			)->addOption(
92			'remove-all',
93			null,
94			InputOption::VALUE_NONE,
95			'Set the mount to be globally applicable'
96			);
97			parent::configure();
98			}
99
100			protected function execute(InputInterface $input, OutputInterface $output) {
101			$mountId = $input->getArgument('mount_id');
102			try {
103			$mount = $this->globalService->getStorage($mountId);
104			} catch (NotFoundException $e) {
105			$output->writeln('<error>Mount with id "' . $mountId . ' not found, check "occ files_external:list" to get available mounts</error>');
106			return 404;
107			}
108
109			if ($mount->getType() === StorageConfig::MOUNT_TYPE_PERSONAl) {
110			$output->writeln('<error>Can\'t change applicables on personal mounts</error>');
111			return 1;
112			}
113
114			$addUsers = $input->getOption('add-user');
115			$removeUsers = $input->getOption('remove-user');
116			$addGroups = $input->getOption('add-group');
117			$removeGroups = $input->getOption('remove-group');
118
119			$applicableUsers = $mount->getApplicableUsers();
120			$applicableGroups = $mount->getApplicableGroups();
121
122			if ((count($addUsers) + count($removeUsers) + count($addGroups) + count($removeGroups) > 0) \|\| $input->getOption('remove-all')) {
123			foreach ($addUsers as $addUser) {
124			if (!$this->userManager->userExists($addUser)) {
125			$output->writeln('<error>User "' . $addUser . '" not found</error>');
126			return 404;
127			}
128			}
129			foreach ($addGroups as $addGroup) {
130			if (!$this->groupManager->groupExists($addGroup)) {
131			$output->writeln('<error>Group "' . $addGroup . '" not found</error>');
132			return 404;
133			}
134			}
135
136			if ($input->getOption('remove-all')) {
137			$applicableUsers = [];
138			$applicableGroups = [];
139			} else {
140			$applicableUsers = array_unique(array_merge($applicableUsers, $addUsers));
141			$applicableUsers = array_values(array_diff($applicableUsers, $removeUsers));
142			$applicableGroups = array_unique(array_merge($applicableGroups, $addGroups));
143			$applicableGroups = array_values(array_diff($applicableGroups, $removeGroups));
144			}
145			$mount->setApplicableUsers($applicableUsers);
146			$mount->setApplicableGroups($applicableGroups);
147			$this->globalService->updateStorage($mount);
			0 ignored issues – show Bug introduced 2016-05-13 10:19 UTC by Report Bug Copy Issue Report Show Similar Issues like this It seems like `$mount` defined by `$this->globalService->getStorage($mountId)` on line `103` can be `null`; however, `OCA\Files_External\Servi...ervice::updateStorage()` 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
150			$this->writeArrayInOutputFormat($input, $output, [
151			'users' => $applicableUsers,
152			'groups' => $applicableGroups
153			]);
154			}
155			}
156

owncloud / core

Pull Request — master (#26700)

apps/files_external/lib/Command/Applicable.php (1 issue)

Labels

Severity

Introduced By

Upgrade to new PHP Analysis Engine

Duplication Side-by-Side

Filter issues like