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

@@ -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 . '.');
+            }
         }
     }
 }

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;
 }

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;
 }

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;
     }

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

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.

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.

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');
+    }
 }