Merge pull request #18 from DanielSiepmann/feature/firstReview

Feature/first review

Classes/Command/IndexCommandController.php

@@ -50,6 +50,6 @@ class IndexCommandController extends CommandController
     {
         // TODO: Allow to index multiple tables at once?
         // TODO: Also allow to index everything?
-        $this->indexerFactory->getIndexer($table)->index();
+        $this->indexerFactory->getIndexer($table)->indexAllDocuments();
     }
 }

Classes/Connection/ConnectionInterface.php

@@ -33,7 +33,7 @@ interface ConnectionInterface
      *
      * @return void
      */
-    public function add($documentType, array $document);
+    public function addDocument($documentType, array $document);
 
     /**
      * Add the given documents.
@@ -48,22 +48,26 @@ interface ConnectionInterface
     /**
      * Will update an existing document.
      *
+     * NOTE: Batch updating is not yet supported.
+     *
      * @param string $documentType
      * @param array $document
      *
      * @return void
      */
-    public function update($documentType, array $document);
+    public function updateDocument($documentType, array $document);
 
     /**
      * Will remove an existing document.
      *
+     * NOTE: Batch deleting is not yet supported.
+     *
      * @param string $documentType
      * @param int $identifier
      *
      * @return void
      */
-    public function delete($documentType, $identifier);
+    public function deleteDocument($documentType, $identifier);
 
     /**
      * Search by given request and return result.

Classes/Connection/Elasticsearch.php

@@ -80,7 +80,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
         $this->documentFactory = $documentFactory;
     }
 
-    public function add($documentType, array $document)
+    public function addDocument($documentType, array $document)
     {
         $this->withType(
             $documentType,
@@ -90,7 +90,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
         );
     }
 
-    public function delete($documentType, $identifier)
+    public function deleteDocument($documentType, $identifier)
     {
         $this->withType(
             $documentType,
@@ -100,7 +100,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
         );
     }
 
-    public function update($documentType, array $document)
+    public function updateDocument($documentType, array $document)
     {
         $this->withType(
             $documentType,
@@ -110,12 +110,6 @@ class Elasticsearch implements Singleton, ConnectionInterface
         );
     }
 
-    /**
-     * Add the given documents to elasticsearch.
-     *
-     * @param string $documentType
-     * @param array $documents
-     */
     public function addDocuments($documentType, array $documents)
     {
         $this->withType(
@@ -141,6 +135,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
 
     /**
      * @param SearchRequestInterface $searchRequest
+     *
      * @return \Elastica\ResultSet
      */
     public function search(SearchRequestInterface $searchRequest)
@@ -152,7 +147,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
 
         // TODO: Return wrapped result to implement our interface.
         // Also update php doc to reflect the change.
-        return $search->search($searchRequest->getSearchTerm());
+        return $search->search('"' . $searchRequest->getSearchTerm() . '"');
     }
 
     /**

Classes/Connection/Elasticsearch/Connection.php

@@ -27,6 +27,8 @@ use TYPO3\CMS\Core\SingletonInterface as Singleton;
  * The current connection to elasticsearch.
  *
  * Wrapper for Elastica\Client.
+ *
+ * TODO: Catch inner exception and throw general ones (at least in Connection-Namespace (not elastic specific))
  */
 class Connection implements Singleton
 {

Classes/Domain/Index/IndexerInterface.php

@@ -26,16 +26,18 @@ namespace Leonmrni\SearchCore\Domain\Index;
 interface IndexerInterface
 {
     /**
-     * Index the index.
+     * Fetches all documents from the indexerService and pushes it to the connection.
      *
      * @return void
      */
-    public function index();
+    public function indexAllDocuments();
 
     /**
-     * Index a single record.
+     * Fetches a single document from the indexerService and pushes it to the connection.
+     *
+     * @param string $identifier identifier, the indexer needs to identify a single document
      *
      * @return void
      */
-    public function indexRecord($identifier);
+    public function indexDocument($identifier);
 }

Classes/Domain/Index/TcaIndexer.php

@@ -65,7 +65,7 @@ class TcaIndexer implements IndexerInterface
         $this->connection = $connection;
     }
 
-    public function index()
+    public function indexAllDocuments()
     {
         $this->logger->info('Start indexing');
         foreach ($this->getRecordGenerator() as $records) {
@@ -79,10 +79,10 @@ class TcaIndexer implements IndexerInterface
         $this->logger->info('Finish indexing');
     }
 
-    public function indexRecord($identifier)
+    public function indexDocument($identifier)
     {
         $this->logger->info('Start indexing single record.', [$identifier]);
-        $this->connection->add($this->tcaTableService->getTableName(), $this->getRecord($identifier));
+        $this->connection->addDocument($this->tcaTableService->getTableName(), $this->getRecord($identifier));
         $this->logger->info('Finish indexing');
     }
 

Classes/Domain/Model/SearchRequest.php

@@ -55,6 +55,6 @@ class SearchRequest implements SearchRequestInterface
      */
     public function getSearchTerm()
     {
-        return '"' . $this->query . '"';
+        return $this->query;
     }
 }

Classes/Domain/Search/SearchService.php

@@ -35,21 +35,6 @@ class SearchService
      */
     protected $connection;
 
-    /**
-     * @var \TYPO3\CMS\Core\Log\Logger
-     */
-    protected $logger;
-
-    /**
-     * Inject log manager to get concrete logger from it.
-     *
-     * @param \TYPO3\CMS\Core\Log\LogManager $logManager
-     */
-    public function injectLogger(\TYPO3\CMS\Core\Log\LogManager $logManager)
-    {
-        $this->logger = $logManager->getLogger(__CLASS__);
-    }
-
     /**
      * @param ConnectionInterface $connection
      */

Classes/Domain/Service/DataHandler.php

@@ -27,6 +27,9 @@ use TYPO3\CMS\Core\SingletonInterface as Singleton;
  *
  * This is the place to add mappings of further parts to adjust the data before
  * sending ot to connection.
+ *
+ * TODO: Probably a candidate for deletion. Currently this class makes use of
+ * extbase DI. We have to resolve this somehow.
  */
 class DataHandler implements Singleton
 {
@@ -64,7 +67,7 @@ class DataHandler implements Singleton
     public function add($table, array $record)
     {
         $this->logger->debug('Record received for add.', [$table, $record]);
-        $this->indexerFactory->getIndexer($table)->indexRecord($record['uid']);
+        $this->indexerFactory->getIndexer($table)->indexDocument($record['uid']);
     }
 
     /**
@@ -73,7 +76,7 @@ class DataHandler implements Singleton
     public function update($table, array $record)
     {
         $this->logger->debug('Record received for update.', [$table, $record]);
-        $this->indexerFactory->getIndexer($table)->indexRecord($record['uid']);
+        $this->indexerFactory->getIndexer($table)->indexDocument($record['uid']);
     }
 
     /**
@@ -83,6 +86,6 @@ class DataHandler implements Singleton
     public function delete($table, $identifier)
     {
         $this->logger->debug('Record received for delete.', [$table, $identifier]);
-        $this->connection->delete($table, $identifier);
+        $this->connection->deleteDocument($table, $identifier);
     }
 }

Tests/Functional/AbstractFunctionalTestCase.php

@@ -26,6 +26,7 @@ use TYPO3\CMS\Core\Tests\FunctionalTestCase as CoreTestCase;
  * All functional tests should extend this base class.
  *
  * It will take care of leaving a clean environment for next test.
+ * TODO: this is in reality an "elastica" abstract case - not search_core ;)
  */
 abstract class AbstractFunctionalTestCase extends CoreTestCase
 {

Tests/Functional/FunctionalTests.xml

@@ -29,6 +29,5 @@
     <logging>
         <log type="coverage-html" target="../../.Build/report/functional/html" lowUpperBound="35" highLowerBound="70"/>
         <log type="coverage-clover" target="../../.Build/report/functional/clover/coverage"/>
-        <log type="coverage-text" target="php://stdout" showUncoveredFiles="false"/>
     </logging>
 </phpunit>

Tests/Functional/Hooks/DataHandlerTest.php

@@ -76,7 +76,7 @@ class DataHandlerTest extends AbstractFunctionalTestCase
         $hook->processDatamap_afterDatabaseOperations('update', 'tt_content', 6, [], $dataHandler);
 
         $response = $this->client->request('typo3content/_search?q=*:*');
-        $this->assertTrue($response->isOK());
+        $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.');
         $this->assertSame($response->getData()['hits']['total'], 1, 'Not exactly 1 document was indexed.');
     }
 
@@ -91,7 +91,7 @@ class DataHandlerTest extends AbstractFunctionalTestCase
         $hook->processCmdmap_deleteAction('tt_content', 6, [], false, $dataHandler);
 
         $response = $this->client->request('typo3content/_search?q=*:*');
-        $this->assertTrue($response->isOK());
+        $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.');
         $this->assertSame($response->getData()['hits']['total'], 0, 'Not exactly 0 document was indexed.');
     }
 
@@ -99,10 +99,11 @@ class DataHandlerTest extends AbstractFunctionalTestCase
      * @test
      * @expectedException \Elastica\Exception\ResponseException
      */
-    public function someUnkownOperationDoesNotBreakSomething()
+    public function someUnknownOperationDoesNotBreakSomething()
     {
         $dataHandler = new CoreDataHandler();
         $hook = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(Hook::class);
+        //TODO: this test is senseless, checking an exception not correct, this operation should not do anything!
         $hook->processDatamap_afterDatabaseOperations('something', 'tt_content', 6, [], $dataHandler);
 
         // Should trigger Exception

Tests/Functional/Indexing/IndexTcaTableTest.php

@@ -44,12 +44,12 @@ class IndexTcaTableTest extends AbstractFunctionalTestCase
         \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
             ->get(IndexerFactory::class)
             ->getIndexer('tt_content')
-            ->index()
+            ->indexAllDocuments()
             ;
 
         $response = $this->client->request('typo3content/_search?q=*:*');
 
-        $this->assertTrue($response->isOK());
+        $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.');
         $this->assertSame($response->getData()['hits']['total'], 1, 'Not exactly 1 document was indexed.');
         $this->assertArraySubset(
             ['_source' => ['header' => 'indexed content element']],
@@ -72,23 +72,24 @@ class IndexTcaTableTest extends AbstractFunctionalTestCase
     }
 
     /**
+     * TODO: this does not test the indexer, it tests the backend
      * @test
      */
-    public function canHandleExisingIndex()
+    public function canHandleExistingIndex()
     {
         $indexer = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
             ->get(IndexerFactory::class)
             ->getIndexer('tt_content')
             ;
 
-        $indexer->index();
+        $indexer->indexAllDocuments();
 
         // Index 2nd time, index already exists in elasticsearch.
-        $indexer->index();
+        $indexer->indexAllDocuments();
 
         $response = $this->client->request('typo3content/_search?q=*:*');
 
-        $this->assertTrue($response->isOK());
+        $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.');
         $this->assertSame($response->getData()['hits']['total'], 1, 'Not exactly 1 document was indexed.');
     }
 
@@ -103,12 +104,12 @@ class IndexTcaTableTest extends AbstractFunctionalTestCase
         \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
             ->get(IndexerFactory::class)
             ->getIndexer('tt_content')
-            ->index()
+            ->indexAllDocuments()
             ;
 
         $response = $this->client->request('typo3content/_search?q=*:*');
 
-        $this->assertTrue($response->isOK());
+        $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.');
         $this->assertSame($response->getData()['hits']['total'], 2, 'Not exactly 2 documents were indexed.');
         $this->assertArraySubset(
             ['_source' => ['header' => 'Also indexable record']],
@@ -134,11 +135,11 @@ class IndexTcaTableTest extends AbstractFunctionalTestCase
         \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
             ->get(IndexerFactory::class)
             ->getIndexer('tt_content')
-            ->index()
+            ->indexAllDocuments()
             ;
 
         $response = $this->client->request('typo3content/_search?q=*:*');
-        $this->assertTrue($response->isOK());
+        $this->assertTrue($response->isOK(), 'Elastica did not answer with ok code.');
         $this->assertSame($response->getData()['hits']['total'], 3, 'Not exactly 3 documents were indexed.');
 
         $response = $this->client->request('typo3content/_search?q=uid:9');

Tests/Unit/UnitTests.xml

@@ -29,6 +29,5 @@
     <logging>
         <log type="coverage-html" target="../../.Build/report/unit/html" lowUpperBound="35" highLowerBound="70"/>
         <log type="coverage-clover" target="../../.Build/report/unit/clover/coverage"/>
-        <log type="coverage-text" target="php://stdout" showUncoveredFiles="false"/>
     </logging>
 </phpunit>