From 1683039bad5ecd1a86115960e084be429574cd79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juan=20Pablo=20Villaf=C3=A1=C3=B1ez?= Date: Tue, 4 Apr 2023 13:29:42 +0200 Subject: [PATCH] Add unit test and adjust code --- lib/Command/FillSecondary.php | 31 ++--- tests/unit/Command/FillSecondaryTest.php | 166 +++++++++++++++++++++++ 2 files changed, 181 insertions(+), 16 deletions(-) create mode 100644 tests/unit/Command/FillSecondaryTest.php diff --git a/lib/Command/FillSecondary.php b/lib/Command/FillSecondary.php index 9da766ff..b22c034f 100644 --- a/lib/Command/FillSecondary.php +++ b/lib/Command/FillSecondary.php @@ -34,6 +34,8 @@ use Symfony\Component\Console\Output\OutputInterface; use Symfony\Component\Console\Question\ChoiceQuestion; use Symfony\Component\Console\Helper\ProgressBar; +use Symfony\Component\Console\Helper\HelperSet; +use Symfony\Component\Console\Helper\QuestionHelper; /** * Class Rebuild @@ -108,7 +110,7 @@ protected function configure() { 'force', 'f', InputOption::VALUE_NONE, - 'Use this option to rebuild the search index without further questions.' + 'Use this option to fill the secondary index without further questions.' ) ->addOption( 'startOver', @@ -123,6 +125,8 @@ protected function configure() { 'The savepoint will be updated after processing this number of files.', '100' ); + + $this->setHelperSet(new HelperSet(['question' => new QuestionHelper()])); // it seems needed for unit tests } /** @@ -148,17 +152,23 @@ public function execute(InputInterface $input, OutputInterface $output) { 'chunkSize' => $chunkSize, ]; + $shouldAbort = true; $this->searchElasticService->partialSetup(); foreach ($users as $user) { $userObject = $this->userManager->get($user); if ($userObject !== null) { - if ($this->shouldAbort($input, $output)) { + if ($shouldAbort && ($shouldAbort = $this->shouldAbort($input, $output))) { + // the $this->shouldAbort method should be executed only once. + // If it returns true, the command is aborted (returning -1). + // If it returns false, the $shouldAbort var will be false, so the + // next condition won't be evaluated the next time. $output->writeln('Aborting.'); return -1; } $this->fillSecondary($connectorName, $userObject, $quiet, $output, $params); } else { $output->writeln("Unknown user $user"); + return -1; } } return 0; @@ -172,22 +182,10 @@ public function execute(InputInterface $input, OutputInterface $output) { * @return bool, returns true when the command has to be aborted, else false */ private function shouldAbort(InputInterface $input, OutputInterface $output) { - /** - * We are using static variable here because this method is private and - * the question should be asked once for the user, instead of asking for - * each user. So at any point we need to maintain a state to know if the - * question was asked or not. - */ - static $result = null; - - if (isset($result)) { - return $result; - } - if (!$input->getOption('force')) { $helper = $this->getHelper('question'); $question = new ChoiceQuestion( - "This will delete all search index data for selected users! Do you want to proceed?", + "This will re-index data for selected users based on already-indexed data! Do you want to proceed?", ['no', 'yes'], 'no' ); @@ -213,7 +211,7 @@ private function shouldAbort(InputInterface $input, OutputInterface $output) { protected function fillSecondary($connectorName, $user, $quiet, $output, $params) { $uid = $user->getUID(); if (!$quiet) { - $output->writeln("Rebuilding Search Index for $uid"); + $output->writeln("Filling secondary index for $uid"); } $home = $this->rootFolder->getUserFolder($uid); @@ -231,5 +229,6 @@ protected function fillSecondary($connectorName, $user, $quiet, $output, $params }; $this->searchElasticService->fillSecondaryIndex($uid, $home, $connectorName, $fillParams); $progressBar->finish(); + $output->writeln(''); } } diff --git a/tests/unit/Command/FillSecondaryTest.php b/tests/unit/Command/FillSecondaryTest.php new file mode 100644 index 00000000..0187f63c --- /dev/null +++ b/tests/unit/Command/FillSecondaryTest.php @@ -0,0 +1,166 @@ +searchElasticService = $this->createMock(SearchElasticService::class); + $this->userManager = $this->createMock(IUserManager::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + + $command = new FillSecondary($this->searchElasticService, $this->userManager, $this->rootFolder); + $this->commandTester = new CommandTester($command); + } + + public function testNoUserId() { + $this->expectException(\RuntimeException::class); + + $this->commandTester->execute([]); + } + + public function testConnectorNoUser() { + $this->expectException(\RuntimeException::class); + + $this->commandTester->execute(['connector_name' => 'con001']); + } + + public function testMissingUser() { + $this->userManager->method('get')->willReturn(null); + + $this->searchElasticService->expects($this->once()) + ->method('partialSetup'); + + $this->commandTester->execute([ + 'connector_name' => 'con001', + 'user_id' => ['user001'], + ]); + $this->assertSame(-1, $this->commandTester->getStatusCode()); + } + + public function testShouldAbort() { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user001'); + + $this->userManager->expects($this->once()) + ->method('get') + ->with('user001') + ->willReturn($user); + + $this->searchElasticService->expects($this->once()) + ->method('partialSetup'); + + $this->commandTester->setInputs(['no']); + $this->commandTester->execute([ + 'connector_name' => 'con001', + 'user_id' => ['user001'], + ]); + $this->assertSame(-1, $this->commandTester->getStatusCode()); + } + + public function testShouldAbortMultipleUsers() { + $user1 = $this->createMock(IUser::class); + $user1->method('getUID')->willReturn('user001'); + $user2 = $this->createMock(IUser::class); + $user2->method('getUID')->willReturn('user002'); + $user3 = $this->createMock(IUser::class); + $user3->method('getUID')->willReturn('user003'); + + $this->userManager->expects($this->once()) + ->method('get') + ->willReturnMap([ + ['user001', false, $user1], + ['user002', false, $user2], + ['user003', false, $user3], + ]); + + $this->searchElasticService->expects($this->once()) + ->method('partialSetup'); + + $this->commandTester->setInputs(['no']); + $this->commandTester->execute([ + 'connector_name' => 'con001', + 'user_id' => ['user001'], + ]); + $this->assertSame(-1, $this->commandTester->getStatusCode()); + } + + public function testExecute() { + $user = $this->createMock(IUser::class); + $user->method('getUID')->willReturn('user001'); + + $this->userManager->expects($this->once()) + ->method('get') + ->with('user001') + ->willReturn($user); + + $this->searchElasticService->expects($this->once()) + ->method('partialSetup'); + + $folder = $this->createMock(Folder::class); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user001') + ->willReturn($folder); + + $this->searchElasticService->expects($this->once()) + ->method('getCountFillSecondaryIndex') + ->with('user001', $folder, 'con001', ['startOver' => false]) + ->willReturn(987); + $this->searchElasticService->expects($this->once()) + ->method('fillSecondaryIndex') + ->with('user001', $folder, 'con001', $this->anything()); + + //$this->commandTester->setInputs(['yes']); + $this->commandTester->execute([ + 'connector_name' => 'con001', + 'user_id' => ['user001'], + '--force' => true, + ]); + $this->assertSame(0, $this->commandTester->getStatusCode()); + } +}