Merge pull request #124 from Codappix/feature/120-pass-facets-configuration

!!!|FEATURE: 120 Pass facet configuration to search service

Classes/Connection/Elasticsearch/Facet.php

@@ -45,25 +45,26 @@ class Facet implements FacetInterface
      */
     protected $options = [];
 
-    public function __construct($name, array $aggregation, ConfigurationContainerInterface $configuration)
+    public function __construct(string $name, array $aggregation, ConfigurationContainerInterface $configuration)
     {
         $this->name = $name;
         $this->buckets = $aggregation['buckets'];
-        $this->field = $configuration->getIfExists('searching.facets.' . $this->name . '.field') ?: '';
+
+        $config = $configuration->getIfExists('searching.facets.' . $this->name) ?: [];
+        foreach ($config as $configEntry) {
+            if (isset($configEntry['field'])) {
+                $this->field = $configEntry['field'];
+                break;
+            }
+        }
     }
 
-    /**
+    public function getName() : string
-     * @return string
-     */
-    public function getName()
     {
         return $this->name;
     }
 
-    /**
+    public function getField() : string
-     * @return string
-     */
-    public function getField()
     {
         return $this->field;
     }
@@ -73,7 +74,7 @@ class Facet implements FacetInterface
      *
      * @return array<FacetOptionInterface>
      */
-    public function getOptions()
+    public function getOptions() : array
     {
         $this->initOptions();
 

Classes/Connection/FacetRequestInterface.php

@@ -28,15 +28,11 @@ interface FacetRequestInterface
     /**
      * The identifier of the facet, used as key in arrays and to get the facet
      * from search request, etc.
-     *
-     * @return string
      */
-    public function getIdentifier();
+    public function getIdentifier() : string;
 
     /**
-     * The field to use for facet building.
+     * The config to use for facet building.
-     *
-     * @return string
      */
-    public function getField();
+    public function getConfig() : array;
 }

Classes/Domain/Model/FacetRequest.php

@@ -30,37 +30,27 @@ class FacetRequest implements FacetRequestInterface
     protected $identifier = '';
 
     /**
-     * @var string
+     * @var array
      */
-    protected $field = '';
+    protected $config = [];
 
     /**
-     * TODO: Add validation / exception?
      * As the facets come from configuration this might be a good idea to help
      * integrators find issues.
-     *
-     * @param string $identifier
-     * @param string $field
      */
-    public function __construct($identifier, $field)
+    public function __construct(string $identifier, array $config)
     {
         $this->identifier = $identifier;
-        $this->field = $field;
+        $this->config = $config;
     }
 
-    /**
+    public function getIdentifier() : string
-     * @return string
-     */
-    public function getIdentifier()
     {
         return $this->identifier;
     }
 
-    /**
+    public function getConfig() : array
-     * @return string
-     */
-    public function getField()
     {
-        return $this->field;
+        return $this->config;
     }
 }

Classes/Domain/Search/QueryFactory.php

@@ -239,11 +239,7 @@ class QueryFactory
         foreach ($searchRequest->getFacets() as $facet) {
             $query = ArrayUtility::arrayMergeRecursiveOverrule($query, [
                 'aggs' => [
-                    $facet->getIdentifier() => [
+                    $facet->getIdentifier() => $facet->getConfig(),
-                        'terms' => [
-                            'field' => $facet->getField(),
-                        ],
-                    ],
                 ],
             ]);
         }

Classes/Domain/Search/SearchService.php

@@ -103,15 +103,10 @@ class SearchService
         }
 
         foreach ($facetsConfig as $identifier => $facetConfig) {
-            if (!isset($facetConfig['field']) || trim($facetConfig['field']) === '') {
-                // TODO: Finish throw
-                throw new \Exception('message', 1499171142);
-            }
-
             $searchRequest->addFacet($this->objectManager->get(
                 FacetRequest::class,
                 $identifier,
-                $facetConfig['field']
+                $facetConfig
             ));
         }
     }

Documentation/source/changelog.rst

