From 6bc3d3781d9003b9bcdaa47bbe00674fbf272c29 Mon Sep 17 00:00:00 2001 From: Liam Dennehy Date: Thu, 28 Jun 2018 17:14:45 +0200 Subject: [PATCH 1/3] Default behaviour when no users are specified on trashbin:cleanup * Add option --all-users to explicitly clean all trashbins * Reject no users on commandline and no --all-users * Warn when --all-users and userids are specified Signed-off-by: Liam Dennehy --- apps/files_trashbin/lib/Command/CleanUp.php | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/files_trashbin/lib/Command/CleanUp.php b/apps/files_trashbin/lib/Command/CleanUp.php index b727dd9e133..d1931b6b3e1 100644 --- a/apps/files_trashbin/lib/Command/CleanUp.php +++ b/apps/files_trashbin/lib/Command/CleanUp.php @@ -29,6 +29,7 @@ use OCP\IUserBackend; use OCP\IUserManager; use Symfony\Component\Console\Command\Command; use Symfony\Component\Console\Input\InputArgument; +use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; @@ -62,13 +63,22 @@ class CleanUp extends Command { ->addArgument( 'user_id', InputArgument::OPTIONAL | InputArgument::IS_ARRAY, - 'remove deleted files of the given user(s), if no user is given all deleted files will be removed' + 'remove deleted files of the given user(s)' + ) + ->addOption( + 'all-users', + null, + InputOption::VALUE_NONE, + 'run action on all users' ); } protected function execute(InputInterface $input, OutputInterface $output) { $users = $input->getArgument('user_id'); if (!empty($users)) { + if ($input->getOption('all-users')) { + $output->writeln('Option --all-users supplied along with users, restricting to supplied users'); + } foreach ($users as $user) { if ($this->userManager->userExists($user)) { $output->writeln("Remove deleted files of $user"); @@ -77,8 +87,8 @@ class CleanUp extends Command { $output->writeln("Unknown user $user"); } } - } else { - $output->writeln('Remove all deleted files'); + } elseif ($input->getOption('all-users')) { + $output->writeln('Remove deleted files for all users'); foreach ($this->userManager->getBackends() as $backend) { $name = get_class($backend); if ($backend instanceof IUserBackend) { From 68e41a3aae803e46be433b372deab16e9b155279 Mon Sep 17 00:00:00 2001 From: Liam Dennehy Date: Thu, 28 Jun 2018 22:31:27 +0200 Subject: [PATCH 2/3] trashbin:cleanup exceptions for invalid options * throw on no parameters provided * throw on --all-users and userid provided Signed-off-by: Liam Dennehy --- apps/files_trashbin/lib/Command/CleanUp.php | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/apps/files_trashbin/lib/Command/CleanUp.php b/apps/files_trashbin/lib/Command/CleanUp.php index d1931b6b3e1..d2f8f72be8b 100644 --- a/apps/files_trashbin/lib/Command/CleanUp.php +++ b/apps/files_trashbin/lib/Command/CleanUp.php @@ -32,6 +32,7 @@ use Symfony\Component\Console\Input\InputArgument; use Symfony\Component\Console\Input\InputOption; use Symfony\Component\Console\Input\InputInterface; use Symfony\Component\Console\Output\OutputInterface; +use Symfony\Component\Console\Exception\InvalidOptionException; class CleanUp extends Command { @@ -75,10 +76,9 @@ class CleanUp extends Command { protected function execute(InputInterface $input, OutputInterface $output) { $users = $input->getArgument('user_id'); - if (!empty($users)) { - if ($input->getOption('all-users')) { - $output->writeln('Option --all-users supplied along with users, restricting to supplied users'); - } + if ((!empty($users)) and ($input->getOption('all-users'))) { + throw new InvalidOptionException('Either specify a user_id or --all-users'); + } elseif (!empty($users)) { foreach ($users as $user) { if ($this->userManager->userExists($user)) { $output->writeln("Remove deleted files of $user"); @@ -106,6 +106,8 @@ class CleanUp extends Command { $offset += $limit; } while (count($users) >= $limit); } + } else { + throw new InvalidOptionException('Either specify a user_id or --all-users'); } } From 68f403cea205e41c02b9d072cf7398dca768c615 Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Fri, 29 Jun 2018 20:48:09 +0200 Subject: [PATCH 3/3] Fix and extend tests Signed-off-by: Roeland Jago Douma --- .../tests/Command/CleanUpTest.php | 47 ++++++++++++++++--- 1 file changed, 41 insertions(+), 6 deletions(-) diff --git a/apps/files_trashbin/tests/Command/CleanUpTest.php b/apps/files_trashbin/tests/Command/CleanUpTest.php index 36b1ff10727..1ea3fc19c05 100644 --- a/apps/files_trashbin/tests/Command/CleanUpTest.php +++ b/apps/files_trashbin/tests/Command/CleanUpTest.php @@ -28,6 +28,9 @@ namespace OCA\Files_Trashbin\Tests\Command; use OCA\Files_Trashbin\Command\CleanUp; +use Symfony\Component\Console\Exception\InvalidOptionException; +use Symfony\Component\Console\Input\InputInterface; +use Symfony\Component\Console\Output\OutputInterface; use Test\TestCase; use OC\User\Manager; use OCP\Files\IRootFolder; @@ -183,8 +186,7 @@ class CleanUpTest extends TestCase { ->setMethods(['removeDeletedFiles']) ->setConstructorArgs([$this->rootFolder, $this->userManager, $this->dbConnection]) ->getMock(); - $backend = $this->getMockBuilder(\OCP\UserInterface::class) - ->disableOriginalConstructor()->getMock(); + $backend = $this->createMock(\OCP\UserInterface::class); $backend->expects($this->once())->method('getUsers') ->with('', 500, 0) ->willReturn($backendUsers); @@ -193,17 +195,50 @@ class CleanUpTest extends TestCase { ->willReturnCallback(function ($user) use ($backendUsers) { $this->assertTrue(in_array($user, $backendUsers)); }); - $inputInterface = $this->getMockBuilder('\Symfony\Component\Console\Input\InputInterface') - ->disableOriginalConstructor()->getMock(); + $inputInterface = $this->createMock(InputInterface::class); $inputInterface->expects($this->once())->method('getArgument') ->with('user_id') ->willReturn($userIds); - $outputInterface = $this->getMockBuilder('\Symfony\Component\Console\Output\OutputInterface') - ->disableOriginalConstructor()->getMock(); + $inputInterface->method('getOption') + ->with('all-users') + ->willReturn(true); + $outputInterface = $this->createMock(OutputInterface::class); $this->userManager->expects($this->once()) ->method('getBackends') ->willReturn([$backend]); $this->invokePrivate($instance, 'execute', [$inputInterface, $outputInterface]); } + public function testExecuteNoUsersAndNoAllUsers() { + $inputInterface = $this->createMock(InputInterface::class); + $inputInterface->expects($this->once())->method('getArgument') + ->with('user_id') + ->willReturn([]); + $inputInterface->method('getOption') + ->with('all-users') + ->willReturn(false); + $outputInterface = $this->createMock(OutputInterface::class); + + $this->expectException(InvalidOptionException::class); + $this->expectExceptionMessage('Either specify a user_id or --all-users'); + + $this->invokePrivate($this->cleanup, 'execute', [$inputInterface, $outputInterface]); + } + + public function testExecuteUsersAndAllUsers() { + $inputInterface = $this->createMock(InputInterface::class); + $inputInterface->expects($this->once())->method('getArgument') + ->with('user_id') + ->willReturn(['user1', 'user2']); + $inputInterface->method('getOption') + ->with('all-users') + ->willReturn(true); + $outputInterface = $this->createMock(OutputInterface::class); + + $this->expectException(InvalidOptionException::class); + $this->expectExceptionMessage('Either specify a user_id or --all-users'); + + $this->invokePrivate($this->cleanup, 'execute', [$inputInterface, $outputInterface]); + } + }