Merge pull request #10041 from liamdennehy/10001/trashbin-defaults

Safe default behaviour when no users are specified on trashbin:cleanup
This commit is contained in:
Roeland Jago Douma 2018-06-29 21:39:16 +02:00 committed by GitHub
commit 9b6c0ac13c
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 57 additions and 10 deletions

View file

@ -29,8 +29,10 @@ 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;
use Symfony\Component\Console\Exception\InvalidOptionException;
class CleanUp extends Command {
@ -62,13 +64,21 @@ 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 ((!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 <info>$user</info>");
@ -77,8 +87,8 @@ class CleanUp extends Command {
$output->writeln("<error>Unknown user $user</error>");
}
}
} 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) {
@ -96,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');
}
}

View file

@ -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]);
}
}