From 30a34c4f153561090694ea2013f07fb11f7b25a4 Mon Sep 17 00:00:00 2001 From: Daniel Siepmann Date: Sat, 27 Oct 2018 11:23:46 +0200 Subject: [PATCH] FEATURE: Always accept comma separated list of identifiers on CLI * Streamline all commands to always accept a comma separated list of identifiers. * Adjust phpdoc to reflect this feature and provide help on CLI. * Refactor code to move recurring logic to own method. * Provide tests for new feature. * Add documentation for feature. --- Classes/Command/IndexCommandController.php | 71 +++++++------ Classes/Domain/Index/AbstractIndexer.php | 25 ++--- Classes/Domain/Index/IndexerInterface.php | 23 ++-- Classes/Domain/Index/TcaIndexer.php | 2 +- Documentation/source/changelog.rst | 2 + .../20181027-added-flush-command.rst | 6 ++ ...81027-allow-multiple-identifier-on-cli.rst | 4 + .../Command/IndexCommandControllerTest.php | 100 +++++++++++++++++- 8 files changed, 173 insertions(+), 60 deletions(-) create mode 100644 Documentation/source/changelog/20181027-added-flush-command.rst create mode 100644 Documentation/source/changelog/20181027-allow-multiple-identifier-on-cli.rst diff --git a/Classes/Command/IndexCommandController.php b/Classes/Command/IndexCommandController.php index e0929c5..9596713 100644 --- a/Classes/Command/IndexCommandController.php +++ b/Classes/Command/IndexCommandController.php @@ -22,6 +22,7 @@ namespace Codappix\SearchCore\Command; */ use Codappix\SearchCore\Domain\Index\IndexerFactory; +use Codappix\SearchCore\Domain\Index\IndexerInterface; use Codappix\SearchCore\Domain\Index\NoMatchingIndexerException; use TYPO3\CMS\Core\Utility\GeneralUtility; use TYPO3\CMS\Extbase\Mvc\Controller\CommandController; @@ -46,58 +47,60 @@ class IndexCommandController extends CommandController } /** - * Will index the given identifier. + * Will index all documents for the given identifiers. * - * @param string $identifier + * @param string $identifier Comma separated list of identifiers. * @return void */ - public function indexCommand(string $identifier) + public function indexCommand(string $identifiers) { - // Allow multiple identifiers per import task - $identifiers = GeneralUtility::trimExplode(',', $identifier, true); - foreach ($identifiers as $value) { - try { - $this->indexerFactory->getIndexer($value)->indexAllDocuments(); - $this->outputLine($value . ' was indexed.'); - } catch (NoMatchingIndexerException $e) { - $this->outputLine('No indexer found for: ' . $value); - } - } + $this->executeForIdentifier($identifiers, function (IndexerInterface $indexer) { + $indexer->indexAllDocuments(); + $this->outputLine('Documents in indice ' . $indexer->getIdentifier() . ' were indexed.'); + }); } /** - * Will delete the given identifier. + * Will delete all indexed documents for the given identifiers. * - * @param string $identifier + * @param string $identifier Comma separated list of identifiers. * @return void */ - public function deleteCommand(string $identifier) + public function deleteCommand(string $identifiers) { - // Allow multiple identifiers per import task - $identifiers = GeneralUtility::trimExplode(',', $identifier, true); - foreach ($identifiers as $value) { - try { - $this->indexerFactory->getIndexer($value)->deleteDocuments(); - $this->outputLine($value . ' was deleted.'); - } catch (NoMatchingIndexerException $e) { - $this->outputLine('No indexer found for: ' . $value); - } - } + $this->executeForIdentifier($identifiers, function (IndexerInterface $indexer) { + $indexer->deleteDocuments(); + $this->outputLine('Documents in indice ' . $indexer->getIdentifier() . ' were deleted.'); + }); } /** - * Will delete the full index. + * Will delete the full index for given identifiers. * - * @param string $identifier + * @param string $identifier Comma separated list of identifiers. * @return void */ - public function flushCommand(string $identifier = 'pages') + public function flushCommand(string $identifiers = 'pages') { - try { - $this->indexerFactory->getIndexer($identifier)->delete(); - $this->outputLine('Default configured indices were deleted via ' . $identifier . '.'); - } catch (NoMatchingIndexerException $e) { - $this->outputLine('No indexer found for: ' . $identifier); + $this->executeForIdentifier($identifiers, function (IndexerInterface $indexer) { + $indexer->delete(); + $this->outputLine('Indice ' . $indexer->getIdentifier() . ' was deleted.'); + }); + } + + /** + * Executes the given callback method for each provided identifier. + * + * An indexer is created for each identifier, which is provided as first argument to the callback. + */ + private function executeForIdentifier(string $identifiers, callable $callback) + { + foreach (GeneralUtility::trimExplode(',', $identifiers, true) as $identifier) { + try { + $callback($this->indexerFactory->getIndexer($identifier)); + } catch (NoMatchingIndexerException $e) { + $this->outputLine('No indexer found for: ' . $identifier . '.'); + } } } } diff --git a/Classes/Domain/Index/AbstractIndexer.php b/Classes/Domain/Index/AbstractIndexer.php index e29ff75..afe61e9 100644 --- a/Classes/Domain/Index/AbstractIndexer.php +++ b/Classes/Domain/Index/AbstractIndexer.php @@ -65,15 +65,6 @@ abstract class AbstractIndexer implements IndexerInterface $this->logger = $logManager->getLogger(__CLASS__); } - /** - * @param string $identifier - * @return void - */ - public function setIdentifier(string $identifier) - { - $this->identifier = $identifier; - } - /** * AbstractIndexer constructor. * @param ConnectionInterface $connection @@ -120,7 +111,7 @@ abstract class AbstractIndexer implements IndexerInterface $this->connection->addDocument($this->getDocumentName(), $record); } catch (NoRecordFoundException $e) { $this->logger->info('Could not index document. Try to delete it therefore.', [$e->getMessage()]); - $this->connection->deleteDocument($this->getDocumentName(), $this->getIdentifier($identifier)); + $this->connection->deleteDocument($this->getDocumentName(), $this->getDocumentIdentifier($identifier)); } $this->logger->info('Finish indexing'); } @@ -194,7 +185,7 @@ abstract class AbstractIndexer implements IndexerInterface $record['search_document_type'] = $this->getDocumentName(); } if (!isset($record['search_identifier']) && isset($record['uid'])) { - $record['search_identifier'] = $this->getIdentifier($record['uid']); + $record['search_identifier'] = $this->getDocumentIdentifier($record['uid']); } } @@ -237,6 +228,16 @@ abstract class AbstractIndexer implements IndexerInterface return 50; } + public function setIdentifier(string $identifier) + { + $this->identifier = $identifier; + } + + public function getIdentifier(): string + { + return $this->identifier; + } + /** * @param integer $offset * @param integer $limit @@ -259,5 +260,5 @@ abstract class AbstractIndexer implements IndexerInterface * @param string $identifier * @return string */ - abstract public function getIdentifier($identifier): string; + abstract public function getDocumentIdentifier($identifier): string; } diff --git a/Classes/Domain/Index/IndexerInterface.php b/Classes/Domain/Index/IndexerInterface.php index ace5243..3debab1 100644 --- a/Classes/Domain/Index/IndexerInterface.php +++ b/Classes/Domain/Index/IndexerInterface.php @@ -41,14 +41,6 @@ interface IndexerInterface */ public function indexDocument(string $identifier); - /** - * Receives the identifier of the indexer itself. - * - * @param string $identifier - * @return void - */ - public function setIdentifier(string $identifier); - /** * Delete the whole index. * @@ -62,4 +54,19 @@ interface IndexerInterface * @return void */ public function deleteDocuments(); + + /** + * Receives the identifier of the indexer itself. + * + * @param string $identifier + * @return void + */ + public function setIdentifier(string $identifier); + + /** + * Returnes the identifier of the indexer. + * + * @return string + */ + public function getIdentifier(): string; } diff --git a/Classes/Domain/Index/TcaIndexer.php b/Classes/Domain/Index/TcaIndexer.php index 74141d0..69fdf98 100644 --- a/Classes/Domain/Index/TcaIndexer.php +++ b/Classes/Domain/Index/TcaIndexer.php @@ -100,7 +100,7 @@ class TcaIndexer extends AbstractIndexer * @param string $identifier * @return string */ - public function getIdentifier($identifier): string + public function getDocumentIdentifier($identifier): string { return $this->getDocumentName() . '-' . $identifier; } diff --git a/Documentation/source/changelog.rst b/Documentation/source/changelog.rst index 016ef89..94ad4d9 100644 --- a/Documentation/source/changelog.rst +++ b/Documentation/source/changelog.rst @@ -5,6 +5,8 @@ Changelog :maxdepth: 1 :glob: + changelog/20181027-added-flush-command + changelog/20181027-allow-multiple-identifiers-on-cli changelog/20181027-remove-cms7-support changelog/20180518-75-make-index-name-configurable changelog/20180424-149-extract-relation-resolver-to-data-processing diff --git a/Documentation/source/changelog/20181027-added-flush-command.rst b/Documentation/source/changelog/20181027-added-flush-command.rst new file mode 100644 index 0000000..569865d --- /dev/null +++ b/Documentation/source/changelog/20181027-added-flush-command.rst @@ -0,0 +1,6 @@ +Feature "Added flush command" +============================= + +A new command to flush indices was added. In contrast to the existing delete command, +this one will delete the whole index, while the existing delete command only deletes +all documents within an index. diff --git a/Documentation/source/changelog/20181027-allow-multiple-identifier-on-cli.rst b/Documentation/source/changelog/20181027-allow-multiple-identifier-on-cli.rst new file mode 100644 index 0000000..bab4211 --- /dev/null +++ b/Documentation/source/changelog/20181027-allow-multiple-identifier-on-cli.rst @@ -0,0 +1,4 @@ +Feature "Allow multiple identifiers on cli" +=========================================== + +All CLI commands except a comma separated list of IDs now. diff --git a/Tests/Unit/Command/IndexCommandControllerTest.php b/Tests/Unit/Command/IndexCommandControllerTest.php index 48ddd34..e3c1e77 100644 --- a/Tests/Unit/Command/IndexCommandControllerTest.php +++ b/Tests/Unit/Command/IndexCommandControllerTest.php @@ -61,7 +61,7 @@ class IndexCommandControllerTest extends AbstractUnitTestCase { $this->subject->expects($this->once()) ->method('outputLine') - ->with('No indexer found for: nonAllowedTable'); + ->with('No indexer found for: nonAllowedTable.'); $this->indexerFactory->expects($this->once()) ->method('getIndexer') ->with('nonAllowedTable') @@ -78,11 +78,14 @@ class IndexCommandControllerTest extends AbstractUnitTestCase $indexerMock = $this->getMockBuilder(TcaIndexer::class) ->disableOriginalConstructor() ->getMock(); + $indexerMock->expects($this->any()) + ->method('getIdentifier') + ->willReturn('allowedTable'); $this->subject->expects($this->never()) ->method('quit'); $this->subject->expects($this->once()) ->method('outputLine') - ->with('allowedTable was indexed.'); + ->with('Documents in indice allowedTable were indexed.'); $this->indexerFactory->expects($this->once()) ->method('getIndexer') ->with('allowedTable') @@ -99,9 +102,12 @@ class IndexCommandControllerTest extends AbstractUnitTestCase $indexerMock = $this->getMockBuilder(TcaIndexer::class) ->disableOriginalConstructor() ->getMock(); + $indexerMock->expects($this->any()) + ->method('getIdentifier') + ->willReturn('allowedTable'); $this->subject->expects($this->once()) ->method('outputLine') - ->with('allowedTable was deleted.'); + ->with('Documents in indice allowedTable were deleted.'); $this->indexerFactory->expects($this->once()) ->method('getIndexer') ->with('allowedTable') @@ -120,9 +126,12 @@ class IndexCommandControllerTest extends AbstractUnitTestCase $indexerMock = $this->getMockBuilder(TcaIndexer::class) ->disableOriginalConstructor() ->getMock(); + $indexerMock->expects($this->any()) + ->method('getIdentifier') + ->willReturn('pages'); $this->subject->expects($this->once()) ->method('outputLine') - ->with('Default configured indices were deleted via pages.'); + ->with('Indice pages was deleted.'); $this->indexerFactory->expects($this->once()) ->method('getIndexer') ->with('pages') @@ -140,7 +149,7 @@ class IndexCommandControllerTest extends AbstractUnitTestCase { $this->subject->expects($this->once()) ->method('outputLine') - ->with('No indexer found for: nonAllowedTable'); + ->with('No indexer found for: nonAllowedTable.'); $this->indexerFactory->expects($this->once()) ->method('getIndexer') ->with('nonAllowedTable') @@ -148,4 +157,85 @@ class IndexCommandControllerTest extends AbstractUnitTestCase $this->subject->deleteCommand('nonAllowedTable'); } + + // As all methods use the same code base, we test the logic for multiple + // identifiers only for indexing. + + /** + * @test + */ + public function indexerExecutesForAllowedTables() + { + $indexerMock = $this->getMockBuilder(TcaIndexer::class) + ->disableOriginalConstructor() + ->getMock(); + $indexerMock->expects($this->any()) + ->method('getIdentifier') + ->will($this->onConsecutiveCalls('allowedTable', 'anotherTable')); + $this->subject->expects($this->never()) + ->method('quit'); + $this->subject->expects($this->exactly(2)) + ->method('outputLine') + ->withConsecutive( + ['Documents in indice allowedTable were indexed.'], + ['Documents in indice anotherTable were indexed.'] + ); + $this->indexerFactory->expects($this->exactly(2)) + ->method('getIndexer') + ->withConsecutive(['allowedTable'], ['anotherTable']) + ->will($this->returnValue($indexerMock)); + + $this->subject->indexCommand('allowedTable, anotherTable'); + } + + /** + * @test + */ + public function indexerSkipsEmptyIdentifier() + { + $indexerMock = $this->getMockBuilder(TcaIndexer::class) + ->disableOriginalConstructor() + ->getMock(); + $indexerMock->expects($this->any()) + ->method('getIdentifier') + ->willReturn('allowedTable'); + $this->subject->expects($this->never()) + ->method('quit'); + $this->subject->expects($this->once()) + ->method('outputLine') + ->with('Documents in indice allowedTable were indexed.'); + $this->indexerFactory->expects($this->once()) + ->method('getIndexer') + ->with('allowedTable') + ->will($this->returnValue($indexerMock)); + + $this->subject->indexCommand('allowedTable, '); + } + + /** + * @test + */ + public function indexerSkipsAndOutputsNonExistingIdentifier() + { + $indexerMock = $this->getMockBuilder(TcaIndexer::class) + ->disableOriginalConstructor() + ->getMock(); + $indexerMock->expects($this->any()) + ->method('getIdentifier') + ->willReturn('allowedTable'); + $this->subject->expects($this->never()) + ->method('quit'); + $this->subject->expects($this->exactly(2)) + ->method('outputLine') + ->withConsecutive( + ['No indexer found for: nonExisting.'], + ['Documents in indice allowedTable were indexed.'] + ); + $this->indexerFactory->expects($this->exactly(2)) + ->method('getIndexer') + ->withConsecutive(['nonExisting'], ['allowedTable']) + ->will($this->onConsecutiveCalls($this->throwException(new NoMatchingIndexerException), $this->returnValue($indexerMock))); + + $this->subject->indexCommand('nonExisting, allowedTable'); + } }