TASK: Refactor type handling for elasticsearch

With V6 of elasticsearch the meaning / handling of index and type has
changed, and it will change further in the future.

We therefore move code to own classes and adjust / refactor calling
code.

Classes/Connection/Elasticsearch.php

@@ -24,6 +24,7 @@ namespace Codappix\SearchCore\Connection;
 use Codappix\SearchCore\Connection\Elasticsearch\SearchResult;
 use Codappix\SearchCore\Domain\Search\QueryFactory;
 use Elastica\Query;
+use Elastica\Type;
 use TYPO3\CMS\Core\SingletonInterface as Singleton;
 use TYPO3\CMS\Extbase\Object\ObjectManagerInterface;
 
@@ -118,7 +119,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
     {
         $this->withType(
             $documentType,
-            function ($type) use ($documentType, $document) {
+            function (Type $type, string $documentType) use ($document) {
                 $type->addDocument($this->documentFactory->getDocument($documentType, $document));
             }
         );
@@ -129,7 +130,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
         try {
             $this->withType(
                 $documentType,
-                function ($type) use ($identifier) {
+                function (Type $type, string $documentType) use ($identifier) {
                     $type->deleteById($identifier);
                 }
             );
@@ -145,7 +146,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
     {
         $this->withType(
             $documentType,
-            function ($type) use ($documentType, $document) {
+            function (Type $type, string $documentType) use ($document) {
                 $type->updateDocument($this->documentFactory->getDocument($documentType, $document));
             }
         );
@@ -155,7 +156,7 @@ class Elasticsearch implements Singleton, ConnectionInterface
     {
         $this->withType(
             $documentType,
-            function ($type) use ($documentType, $documents) {
+            function (Type $type, string $documentType) use ($documents) {
                 $type->addDocuments($this->documentFactory->getDocuments($documentType, $documents));
             }
         );
@@ -205,23 +206,6 @@ class Elasticsearch implements Singleton, ConnectionInterface
         }
     }
 
-    /**
-     * Execute given callback with Elastica Type based on provided documentType
-     */
-    protected function withType(string $documentType, callable $callback)
-    {
-        $type = $this->getType($documentType);
-        // TODO: Check whether it's to heavy to send it so often e.g. for every single document.
-        // Perhaps add command controller to submit mapping?!
-        // Also it's not possible to change mapping without deleting index first.
-        // Mattes told about a solution.
-        // So command looks like the best way so far, except we manage mattes solution.
-        // Still then this should be done once. So perhaps singleton which tracks state and does only once?
-        $this->mappingFactory->getMapping($type, $documentType)->send();
-        $callback($type);
-        $type->getIndex()->refresh();
-    }
-
     public function search(SearchRequestInterface $searchRequest): SearchResultInterface
     {
         $this->logger->debug('Search for', [$searchRequest->getSearchTerm()]);
@@ -233,14 +217,20 @@ class Elasticsearch implements Singleton, ConnectionInterface
         return $this->objectManager->get(SearchResult::class, $searchRequest, $search->search());
     }
 
-    protected function getType($documentType): \Elastica\Type
+    /**
+     * Execute given callback with Elastica Type based on provided documentType
+     */
+    private function withType(string $documentType, callable $callback)
     {
-        return $this->typeFactory->getType(
+        $type = $this->typeFactory->getType($documentType);
-            $this->indexFactory->getIndex(
+        // TODO: Check whether it's to heavy to send it so often e.g. for every single document.
-                $this->connection,
+        // Perhaps add command controller to submit mapping?!
-                $documentType
+        // Also it's not possible to change mapping without deleting index first.
-            ),
+        // Mattes told about a solution.
-            'document'
+        // So command looks like the best way so far, except we manage mattes solution.
-        );
+        // Still then this should be done once. So perhaps singleton which tracks state and does only once?
+        $this->mappingFactory->getMapping($documentType)->send();
+        $callback($type, $documentType);
+        $type->getIndex()->refresh();
     }
 }

Classes/Connection/Elasticsearch/MappingFactory.php

@@ -35,29 +35,39 @@ class MappingFactory implements Singleton
      */
     protected $configuration;
 
+    /**
+     * @var TypeFactory
+     */
+    protected $typeFactory;
+
     /**
      * @param ConfigurationContainerInterface $configuration
      */
-    public function __construct(ConfigurationContainerInterface $configuration)
+    public function __construct(
-    {
+        ConfigurationContainerInterface $configuration,
+        TypeFactory $typeFactory
+    ) {
         $this->configuration = $configuration;
+        $this->typeFactory = $typeFactory;
     }
 
     /**
      * Get an mapping based on type.
      */
-    public function getMapping(\Elastica\Type $type, string $documentType = null): \Elastica\Type\Mapping
+    public function getMapping(string $documentType): \Elastica\Type\Mapping
     {
+        $type = $this->typeFactory->getType($documentType);
+
         $mapping = new \Elastica\Type\Mapping();
         $mapping->setType($type);
 
-        $configuration = $this->getConfiguration($documentType ?? $type->getName());
+        $configuration = $this->getConfiguration($documentType);
         $mapping->setProperties($configuration);
 
         return $mapping;
     }
 
-    protected function getConfiguration(string $identifier): array
+    private function getConfiguration(string $identifier): array
     {
         try {
             return $this->configuration->get('indexing.' . $identifier . '.mapping');

Classes/Connection/Elasticsearch/TypeFactory.php

@@ -21,6 +21,7 @@ namespace Codappix\SearchCore\Connection\Elasticsearch;
  * 02110-1301, USA.
  */
 
+use Codappix\SearchCore\Connection\Elasticsearch\IndexFactory;
 use TYPO3\CMS\Core\SingletonInterface as Singleton;
 
 /**
@@ -30,11 +31,30 @@ use TYPO3\CMS\Core\SingletonInterface as Singleton;
  */
 class TypeFactory implements Singleton
 {
+    /**
+     * @var IndexFactory
+     */
+    protected $indexFactory;
+
+    /**
+     * @var Connection
+     */
+    private $connection;
+
+    public function __construct(
+        Connection $connection,
+        IndexFactory $indexFactory
+    ) {
+        $this->indexFactory = $indexFactory;
+        $this->connection = $connection;
+    }
+
     /**
      * Get an index bases on TYPO3 table name.
      */
-    public function getType(\Elastica\Index $index, string $documentType): \Elastica\Type
+    public function getType(string $documentType): \Elastica\Type
     {
-        return $index->getType($documentType);
+        $index = $this->indexFactory->getIndex($this->connection, $documentType);
+        return $index->getType('document');
     }
 }

Tests/Unit/Connection/Elasticsearch/MappingFactoryTest.php

@@ -23,7 +23,9 @@ namespace Codappix\SearchCore\Tests\Unit\Connection\Elasticsearch;
 
 use Codappix\SearchCore\Configuration\ConfigurationContainerInterface;
 use Codappix\SearchCore\Connection\Elasticsearch\MappingFactory;
+use Codappix\SearchCore\Connection\Elasticsearch\TypeFactory;
 use Codappix\SearchCore\Tests\Unit\AbstractUnitTestCase;
+use Elastica\Type;
 
 class MappingFactoryTest extends AbstractUnitTestCase
 {
@@ -32,12 +34,24 @@ class MappingFactoryTest extends AbstractUnitTestCase
      */
     protected $subject;
 
+    /**
+     * @var ConfigurationContainerInterface
+     */
+    protected $configurationMock;
+
+    /**
+     * @var TypeFactory
+     */
+    protected $typeFactoryMock;
+
     public function setUp()
     {
         parent::setUp();
 
-        $this->configuration = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock();
+        $this->configurationMock = $this->getMockBuilder(ConfigurationContainerInterface::class)->getMock();
-        $this->subject = new MappingFactory($this->configuration);
+        $this->typeFactoryMock = $this->getMockBuilder(TypeFactory::class)->disableOriginalConstructor()->getMock();
+
+        $this->subject = new MappingFactory($this->configurationMock, $this->typeFactoryMock);
     }
 
     /**
@@ -45,28 +59,27 @@ class MappingFactoryTest extends AbstractUnitTestCase
      */
     public function typoScriptConfigurationIsProvidedToIndex()
     {
-        $indexName = 'someIndex';
+        $documentType = 'someDocument';
         $configuration = [
             'channel' => [
                 'type' => 'keyword',
             ],
         ];
-        $type = $this->getMockBuilder(\Elastica\Type::class)
-            ->disableOriginalConstructor()
-            ->getMock();
-        $type->expects($this->any())
-            ->method('getName')
-            ->willReturn($indexName);
-        $this->configuration->expects($this->once())
-            ->method('get')
-            ->with('indexing.' . $indexName . '.mapping')
-            ->willReturn($configuration);
-        $mapping = $this->subject->getMapping($type)->toArray()[$indexName];
 
+        $typeMock = $this->getMockBuilder(Type::class)->disableOriginalConstructor()->getMock();
+
+        $this->typeFactoryMock->expects($this->any())
+            ->method('getType')
+            ->with($documentType)
+            ->willReturn($typeMock);
+        $this->configurationMock->expects($this->once())
+            ->method('get')
+            ->with('indexing.' . $documentType . '.mapping')
+            ->willReturn($configuration);
+
+        $mapping = $this->subject->getMapping($documentType)->toArray()[''];
         $this->assertArraySubset(
-            [
+            $configuration,
-                'channel' => $configuration['channel']
-            ],
             $mapping['properties'],
             true,
             'Configuration for properties was not set for mapping.'