Add sort-abilitiy (#61)

* Update docs, stop using forced keyword * Add fielddata argument for sorting. And sort * Fix the fixture file
Firesphere · Oct 19, 2023 · 2c41813 · 2c41813
1 parent 77a257d
commit 2c41813
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 30 deletions.
diff --git a/docs/03-Set-up-and-Configuration.md b/docs/03-Set-up-and-Configuration.md
@@ -48,11 +48,6 @@ Firesphere\ElasticSearch\Services\ElasticCoreService:
         apiKey: MyBase64EncodedApiKeyHere===
 ```
 
-### Debug
-
-You can force the debugging to false, by setting the debug flag. If you omit this tag, CLI and Dev mode
-will have debugging enabled.
-
 #### ShowInSearch
 
 `ShowInSearch` is handled by the module itself, so there is no need to configure it within your YML/PHP index definition. 
@@ -68,7 +63,6 @@ There is no effective need for items to be in the search, if they're not suppose
 be displayed.
 
 #### Dirty classes
-*NOTE* This is currently unfinished
 
 If a change fails to update, a `DirtyClass` is created, recording the need for updating
 said object. It is recommended to automatically run the `ClearDirtyClasses` task every few hours
@@ -105,7 +99,6 @@ Firesphere\ElasticSearch\Indexes\ElasticIndex:
         Title: TestObject
 
 ```
-**NOTE** Facets are on to-do
 
 #### MySearchIndex
 

diff --git a/docs/06-Advanced-Options/01-Faceting.md b/docs/06-Advanced-Options/01-Faceting.md
@@ -22,16 +22,9 @@ will result in:
 
  `UserID:1 AND UserID:2 AND Parent:5`
 
-### OR facets
-
-Using OR facets, each facet is treated as a separate part of the query. In the above example, 
-it would lead to the following query:
-
-`UserID:1 AND UserID:2 OR Parent:5`
-
 ## Difference between FacetFields and FacetFilters
 
-- Facet _fields_, are the fields that are expected to be returned by Solr and need to be configured.
+- Facet _fields_, are the fields that are expected to be returned by Elastic and need to be configured.
 - Facet _filters_, are the actual filters, that are applied at query time, to narrow down the results by the selected Facets.
 
 ## Applying facets
@@ -49,7 +42,7 @@ To use AND facets, this example should get you started:
 ```php
     $data = Controller::curr()->getRequest()->getVars();
     $index = Injector::inst()->get(MyIndex::class);
-    $query = Injector::inst()->get(BaseQuery::class);
+    $query = Injector::inst()->get(ElasticQuery::class);
     $facetedFields = $index->getFacetFields();
     foreach ($facetedFields as $className => $field) {
         // Title of your field, as defined in the FacetFields
@@ -62,7 +55,7 @@ To use AND facets, this example should get you started:
 
 *Note*, `addFacetFilter` and `addAndFacetFilter` are interchangeable.
 
-### OR facets
+### OR facets **TODO**
 
 To use OR facets, this example should get you started:
 

diff --git a/src/Queries/QueryBuilder.php b/src/Queries/QueryBuilder.php
@@ -48,6 +48,7 @@ public static function buildQuery(BaseQuery $query, CoreIndex $index): array
         $highlights = $self->getHighlighter();
         $suggests = $self->getSuggestTermList();
         $aggregates = $self->getAggregates();
+        $sort = $self->getSort();
         $body = [];
         if (count($terms)) {
             $body['query']['bool'] = $terms;
@@ -64,6 +65,9 @@ public static function buildQuery(BaseQuery $query, CoreIndex $index): array
         if (count($aggregates)) {
             $body['aggs'] = $aggregates;
         }
+        if (count($sort)) {
+            $body['sort'] = $sort;
+        }
 
         return [
             'index' => $index->getIndexName(),
@@ -273,15 +277,20 @@ private function getSuggestTermList()
         return $suggest;
     }
 
-    public function getAggregates()
+    /**
+     * Build the query part for aggregation/faceting
+     *
+     * @return array
+     */
+    private function getAggregates()
     {
         $aggregates = [];
 
         $facets = $this->index->getFacetFields();
 
         foreach ($facets as $class => $facet) {
             $shortClass = ClassInfo::shortName($facet['BaseClass']);
-            $field = sprintf('%s.%s.keyword', $shortClass, $facet['Field']);
+            $field = sprintf('%s.%s', $shortClass, $facet['Field']);
             $aggregates[$facet['Title']] = [
                 'terms' => [
                     'field' => $field
@@ -292,4 +301,9 @@ public function getAggregates()
 
         return $aggregates;
     }
+
+    private function getSort()
+    {
+        return $this->query->getSort();
+    }
 }
diff --git a/src/Tasks/ElasticConfigureTask.php b/src/Tasks/ElasticConfigureTask.php
@@ -40,6 +40,16 @@ class ElasticConfigureTask extends BuildTask
      */
     protected $service;
 
+    /**
+     * DBHTML and DBText etc. should never be made sortable
+     * It doesn't make sense for large text objects
+     * @var string[]
+     */
+    private static $unSsortables = [
+        'HTML',
+        'Text'
+    ];
+
     /**
      * @throws NotFoundExceptionInterface
      */
@@ -108,7 +118,6 @@ protected function configureIndex($instance): Elasticsearch
     {
         $indexName = $instance->getIndexName();
 
-
         $instanceConfig = $this->createConfigForIndex($instance);
 
         $mappings = $this->convertForJSON($instanceConfig);
@@ -123,14 +132,15 @@ protected function configureIndex($instance): Elasticsearch
             $msg = sprintf($msg, 'Updating', $indexName);
             DB::alteration_message($msg);
             $this->getLogger()->info($msg);
+
             return $client->indices()->putMapping($body);
-        } else {
-            $body['body']['mappings'] = $mappings;
-            $msg = sprintf($msg, 'Creating', $indexName);
-            DB::alteration_message($msg);
-            $this->getLogger()->info($msg);
-            return $client->indices()->create($body);
         }
+        $body['body']['mappings'] = $mappings;
+        $msg = sprintf($msg, 'Creating', $indexName);
+        DB::alteration_message($msg);
+        $this->getLogger()->info($msg);
+
+        return $client->indices()->create($body);
     }
 
     /**
@@ -171,6 +181,13 @@ protected function convertForJSON($config)
             $base[$conf['name']] = [
                 'type' => $typeMap[$conf['type'] ?? '*']
             ];
+            $shouldHold = true;
+            foreach (self::$unSsortables as $unSortable) {
+                $shouldHold = !str_contains($conf['type'], $unSortable) && $shouldHold;
+            }
+            if ($shouldHold && $typeMap[$conf['type']] === 'text') {
+                $base[$conf['name']]['fielddata'] = true;
+            }
         }
 
         return ['properties' => $base];

diff --git a/tests/Fixtures/elasticresponse.json b/tests/Fixtures/elasticresponse.json
@@ -88,7 +88,7 @@
       "sum_other_doc_count": 0,
       "buckets": [
         {
-          "key": "TestObject",
+          "key": 1,
           "doc_count": 1
         }
       ]
@@ -104,4 +104,4 @@
       }
     ]
   }
-}
+}
diff --git a/tests/unit/Indexes/ElasticIndexTest.php b/tests/unit/Indexes/ElasticIndexTest.php
@@ -58,6 +58,7 @@ public function testAddSetGet()
         $conf['FulltextFields'] = array_merge(
             $conf['FulltextFields'] ?? [],
         );
+        $conf['FulltextFields'][] = 'TestObject.ID';
         $this->assertEquals($conf['FulltextFields'], $index->getFulltextFields());
         $index->addFulltextField('Dummyfield');
         $conf['FulltextFields'][] = 'Dummyfield';
@@ -85,7 +86,7 @@ public function testAddSetGet()
         $expectedFacets = [
             'TestObject' => [
                 'BaseClass' => 'Page',
-                'Field'     => 'TestObject.Title',
+                'Field'     => 'TestObject.ID',
                 'Title'     => 'TestObject',
             ]
         ];

diff --git a/tests/unit/Queries/QueryBuilderTest.php b/tests/unit/Queries/QueryBuilderTest.php
@@ -68,7 +68,7 @@ class QueryBuilderTest extends SapphireTest
             'aggs'    => [
                 'TestObject' => [
                     'terms' => [
-                        'field' => 'Page.TestObject.Title.keyword'
+                        'field' => 'Page.TestObject.ID'
                     ]
                 ]
             ]
@@ -123,5 +123,11 @@ public function testBuildQuery()
         $this->assertEquals('Test', $resultQuery['body']['suggest']['0-partterm']['text']);
         $this->assertEquals('Tset', $resultQuery['body']['suggest']['1-partterm']['text']);
         $this->assertEquals('Test Tset', $resultQuery['body']['suggest']['1-fullterm']['text']);
+
+        $query->setSort(['Title' => 'asc']);
+
+        $resultQuery = QueryBuilder::buildQuery($query, $idx);
+
+        $this->assertEquals(['Title' => 'asc'], $resultQuery['body']['sort']);
     }
 }