From 44cb5824e7d9a9dfb7365f07f103c5c2a05dc7bf Mon Sep 17 00:00:00 2001
From: Mathieu Darse <mdarse@jolicode.com>
Date: Thu, 16 Jul 2015 20:11:43 +0200
Subject: [PATCH] Implement private fields on range an equal nodes

New QueryContext::get() method
Removed QueryContext::normalizeField(), can be replaced with get() and a call to
Field::getIndexField().
---
 .../Elastic/AST/FieldEqualsExpression.php     | 14 ++++++---
 .../Elastic/AST/RangeExpression.php           | 14 ++++++---
 .../Elastic/Search/QueryContext.php           | 30 ++++++++-----------
 .../Elastic/Search/QueryHelper.php            | 27 ++++++++++++++---
 .../SearchEngine/Search/QueryContextTest.php  | 13 --------
 5 files changed, 56 insertions(+), 42 deletions(-)

diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php
index 6074ee941b..74d06e2d69 100644
--- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php
+++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php
@@ -2,7 +2,9 @@
 
 namespace Alchemy\Phrasea\SearchEngine\Elastic\AST;
 
+use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
+use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper;
 
 class FieldEqualsExpression extends Node
 {
@@ -17,11 +19,15 @@ class FieldEqualsExpression extends Node
 
     public function buildQuery(QueryContext $context)
     {
-        $field = $context->normalizeField($this->field->getValue());
-        $query = array();
-        $query['term'][$field] = $this->value;
+        $structure_field = $context->get($this->field);
+        if (!$structure_field) {
+            throw new QueryException(sprintf('Field "%s" does not exist', $this->field->getValue()));
+        }
 
-        return $query;
+        $query = [];
+        $query['term'][$structure_field->getIndexField()] = $this->value;
+
+        return QueryHelper::wrapPrivateFieldQuery($structure_field, $query);
     }
 
     public function getTermNodes()
diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php
index a8b95e7455..8f921d418d 100644
--- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php
+++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php
@@ -2,7 +2,9 @@
 
 namespace Alchemy\Phrasea\SearchEngine\Elastic\AST;
 
+use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
+use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper;
 
 class RangeExpression extends Node
 {
@@ -59,11 +61,15 @@ class RangeExpression extends Node
             }
         }
 
-        $field = $context->normalizeField($this->field->getValue());
-        $query = array();
-        $query['range'][$field] = $params;
+        $structure_field = $context->get($this->field);
+        if (!$structure_field) {
+            throw new QueryException(sprintf('Field "%s" does not exist', $this->field->getValue()));
+        }
 
-        return $query;
+        $query = [];
+        $query['range'][$structure_field->getIndexField()] = $params;
+
+        return QueryHelper::wrapPrivateFieldQuery($structure_field, $query);
     }
 
     public function getTermNodes()
diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php
index aa981930b3..0abf48256e 100644
--- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php
+++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php
@@ -4,6 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Search;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field;
+use Alchemy\Phrasea\SearchEngine\Elastic\AST\Field as ASTField;
 use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Structure;
 
 /**
@@ -91,6 +92,18 @@ class QueryContext
         return array_values($fields);
     }
 
+    public function get($name)
+    {
+        if ($name instanceof ASTField) {
+            $name = $name->getValue();
+        }
+        $field = $this->structure->get($name);
+        if (!$field) {
+            return null;
+        }
+        return $field;
+    }
+
     /**
      * @todo Maybe we should put this logic in Field class?
      */
@@ -112,23 +125,6 @@ class QueryContext
         return $fields;
     }
 
-    /**
-     * Returns normalized name or null
-     *
-     * @param string $name
-     * @return null|string
-     * @deprecated Use getIndexField() on Field instance
-     */
-    public function normalizeField($name)
-    {
-        $field = $this->structure->get($name);
-        if (!$field) {
-            return null;
-        }
-        // TODO Field label dereferencing (we only want names)
-        return $field->getIndexField();
-    }
-
     public function getFields()
     {
         return $this->fields;
diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php
index 3ad5cf825c..1235334899 100644
--- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php
+++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php
@@ -2,6 +2,8 @@
 
 namespace Alchemy\Phrasea\SearchEngine\Elastic\Search;
 
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field;
+
 class QueryHelper
 {
     private function __construct() {}
@@ -28,15 +30,32 @@ class QueryHelper
         foreach ($fields_map as $hash => $fields) {
             // Right to query on a private field is dependant of document collection
             // Here we make sure we can only match on allowed collections
-            $query = [];
-            $query['bool']['must'][0]['terms']['base_id'] = $collections_map[$hash];
-            $query['bool']['must'][1] = $matcher_callback->__invoke($fields);
-            $queries[] = $query;
+            $queries[] = self::restrictQueryToCollections(
+                $matcher_callback->__invoke($fields),
+                $collections_map[$hash]
+            );
         }
 
         return $queries;
     }
 
+    public static function wrapPrivateFieldQuery(Field $field, array $query)
+    {
+        if ($field->isPrivate()) {
+            return self::restrictQueryToCollections($query, $field->getDependantCollections());
+        } else {
+            return $query;
+        }
+    }
+
+    private static function restrictQueryToCollections(array $query, array $collections)
+    {
+        $wrapper = [];
+        $wrapper['bool']['must'][0]['terms']['base_id'] = $collections;
+        $wrapper['bool']['must'][1] = $query;
+        return $wrapper;
+    }
+
     private static function hashCollections(array $collections)
     {
         sort($collections, SORT_REGULAR);
diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/Search/QueryContextTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/Search/QueryContextTest.php
index 24ce78f9a1..ac2e73b9b6 100644
--- a/tests/Alchemy/Tests/Phrasea/SearchEngine/Search/QueryContextTest.php
+++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/Search/QueryContextTest.php
@@ -22,19 +22,6 @@ class QueryContextTest extends \PHPUnit_Framework_TestCase
         $this->assertEquals(['some_field'], $narrowed->getFields());
     }
 
-    public function testFieldNormalization()
-    {
-        $public_field = new Field('foo', Mapping::TYPE_STRING, ['private' => false]);
-        $restricted_field = new Field('bar', Mapping::TYPE_STRING, ['private' => true]);
-        $structure = $this->prophesize(Structure::class);
-        $structure->get('foo')->willReturn($public_field);
-        $structure->get('bar')->willReturn($restricted_field);
-
-        $context = new QueryContext($structure->reveal(), [], 'fr');
-        $this->assertEquals('caption.foo', $context->normalizeField('foo'));
-        $this->assertEquals('private_caption.bar', $context->normalizeField('bar'));
-    }
-
     public function testGetUnrestrictedFields()
     {
         $foo_field = new Field('foo', Mapping::TYPE_STRING, ['private' => false]);