@@ -0,0 +1,8 @@
+Changelog
+=========
+
+.. toctree::
+   :maxdepth: 1
+   :glob:
+
+   changelog/*

Documentation/source/changelog/20180406-120-facet-configuration.rst

@@ -0,0 +1,40 @@
+Breacking Change 120 "Pass facets configuration to elasticsearch"
+=================================================================
+
+In order to allow arbitrary facet configuration, we do not process the facet configuration anymore.
+Instead integrators are able to configure facets for search service "as is". We just pipe the
+configuration through.
+
+Therefore the following, which worked before, does not work anymore:
+
+.. code-block:: typoscript
+   :linenos:
+   :emphasize-lines: 4
+
+    plugin.tx_searchcore.settings.search {
+        facets {
+            category {
+                field = categories
+            }
+        }
+    }
+
+Instead you have to provide the full configuration yourself:
+
+.. code-block:: typoscript
+   :linenos:
+   :emphasize-lines: 4,6
+
+    plugin.tx_searchcore.settings.search {
+        facets {
+            category {
+                terms {
+                    field = categories
+                }
+            }
+        }
+    }
+
+You need to add line 4 and 6, the additional level ``terms`` for elasticsearch.
+
+See :issue:`120`.

Documentation/source/index.rst

@@ -15,3 +15,4 @@ Table of Contents
    connections
    indexer
    development
+   changelog

Tests/Functional/Connection/Elasticsearch/FacetTest.php

@@ -0,0 +1,75 @@
+<?php
+namespace Codappix\SearchCore\Tests\Functional\Connection\Elasticsearch;
+
+/*
+ * Copyright (C) 2017  Daniel Siepmann <coding@daniel-siepmann.de>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
+ * 02110-1301, USA.
+ */
+
+use Codappix\SearchCore\Domain\Index\IndexerFactory;
+use Codappix\SearchCore\Domain\Model\SearchRequest;
+use Codappix\SearchCore\Domain\Search\SearchService;
+use TYPO3\CMS\Extbase\Object\ObjectManager;
+
+class FacetTest extends AbstractFunctionalTestCase
+{
+    protected function getTypoScriptFilesForFrontendRootPage()
+    {
+        return array_merge(parent::getTypoScriptFilesForFrontendRootPage(), ['EXT:search_core/Tests/Functional/Fixtures/Searching/Facet.ts']);
+    }
+
+    protected function getDataSets()
+    {
+        return array_merge(
+            parent::getDataSets(),
+            ['Tests/Functional/Fixtures/Searching/Filter.xml']
+        );
+    }
+
+    /**
+     * @test
+     */
+    public function itsPossibleToFetchFacetsForField()
+    {
+        \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
+            ->get(IndexerFactory::class)
+            ->getIndexer('tt_content')
+            ->indexAllDocuments()
+            ;
+
+        $searchService = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
+            ->get(SearchService::class);
+
+        $searchRequest = new SearchRequest();
+        $result = $searchService->search($searchRequest);
+
+        $this->assertSame(1, count($result->getFacets()), 'Did not receive the single defined facet.');
+
+        $facet = current($result->getFacets());
+        $this->assertSame('contentTypes', $facet->getName(), 'Name of facet was not as expected.');
+        $this->assertSame('CType', $facet->getField(), 'Field of facet was not expected.');
+
+        $options = $facet->getOptions();
+        $this->assertSame(2, count($options), 'Did not receive the expected number of possible options for facet.');
+        $option = $options['HTML'];
+        $this->assertSame('HTML', $option->getName(), 'Option did not have expected Name.');
+        $this->assertSame(1, $option->getCount(), 'Option did not have expected count.');
+        $option = $options['Header'];
+        $this->assertSame('Header', $option->getName(), 'Option did not have expected Name.');
+        $this->assertSame(1, $option->getCount(), 'Option did not have expected count.');
+    }
+}

Tests/Functional/Connection/Elasticsearch/FilterTest.php

@@ -58,37 +58,4 @@ class FilterTest extends AbstractFunctionalTestCase
         $this->assertSame(5, $result->getResults()[0]['uid'], 'Did not get the expected result entry.');
         $this->assertSame(1, count($result), 'Did not receive the single filtered element.');
     }
-
-    /**
-     * @test
-     */
-    public function itsPossibleToFetchFacetsForField()
-    {
-        \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
-            ->get(IndexerFactory::class)
-            ->getIndexer('tt_content')
-            ->indexAllDocuments()
-            ;
-
-        $searchService = \TYPO3\CMS\Core\Utility\GeneralUtility::makeInstance(ObjectManager::class)
-            ->get(SearchService::class);
-
-        $searchRequest = new SearchRequest('Search Word');
-        $result = $searchService->search($searchRequest);
-
-        $this->assertSame(1, count($result->getFacets()), 'Did not receive the single defined facet.');
-
-        $facet = current($result->getFacets());
-        $this->assertSame('contentTypes', $facet->getName(), 'Name of facet was not as expected.');
-        $this->assertSame('CType', $facet->getField(), 'Field of facet was not expected.');
-
-        $options = $facet->getOptions();
-        $this->assertSame(2, count($options), 'Did not receive the expected number of possible options for facet.');
-        $option = $options['HTML'];
-        $this->assertSame('HTML', $option->getName(), 'Option did not have expected Name.');
-        $this->assertSame(1, $option->getCount(), 'Option did not have expected count.');
-        $option = $options['Header'];
-        $this->assertSame('Header', $option->getName(), 'Option did not have expected Name.');
-        $this->assertSame(1, $option->getCount(), 'Option did not have expected count.');
-    }
 }

Tests/Functional/Fixtures/BasicSetup.ts

@@ -37,12 +37,6 @@ plugin {
             }
 
             searching {
-                facets {
-                    contentTypes {
-                        field = CType
-                    }
-                }
-
                 fields {
                     query = _all
                 }

Tests/Functional/Fixtures/Searching/Facet.ts

@@ -0,0 +1,17 @@
+plugin {
+    tx_searchcore {
+        settings {
+            searching {
+                facets {
+                    contentTypes {
+                        terms {
+                            field = CType
+                        }
+                    }
+                }
+            }
+        }
+    }
+}
+
+module.tx_searchcore < plugin.tx_searchcore

Tests/Unit/Domain/Search/QueryFactoryTest.php

@@ -118,8 +118,8 @@ class QueryFactoryTest extends AbstractUnitTestCase
     {
         $this->configureConfigurationMockWithDefault();
         $searchRequest = new SearchRequest('SearchWord');
-        $searchRequest->addFacet(new FacetRequest('Identifier', 'FieldName'));
+        $searchRequest->addFacet(new FacetRequest('Identifier', ['terms' => ['field' => 'FieldName']]));
-        $searchRequest->addFacet(new FacetRequest('Identifier 2', 'FieldName 2'));
+        $searchRequest->addFacet(new FacetRequest('Identifier 2', ['terms' => ['field' => 'FieldName 2']]));
 
         $query = $this->subject->create($searchRequest);
         $this->assertSame(