From b96f0c99b0cf20b6997e1019c624c1848c006434 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 9 Jun 2016 14:49:53 +0200 Subject: [PATCH 1/9] Add a occ command to list/get user preferences --- core/Command/User/Setting.php | 160 ++++++++++++++++++++++++++++++++++ core/register_command.php | 1 + 2 files changed, 161 insertions(+) create mode 100644 core/Command/User/Setting.php diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php new file mode 100644 index 00000000000..887439ba2e0 --- /dev/null +++ b/core/Command/User/Setting.php @@ -0,0 +1,160 @@ + + * @author Laurens Post + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @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 + * + */ + +namespace OC\Core\Command\User; + +use OC\Core\Command\Base; +use OCP\IConfig; +use OCP\IDBConnection; +use OCP\IUserManager; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Input\InputOption; +use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Input\InputArgument; + +class Setting extends Base { + /** @var IUserManager */ + protected $userManager; + + /** @var IConfig */ + protected $config; + + /** @var IDBConnection */ + protected $connection; + + /** + * @param IUserManager $userManager + * @param IConfig $config + * @param IDBConnection $connection + */ + public function __construct(IUserManager $userManager, IConfig $config, IDBConnection $connection) { + parent::__construct(); + $this->userManager = $userManager; + $this->config = $config; + $this->connection = $connection; + } + + protected function configure() { + parent::configure(); + $this + ->setName('user:setting') + ->setDescription('adds a user') + ->addArgument( + 'uid', + InputArgument::REQUIRED, + 'User ID used to login' + ) + ->addArgument( + 'app', + InputArgument::OPTIONAL, + 'Restrict the preferences to a given app', + '' + ) + ->addArgument( + 'key', + InputArgument::OPTIONAL, + 'Setting key to set, get or delete', + '' + ) + ->addOption( + 'default-value', + null, + InputOption::VALUE_REQUIRED, + '(Only applicable on get) If no default value is set and the config does not exist, the command will exit with 1' + ) + ->addOption( + 'ignore-missing-user', + null, + InputOption::VALUE_NONE, + 'Use this option to ignore errors when the user does not exist' + ) + ; + } + + protected function checkInput(InputInterface $input) { + $uid = $input->getArgument('uid'); + if (!$input->getOption('ignore-missing-user') && !$this->userManager->userExists($uid)) { + throw new \InvalidArgumentException('The user "' . $uid . '" does not exists.'); + } + + if ($input->getArgument('key') === '' && $input->hasParameterOption('--default-value')) { + throw new \InvalidArgumentException('The default-value option can only be used when getting a single setting.'); + } + } + + protected function execute(InputInterface $input, OutputInterface $output) { + try { + $this->checkInput($input); + } catch (\InvalidArgumentException $e) { + $output->writeln('' . $e->getMessage() . ''); + return 1; + } + + $uid = $input->getArgument('uid'); + $app = $input->getArgument('app'); + $key = $input->getArgument('key'); + + if ($key !== '') { + $value = $this->config->getUserValue($uid, $app, $key, null); + if ($value !== null) { + $output->writeln($value); + } else { + if ($input->hasParameterOption('--default-value')) { + $output->writeln($input->getOption('default-value')); + } else { + $output->writeln('The setting does not exist for user "' . $uid . '".'); + return 1; + } + } + } else { + $this->listUserSettings($input, $output, $uid, $app); + } + + return 0; + } + + protected function listUserSettings(InputInterface $input, OutputInterface $output, $uid, $app) { + $settings = $this->getUserSettings($uid, $app); + + $this->writeArrayInOutputFormat($input, $output, $settings); + } + + protected function getUserSettings($uid, $app) { + $query = $this->connection->getQueryBuilder(); + $query->select('*') + ->from('preferences') + ->where($query->expr()->eq('userid', $query->createNamedParameter($uid))); + + if ($app !== '') { + $query->andWhere($query->expr()->eq('appid', $query->createNamedParameter($app))); + } + + $result = $query->execute(); + $settings = []; + while ($row = $result->fetch()) { + $settings[$row['appid']][$row['configkey']] = $row['configvalue']; + } + $result->closeCursor(); + + return $settings; + } +} diff --git a/core/register_command.php b/core/register_command.php index ebb6ce8b723..91b00df20f1 100644 --- a/core/register_command.php +++ b/core/register_command.php @@ -136,6 +136,7 @@ if (\OC::$server->getConfig()->getSystemValue('installed', false)) { $application->add(new OC\Core\Command\User\LastSeen(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\Report(\OC::$server->getUserManager())); $application->add(new OC\Core\Command\User\ResetPassword(\OC::$server->getUserManager())); + $application->add(new OC\Core\Command\User\Setting(\OC::$server->getUserManager(), \OC::$server->getConfig(), \OC::$server->getDatabaseConnection())); $application->add(new OC\Core\Command\Security\ListCertificates(\OC::$server->getCertificateManager(null), \OC::$server->getL10N('core'))); $application->add(new OC\Core\Command\Security\ImportCertificate(\OC::$server->getCertificateManager(null))); From c3c7a5fd2c167b297b8186af693685b3d430bdb9 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 9 Jun 2016 15:07:06 +0200 Subject: [PATCH 2/9] Allow setting values --- core/Command/User/Setting.php | 39 +++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index 887439ba2e0..568f1cc2d09 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -75,17 +75,33 @@ class Setting extends Base { 'Setting key to set, get or delete', '' ) + ->addOption( + 'ignore-missing-user', + null, + InputOption::VALUE_NONE, + 'Use this option to ignore errors when the user does not exist' + ) + + // Get ->addOption( 'default-value', null, InputOption::VALUE_REQUIRED, '(Only applicable on get) If no default value is set and the config does not exist, the command will exit with 1' ) + + // Set ->addOption( - 'ignore-missing-user', + 'value', + null, + InputOption::VALUE_REQUIRED, + 'The new value of the config' + ) + ->addOption( + 'update-only', null, InputOption::VALUE_NONE, - 'Use this option to ignore errors when the user does not exist' + 'Only updates the value, if it is not set before, it is not being added' ) ; } @@ -97,7 +113,14 @@ class Setting extends Base { } if ($input->getArgument('key') === '' && $input->hasParameterOption('--default-value')) { - throw new \InvalidArgumentException('The default-value option can only be used when getting a single setting.'); + throw new \InvalidArgumentException('The "default-value" option can only be used when getting a single setting.'); + } + + if ($input->hasParameterOption('--value') && $input->hasParameterOption('--default-value')) { + throw new \InvalidArgumentException('The "value" option can not be used together with "default-value".'); + } + if ($input->hasParameterOption('--update-only') && !$input->hasParameterOption('--value')) { + throw new \InvalidArgumentException('The "update-only" option can only be used together with "value".'); } } @@ -115,7 +138,15 @@ class Setting extends Base { if ($key !== '') { $value = $this->config->getUserValue($uid, $app, $key, null); - if ($value !== null) { + if ($input->hasParameterOption('--value')) { + if ($input->hasParameterOption('--update-only') && $value === null) { + $output->writeln('The setting does not exist for user "' . $uid . '".'); + return 1; + } + + $this->config->setUserValue($uid, $app, $key, $input->getOption('value')); + + } else if ($value !== null) { $output->writeln($value); } else { if ($input->hasParameterOption('--default-value')) { From db6dba96194a63eeb68c5240da0bf686f79a0c59 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 9 Jun 2016 15:14:15 +0200 Subject: [PATCH 3/9] Allow deleting a setting --- core/Command/User/Setting.php | 42 +++++++++++++++++++++++++++++++++-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index 568f1cc2d09..a5ce74b5a79 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -95,7 +95,7 @@ class Setting extends Base { 'value', null, InputOption::VALUE_REQUIRED, - 'The new value of the config' + 'The new value of the setting' ) ->addOption( 'update-only', @@ -103,6 +103,20 @@ class Setting extends Base { InputOption::VALUE_NONE, 'Only updates the value, if it is not set before, it is not being added' ) + + // Delete + ->addOption( + 'delete', + null, + InputOption::VALUE_NONE, + 'Specify this option to delete the config' + ) + ->addOption( + 'error-if-not-exists', + null, + InputOption::VALUE_NONE, + 'Checks whether the setting exists before deleting it' + ) ; } @@ -113,15 +127,31 @@ class Setting extends Base { } if ($input->getArgument('key') === '' && $input->hasParameterOption('--default-value')) { - throw new \InvalidArgumentException('The "default-value" option can only be used when getting a single setting.'); + throw new \InvalidArgumentException('The "default-value" option can only be used when specifying a key.'); } + if ($input->getArgument('key') === '' && $input->hasParameterOption('--value')) { + throw new \InvalidArgumentException('The "value" option can only be used when specifying a key.'); + } if ($input->hasParameterOption('--value') && $input->hasParameterOption('--default-value')) { throw new \InvalidArgumentException('The "value" option can not be used together with "default-value".'); } if ($input->hasParameterOption('--update-only') && !$input->hasParameterOption('--value')) { throw new \InvalidArgumentException('The "update-only" option can only be used together with "value".'); } + + if ($input->getArgument('key') === '' && $input->hasParameterOption('--delete')) { + throw new \InvalidArgumentException('The "delete" option can only be used when specifying a key.'); + } + if ($input->hasParameterOption('--delete') && $input->hasParameterOption('--default-value')) { + throw new \InvalidArgumentException('The "value" option can not be used together with "default-value".'); + } + if ($input->hasParameterOption('--delete') && $input->hasParameterOption('--value')) { + throw new \InvalidArgumentException('The "value" option can not be used together with "value".'); + } + if ($input->hasParameterOption('--error-if-not-exists') && !$input->hasParameterOption('--delete')) { + throw new \InvalidArgumentException('The "error-if-not-exists" option can only be used together with "delete".'); + } } protected function execute(InputInterface $input, OutputInterface $output) { @@ -146,6 +176,14 @@ class Setting extends Base { $this->config->setUserValue($uid, $app, $key, $input->getOption('value')); + } else if ($input->hasParameterOption('--delete')) { + if ($input->hasParameterOption('--error-if-not-exists') && $value === null) { + $output->writeln('The setting does not exist for user "' . $uid . '".'); + return 1; + } + + $this->config->deleteUserValue($uid, $app, $key); + } else if ($value !== null) { $output->writeln($value); } else { From 01899b8cf1f8b00aae72798cdb02ad71b1972288 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jun 2016 16:04:38 +0200 Subject: [PATCH 4/9] Add tests for checkInput() --- core/Command/User/Setting.php | 14 +- tests/Core/Command/User/SettingTest.php | 211 ++++++++++++++++++++++++ 2 files changed, 218 insertions(+), 7 deletions(-) create mode 100644 tests/Core/Command/User/SettingTest.php diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index a5ce74b5a79..e2feb26cfe2 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -136,20 +136,20 @@ class Setting extends Base { if ($input->hasParameterOption('--value') && $input->hasParameterOption('--default-value')) { throw new \InvalidArgumentException('The "value" option can not be used together with "default-value".'); } - if ($input->hasParameterOption('--update-only') && !$input->hasParameterOption('--value')) { + if ($input->getOption('update-only') && !$input->hasParameterOption('--value')) { throw new \InvalidArgumentException('The "update-only" option can only be used together with "value".'); } - if ($input->getArgument('key') === '' && $input->hasParameterOption('--delete')) { + if ($input->getArgument('key') === '' && $input->getOption('delete')) { throw new \InvalidArgumentException('The "delete" option can only be used when specifying a key.'); } - if ($input->hasParameterOption('--delete') && $input->hasParameterOption('--default-value')) { - throw new \InvalidArgumentException('The "value" option can not be used together with "default-value".'); + if ($input->getOption('delete') && $input->hasParameterOption('--default-value')) { + throw new \InvalidArgumentException('The "delete" option can not be used together with "default-value".'); } - if ($input->hasParameterOption('--delete') && $input->hasParameterOption('--value')) { - throw new \InvalidArgumentException('The "value" option can not be used together with "value".'); + if ($input->getOption('delete') && $input->hasParameterOption('--value')) { + throw new \InvalidArgumentException('The "delete" option can not be used together with "value".'); } - if ($input->hasParameterOption('--error-if-not-exists') && !$input->hasParameterOption('--delete')) { + if ($input->getOption('error-if-not-exists') && !$input->getOption('delete')) { throw new \InvalidArgumentException('The "error-if-not-exists" option can only be used together with "delete".'); } } diff --git a/tests/Core/Command/User/SettingTest.php b/tests/Core/Command/User/SettingTest.php new file mode 100644 index 00000000000..1ed275d6673 --- /dev/null +++ b/tests/Core/Command/User/SettingTest.php @@ -0,0 +1,211 @@ + + * + * @copyright Copyright (c) 2016, ownCloud, Inc. + * @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 + * + */ + +namespace Tests\Core\Command\User; + + +use OC\Core\Command\User\Setting; +use Test\TestCase; + +class SettingTest extends TestCase { + /** @var \OCP\IUserManager|\PHPUnit_Framework_MockObject_MockObject */ + protected $userManager; + /** @var \OCP\IConfig|\PHPUnit_Framework_MockObject_MockObject */ + protected $config; + /** @var \OCP\IDBConnection|\PHPUnit_Framework_MockObject_MockObject */ + protected $connection; + /** @var \Symfony\Component\Console\Input\InputInterface|\PHPUnit_Framework_MockObject_MockObject */ + protected $consoleInput; + /** @var \Symfony\Component\Console\Output\OutputInterface|\PHPUnit_Framework_MockObject_MockObject */ + protected $consoleOutput; + + /** @var \Symfony\Component\Console\Command\Command */ + protected $command; + + protected function setUp() { + parent::setUp(); + + $this->userManager = $this->getMockBuilder('OCP\IUserManager') + ->disableOriginalConstructor() + ->getMock(); + $this->config = $this->getMockBuilder('OCP\IConfig') + ->disableOriginalConstructor() + ->getMock(); + $this->connection = $this->getMockBuilder('OCP\IDBConnection') + ->disableOriginalConstructor() + ->getMock(); + $this->consoleInput = $this->getMockBuilder('Symfony\Component\Console\Input\InputInterface') + ->disableOriginalConstructor() + ->getMock(); + $this->consoleOutput = $this->getMockBuilder('Symfony\Component\Console\Output\OutputInterface') + ->disableOriginalConstructor() + ->getMock(); + + $this->command = new Setting($this->userManager, $this->config, $this->connection); + } + + public function dataCheckInput() { + return [ + [ + [['uid', 'username']], + [['ignore-missing-user', true]], + [], + false, + false, + ], + [ + [['uid', 'username']], + [['ignore-missing-user', false]], + [], + null, + 'The user "username" does not exists.', + ], + + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true]], + [['--default-value', true]], + false, + false, + ], + [ + [['uid', 'username'], ['key', '']], + [['ignore-missing-user', true]], + [['--default-value', true]], + false, + 'The "default-value" option can only be used when specifying a key.', + ], + + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true]], + [['--value', true]], + false, + false, + ], + [ + [['uid', 'username'], ['key', '']], + [['ignore-missing-user', true]], + [['--value', true]], + false, + 'The "value" option can only be used when specifying a key.', + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true]], + [['--value', true], ['--default-value', true]], + false, + 'The "value" option can not be used together with "default-value".', + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['update-only', true]], + [['--value', true]], + false, + false, + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['update-only', true]], + [['--value', false]], + false, + 'The "update-only" option can only be used together with "value".', + ], + + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['delete', true]], + [], + false, + false, + ], + [ + [['uid', 'username'], ['key', '']], + [['ignore-missing-user', true], ['delete', true]], + [], + false, + 'The "delete" option can only be used when specifying a key.', + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['delete', true]], + [['--default-value', true]], + false, + 'The "delete" option can not be used together with "default-value".', + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['delete', true]], + [['--value', true]], + false, + 'The "delete" option can not be used together with "value".', + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['delete', true], ['error-if-not-exists', true]], + [], + false, + false, + ], + [ + [['uid', 'username'], ['key', 'configkey']], + [['ignore-missing-user', true], ['delete', false], ['error-if-not-exists', true]], + [], + false, + 'The "error-if-not-exists" option can only be used together with "delete".', + ], + ]; + } + + /** + * @dataProvider dataCheckInput + * + * @param $arguments + * @param $options + * @param $parameterOptions + * @param $user + * @param $expectedException + */ + public function testCheckInput($arguments, $options, $parameterOptions, $user, $expectedException) { + $this->consoleInput->expects($this->any()) + ->method('getArgument') + ->willReturnMap($arguments); + $this->consoleInput->expects($this->any()) + ->method('getOption') + ->willReturnMap($options); + $this->consoleInput->expects($this->any()) + ->method('hasParameterOption') + ->willReturnMap($parameterOptions); + + if ($user !== false) { + $this->userManager->expects($this->once()) + ->method('userExists') + ->willReturn($user); + } + + try { + $this->invokePrivate($this->command, 'checkInput', [$this->consoleInput]); + $this->assertFalse($expectedException); + } catch (\InvalidArgumentException $e) { + $this->assertEquals($expectedException, $e->getMessage()); + } + } +} From f574a9d44f28b9d2279adbc63ae4a3a98ec19b10 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Mon, 13 Jun 2016 16:16:16 +0200 Subject: [PATCH 5/9] Make sure the exception is catched --- tests/Core/Command/User/SettingTest.php | 36 +++++++++++++++++++++---- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/tests/Core/Command/User/SettingTest.php b/tests/Core/Command/User/SettingTest.php index 1ed275d6673..a40aeb748e8 100644 --- a/tests/Core/Command/User/SettingTest.php +++ b/tests/Core/Command/User/SettingTest.php @@ -37,9 +37,6 @@ class SettingTest extends TestCase { /** @var \Symfony\Component\Console\Output\OutputInterface|\PHPUnit_Framework_MockObject_MockObject */ protected $consoleOutput; - /** @var \Symfony\Component\Console\Command\Command */ - protected $command; - protected function setUp() { parent::setUp(); @@ -58,8 +55,23 @@ class SettingTest extends TestCase { $this->consoleOutput = $this->getMockBuilder('Symfony\Component\Console\Output\OutputInterface') ->disableOriginalConstructor() ->getMock(); + } + + public function getCommand(array $methods = []) { + if (empty($methods)) { + return new Setting($this->userManager, $this->config, $this->connection); + } else { + $mock = $this->getMockBuilder('OC\Core\Command\User\Setting') + ->setConstructorArgs([ + $this->userManager, + $this->config, + $this->connection, + ]) + ->setMethods($methods) + ->getMock(); + return $mock; + } - $this->command = new Setting($this->userManager, $this->config, $this->connection); } public function dataCheckInput() { @@ -201,11 +213,25 @@ class SettingTest extends TestCase { ->willReturn($user); } + $command = $this->getCommand(); try { - $this->invokePrivate($this->command, 'checkInput', [$this->consoleInput]); + $this->invokePrivate($command, 'checkInput', [$this->consoleInput]); $this->assertFalse($expectedException); } catch (\InvalidArgumentException $e) { $this->assertEquals($expectedException, $e->getMessage()); } } + + public function testCheckInputExceptionCatch() { + $command = $this->getCommand(['checkInput']); + $command->expects($this->once()) + ->method('checkInput') + ->willThrowException(new \InvalidArgumentException('test')); + + $this->consoleOutput->expects($this->once()) + ->method('writeln') + ->with('test'); + + $this->assertEquals(1, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput])); + } } From dcacdde1ea24b4e4328d313d43998d4e4515c368 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 14 Jun 2016 11:31:32 +0200 Subject: [PATCH 6/9] Add tests for set/get/delete/list --- core/Command/User/Setting.php | 16 +- tests/Core/Command/User/SettingTest.php | 239 +++++++++++++++++++++++- 2 files changed, 241 insertions(+), 14 deletions(-) diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index e2feb26cfe2..b9f7566f576 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -175,6 +175,7 @@ class Setting extends Base { } $this->config->setUserValue($uid, $app, $key, $input->getOption('value')); + return 0; } else if ($input->hasParameterOption('--delete')) { if ($input->hasParameterOption('--error-if-not-exists') && $value === null) { @@ -183,28 +184,25 @@ class Setting extends Base { } $this->config->deleteUserValue($uid, $app, $key); + return 0; } else if ($value !== null) { $output->writeln($value); + return 0; } else { if ($input->hasParameterOption('--default-value')) { $output->writeln($input->getOption('default-value')); + return 0; } else { $output->writeln('The setting does not exist for user "' . $uid . '".'); return 1; } } } else { - $this->listUserSettings($input, $output, $uid, $app); + $settings = $this->getUserSettings($uid, $app); + $this->writeArrayInOutputFormat($input, $output, $settings); + return 0; } - - return 0; - } - - protected function listUserSettings(InputInterface $input, OutputInterface $output, $uid, $app) { - $settings = $this->getUserSettings($uid, $app); - - $this->writeArrayInOutputFormat($input, $output, $settings); } protected function getUserSettings($uid, $app) { diff --git a/tests/Core/Command/User/SettingTest.php b/tests/Core/Command/User/SettingTest.php index a40aeb748e8..07ffbe30366 100644 --- a/tests/Core/Command/User/SettingTest.php +++ b/tests/Core/Command/User/SettingTest.php @@ -190,11 +190,11 @@ class SettingTest extends TestCase { /** * @dataProvider dataCheckInput * - * @param $arguments - * @param $options - * @param $parameterOptions - * @param $user - * @param $expectedException + * @param array $arguments + * @param array $options + * @param array $parameterOptions + * @param mixed $user + * @param string $expectedException */ public function testCheckInput($arguments, $options, $parameterOptions, $user, $expectedException) { $this->consoleInput->expects($this->any()) @@ -234,4 +234,233 @@ class SettingTest extends TestCase { $this->assertEquals(1, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput])); } + + public function dataExecuteDelete() { + return [ + ['config', false, null, 0], + ['config', true, null, 0], + [null, false, null, 0], + [null, true, 'The setting does not exist for user "username".', 1], + ]; + } + + /** + * @dataProvider dataExecuteDelete + * + * @param string|null $value + * @param bool $errorIfNotExists + * @param string $expectedLine + * @param int $expectedReturn + */ + public function testExecuteDelete($value, $errorIfNotExists, $expectedLine, $expectedReturn) { + $command = $this->getCommand([ + 'writeArrayInOutputFormat', + 'checkInput', + 'getUserSettings', + ]); + + $this->consoleInput->expects($this->any()) + ->method('getArgument') + ->willReturnMap([ + ['uid', 'username'], + ['app', 'appname'], + ['key', 'configkey'], + ]); + + $command->expects($this->once()) + ->method('checkInput'); + + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('username', 'appname', 'configkey', null) + ->willReturn($value); + + $this->consoleInput->expects($this->atLeastOnce()) + ->method('hasParameterOption') + ->willReturnMap([ + ['--delete', true], + ['--error-if-not-exists', $errorIfNotExists], + ]); + + if ($expectedLine === null) { + $this->consoleOutput->expects($this->never()) + ->method('writeln'); + $this->config->expects($this->once()) + ->method('deleteUserValue') + ->with('username', 'appname', 'configkey'); + + } else { + $this->consoleOutput->expects($this->once()) + ->method('writeln') + ->with($expectedLine); + $this->config->expects($this->never()) + ->method('deleteUserValue'); + } + + $this->assertEquals($expectedReturn, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput])); + } + + public function dataExecuteSet() { + return [ + ['config', false, null, 0], + ['config', true, null, 0], + [null, false, null, 0], + [null, true, 'The setting does not exist for user "username".', 1], + ]; + } + + /** + * @dataProvider dataExecuteSet + * + * @param string|null $value + * @param bool $updateOnly + * @param string $expectedLine + * @param int $expectedReturn + */ + public function testExecuteSet($value, $updateOnly, $expectedLine, $expectedReturn) { + $command = $this->getCommand([ + 'writeArrayInOutputFormat', + 'checkInput', + 'getUserSettings', + ]); + + $this->consoleInput->expects($this->any()) + ->method('getArgument') + ->willReturnMap([ + ['uid', 'username'], + ['app', 'appname'], + ['key', 'configkey'], + ]); + + $command->expects($this->once()) + ->method('checkInput'); + + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('username', 'appname', 'configkey', null) + ->willReturn($value); + + $this->consoleInput->expects($this->atLeastOnce()) + ->method('hasParameterOption') + ->willReturnMap([ + ['--value', true], + ['--update-only', $updateOnly], + ]); + + if ($expectedLine === null) { + $this->consoleOutput->expects($this->never()) + ->method('writeln'); + + $this->consoleInput->expects($this->once()) + ->method('getOption') + ->with('value') + ->willReturn('setValue'); + + $this->config->expects($this->once()) + ->method('setUserValue') + ->with('username', 'appname', 'configkey', 'setValue'); + } else { + $this->consoleOutput->expects($this->once()) + ->method('writeln') + ->with($expectedLine); + $this->config->expects($this->never()) + ->method('setUserValue'); + } + + $this->assertEquals($expectedReturn, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput])); + } + + public function dataExecuteGet() { + return [ + ['config', null, 'config', 0], + [null, 'config', 'config', 0], + [null, null, 'The setting does not exist for user "username".', 1], + ]; + } + + /** + * @dataProvider dataExecuteGet + * + * @param string|null $value + * @param string|null $defaultValue + * @param string $expectedLine + * @param int $expectedReturn + */ + public function testExecuteGet($value, $defaultValue, $expectedLine, $expectedReturn) { + $command = $this->getCommand([ + 'writeArrayInOutputFormat', + 'checkInput', + 'getUserSettings', + ]); + + $this->consoleInput->expects($this->any()) + ->method('getArgument') + ->willReturnMap([ + ['uid', 'username'], + ['app', 'appname'], + ['key', 'configkey'], + ]); + + $command->expects($this->once()) + ->method('checkInput'); + + $this->config->expects($this->once()) + ->method('getUserValue') + ->with('username', 'appname', 'configkey', null) + ->willReturn($value); + + if ($value === null) { + if ($defaultValue === null) { + $this->consoleInput->expects($this->atLeastOnce()) + ->method('hasParameterOption') + ->willReturnMap([ + ['--default-value', false], + ]); + } else { + $this->consoleInput->expects($this->atLeastOnce()) + ->method('hasParameterOption') + ->willReturnMap([ + ['--default-value', true], + ]); + $this->consoleInput->expects($this->once()) + ->method('getOption') + ->with('default-value') + ->willReturn($defaultValue); + } + } + + $this->consoleOutput->expects($this->once()) + ->method('writeln') + ->with($expectedLine); + + $this->assertEquals($expectedReturn, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput])); + } + + public function testExecuteList() { + $command = $this->getCommand([ + 'writeArrayInOutputFormat', + 'checkInput', + 'getUserSettings', + ]); + + $this->consoleInput->expects($this->any()) + ->method('getArgument') + ->willReturnMap([ + ['uid', 'username'], + ['app', 'appname'], + ['key', ''], + ]); + + $command->expects($this->once()) + ->method('checkInput'); + $command->expects($this->once()) + ->method('getUserSettings') + ->willReturn(['settings']); + $command->expects($this->once()) + ->method('writeArrayInOutputFormat') + ->with($this->consoleInput, $this->consoleOutput, ['settings']); + + + $this->assertEquals(0, $this->invokePrivate($command, 'execute', [$this->consoleInput, $this->consoleOutput])); + } } From 9ed62c3b81cb684376ed86a7940ca4469d4f76b2 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Tue, 14 Jun 2016 15:22:13 +0200 Subject: [PATCH 7/9] Fix descriptions --- core/Command/User/Setting.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index b9f7566f576..c59f079df65 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -57,7 +57,7 @@ class Setting extends Base { parent::configure(); $this ->setName('user:setting') - ->setDescription('adds a user') + ->setDescription('Read and modify user settings') ->addArgument( 'uid', InputArgument::REQUIRED, @@ -66,7 +66,7 @@ class Setting extends Base { ->addArgument( 'app', InputArgument::OPTIONAL, - 'Restrict the preferences to a given app', + 'Restrict the settings to a given app', '' ) ->addArgument( From 4656b79c8ebf94086e9287ee2580671e3fb68027 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Wed, 13 Jul 2016 18:46:27 +0200 Subject: [PATCH 8/9] FIx my email --- core/Command/User/Setting.php | 3 +-- tests/Core/Command/User/SettingTest.php | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index c59f079df65..0a1ca1990a7 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -1,7 +1,6 @@ - * @author Laurens Post + * @author Joas Schilling * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 diff --git a/tests/Core/Command/User/SettingTest.php b/tests/Core/Command/User/SettingTest.php index 07ffbe30366..b05ee42d2dd 100644 --- a/tests/Core/Command/User/SettingTest.php +++ b/tests/Core/Command/User/SettingTest.php @@ -1,6 +1,6 @@ + * @author Joas Schilling * * @copyright Copyright (c) 2016, ownCloud, Inc. * @license AGPL-3.0 From aaf2be4c3d47395155bdd7c508efc14a54dee512 Mon Sep 17 00:00:00 2001 From: Joas Schilling Date: Thu, 14 Jul 2016 15:09:47 +0200 Subject: [PATCH 9/9] Use argument instead of value --- core/Command/User/Setting.php | 24 ++++++++-------- tests/Core/Command/User/SettingTest.php | 38 ++++++++++++------------- 2 files changed, 30 insertions(+), 32 deletions(-) diff --git a/core/Command/User/Setting.php b/core/Command/User/Setting.php index 0a1ca1990a7..7a62b5529b3 100644 --- a/core/Command/User/Setting.php +++ b/core/Command/User/Setting.php @@ -90,11 +90,11 @@ class Setting extends Base { ) // Set - ->addOption( + ->addArgument( 'value', - null, - InputOption::VALUE_REQUIRED, - 'The new value of the setting' + InputArgument::OPTIONAL, + 'The new value of the setting', + null ) ->addOption( 'update-only', @@ -129,13 +129,13 @@ class Setting extends Base { throw new \InvalidArgumentException('The "default-value" option can only be used when specifying a key.'); } - if ($input->getArgument('key') === '' && $input->hasParameterOption('--value')) { - throw new \InvalidArgumentException('The "value" option can only be used when specifying a key.'); + if ($input->getArgument('key') === '' && $input->getArgument('value') !== null) { + throw new \InvalidArgumentException('The value argument can only be used when specifying a key.'); } - if ($input->hasParameterOption('--value') && $input->hasParameterOption('--default-value')) { - throw new \InvalidArgumentException('The "value" option can not be used together with "default-value".'); + if ($input->getArgument('value') !== null && $input->hasParameterOption('--default-value')) { + throw new \InvalidArgumentException('The value argument can not be used together with "default-value".'); } - if ($input->getOption('update-only') && !$input->hasParameterOption('--value')) { + if ($input->getOption('update-only') && $input->getArgument('value') === null) { throw new \InvalidArgumentException('The "update-only" option can only be used together with "value".'); } @@ -145,7 +145,7 @@ class Setting extends Base { if ($input->getOption('delete') && $input->hasParameterOption('--default-value')) { throw new \InvalidArgumentException('The "delete" option can not be used together with "default-value".'); } - if ($input->getOption('delete') && $input->hasParameterOption('--value')) { + if ($input->getOption('delete') && $input->getArgument('value') !== null) { throw new \InvalidArgumentException('The "delete" option can not be used together with "value".'); } if ($input->getOption('error-if-not-exists') && !$input->getOption('delete')) { @@ -167,13 +167,13 @@ class Setting extends Base { if ($key !== '') { $value = $this->config->getUserValue($uid, $app, $key, null); - if ($input->hasParameterOption('--value')) { + if ($input->getArgument('value') !== null) { if ($input->hasParameterOption('--update-only') && $value === null) { $output->writeln('The setting does not exist for user "' . $uid . '".'); return 1; } - $this->config->setUserValue($uid, $app, $key, $input->getOption('value')); + $this->config->setUserValue($uid, $app, $key, $input->getArgument('value')); return 0; } else if ($input->hasParameterOption('--delete')) { diff --git a/tests/Core/Command/User/SettingTest.php b/tests/Core/Command/User/SettingTest.php index b05ee42d2dd..002e8dfef2f 100644 --- a/tests/Core/Command/User/SettingTest.php +++ b/tests/Core/Command/User/SettingTest.php @@ -107,37 +107,37 @@ class SettingTest extends TestCase { ], [ - [['uid', 'username'], ['key', 'configkey']], + [['uid', 'username'], ['key', 'configkey'], ['value', '']], [['ignore-missing-user', true]], - [['--value', true]], + [], false, false, ], [ - [['uid', 'username'], ['key', '']], + [['uid', 'username'], ['key', ''], ['value', '']], [['ignore-missing-user', true]], - [['--value', true]], + [], false, - 'The "value" option can only be used when specifying a key.', + 'The value argument can only be used when specifying a key.', ], [ - [['uid', 'username'], ['key', 'configkey']], + [['uid', 'username'], ['key', 'configkey'], ['value', '']], [['ignore-missing-user', true]], - [['--value', true], ['--default-value', true]], + [['--default-value', true]], false, - 'The "value" option can not be used together with "default-value".', + 'The value argument can not be used together with "default-value".', ], [ - [['uid', 'username'], ['key', 'configkey']], + [['uid', 'username'], ['key', 'configkey'], ['value', '']], [['ignore-missing-user', true], ['update-only', true]], - [['--value', true]], + [], false, false, ], [ - [['uid', 'username'], ['key', 'configkey']], + [['uid', 'username'], ['key', 'configkey'], ['value', null]], [['ignore-missing-user', true], ['update-only', true]], - [['--value', false]], + [], false, 'The "update-only" option can only be used together with "value".', ], @@ -164,9 +164,9 @@ class SettingTest extends TestCase { 'The "delete" option can not be used together with "default-value".', ], [ - [['uid', 'username'], ['key', 'configkey']], + [['uid', 'username'], ['key', 'configkey'], ['value', '']], [['ignore-missing-user', true], ['delete', true]], - [['--value', true]], + [], false, 'The "delete" option can not be used together with "value".', ], @@ -324,12 +324,13 @@ class SettingTest extends TestCase { 'getUserSettings', ]); - $this->consoleInput->expects($this->any()) + $this->consoleInput->expects($this->atLeast(4)) ->method('getArgument') ->willReturnMap([ ['uid', 'username'], ['app', 'appname'], ['key', 'configkey'], + ['value', 'setValue'], ]); $command->expects($this->once()) @@ -343,7 +344,6 @@ class SettingTest extends TestCase { $this->consoleInput->expects($this->atLeastOnce()) ->method('hasParameterOption') ->willReturnMap([ - ['--value', true], ['--update-only', $updateOnly], ]); @@ -351,10 +351,8 @@ class SettingTest extends TestCase { $this->consoleOutput->expects($this->never()) ->method('writeln'); - $this->consoleInput->expects($this->once()) - ->method('getOption') - ->with('value') - ->willReturn('setValue'); + $this->consoleInput->expects($this->never()) + ->method('getOption'); $this->config->expects($this->once()) ->method('setUserValue')