Test value compatibility before creating ES query

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php

@@ -24,7 +24,7 @@ class EqualExpression extends Node
     public function buildQuery(QueryContext $context)
     {
         if (!$this->key->isValueCompatible($this->value, $context)) {
-            return null;
+            throw new QueryException(sprintf('Value "%s" for metadata tag "%s" is not valid.', $this->value, $this->key));
         }
 
         $query = [

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php

@@ -6,6 +6,7 @@ use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryPostProcessor;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\ValueChecker;
 use Assert\Assertion;
 
 class FieldKey implements Key, QueryPostProcessor
@@ -26,7 +27,7 @@ class FieldKey implements Key, QueryPostProcessor
 
     public function isValueCompatible($value, QueryContext $context)
     {
-        return $this->getField($context)->isValueCompatible($value);
+        return ValueChecker::isValueCompatible($this->getField($context), $value);
     }
 
     public function postProcessQuery($query, QueryContext $context)

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MatchExpression.php

@@ -3,6 +3,7 @@
 namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\AST\Node;
+use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
 use Assert\Assertion;
 
@@ -20,6 +21,10 @@ class MatchExpression extends Node
 
     public function buildQuery(QueryContext $context)
     {
+        if (!$this->key->isValueCompatible($this->value, $context)) {
+            throw new QueryException(sprintf('Value "%s" for metadata tag "%s" is not valid.', $this->value, $this->key));
+        }
+
         return [
             'match' => [
                 $this->key->getIndexField($context) => $this->value

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php

@@ -4,6 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\ValueChecker;
 use Assert\Assertion;
 
 class MetadataKey implements Key
@@ -24,7 +25,7 @@ class MetadataKey implements Key
 
     public function isValueCompatible($value, QueryContext $context)
     {
-        return $this->getTag($context)->isValueCompatible($value);
+        return ValueChecker::isValueCompatible($this->getTag($context), $value);
     }
 
     private function getTag(QueryContext $context)

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php

@@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper;
-use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field as StructureField;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\ValueChecker;
 
 class QuotedTextNode extends Node
 {
@@ -38,11 +38,11 @@ class QuotedTextNode extends Node
         };
 
         $unrestricted_fields = $context->getUnrestrictedFields();
-        $unrestricted_fields = StructureField::filterByValueCompatibility($unrestricted_fields, $this->text);
+        $unrestricted_fields = ValueChecker::filterByValueCompatibility($unrestricted_fields, $this->text);
         $query = $query_builder($unrestricted_fields);
 
         $private_fields = $context->getPrivateFields();
-        $private_fields = StructureField::filterByValueCompatibility($private_fields, $this->text);
+        $private_fields = ValueChecker::filterByValueCompatibility($private_fields, $this->text);
         foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $query_builder) as $private_field_query) {
             $query = QueryHelper::applyBooleanClause($query, 'should', $private_field_query);
         }

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php

@@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper;
-use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field as StructureField;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\ValueChecker;
 
 class RawNode extends Node
 {
@@ -56,11 +56,11 @@ class RawNode extends Node
         };
 
         $unrestricted_fields = $context->getUnrestrictedFields();
-        $unrestricted_fields = StructureField::filterByValueCompatibility($unrestricted_fields, $this->text);
+        $unrestricted_fields = ValueChecker::filterByValueCompatibility($unrestricted_fields, $this->text);
         $query = $query_builder($unrestricted_fields);
 
         $private_fields = $context->getPrivateFields();
-        $private_fields = StructureField::filterByValueCompatibility($private_fields, $this->text);
+        $private_fields = ValueChecker::filterByValueCompatibility($private_fields, $this->text);
         foreach (QueryHelper::wrapPrivateFieldQueries($private_fields, $query_builder) as $private_field_query) {
             $query = QueryHelper::applyBooleanClause($query, 'should', $private_field_query);
         }

lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php

@@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext;
 use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper;
-use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field as StructureField;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\ValueChecker;
 use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Term;
 
 class TextNode extends AbstractTermNode implements ContextAbleInterface
@@ -41,7 +41,7 @@ class TextNode extends AbstractTermNode implements ContextAbleInterface
         $query_builder = function (array $fields) use ($context) {
             // Full text
             $index_fields = [];
-            foreach (StructureField::filterByValueCompatibility($fields, $this->text) as $field) {
+            foreach (ValueChecker::filterByValueCompatibility($fields, $this->text) as $field) {
                 foreach ($context->localizeField($field) as $f) {
                     $index_fields[] = $f;
                 }

lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Field.php

@@ -4,7 +4,6 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Structure;
 
 use Alchemy\Phrasea\SearchEngine\Elastic\Exception\MergeException;
 use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
-use Alchemy\Phrasea\SearchEngine\Elastic\RecordHelper;
 use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Concept;
 use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\Helper as ThesaurusHelper;
 use Assert\Assertion;
@@ -13,7 +12,7 @@ use databox_field;
 /**
  * @todo Field labels
  */
-class Field
+class Field implements Typed
 {
     private $name;
     private $type;
@@ -119,41 +118,6 @@ class Field
         );
     }
 
-    public function isValueCompatible($value)
-    {
-        return count(self::filterByValueCompatibility([$this], $value)) > 0;
-    }
-
-    public static function filterByValueCompatibility(array $fields, $value)
-    {
-        $is_numeric = is_numeric($value);
-        $is_valid_date = RecordHelper::validateDate($value);
-        $filtered = [];
-        foreach ($fields as $field) {
-            switch ($field->type) {
-                case Mapping::TYPE_FLOAT:
-                case Mapping::TYPE_DOUBLE:
-                case Mapping::TYPE_INTEGER:
-                case Mapping::TYPE_LONG:
-                case Mapping::TYPE_SHORT:
-                case Mapping::TYPE_BYTE:
-                    if ($is_numeric) {
-                        $filtered[] = $field;
-                    }
-                    break;
-                case Mapping::TYPE_DATE:
-                    if ($is_valid_date) {
-                        $filtered[] = $field;
-                    }
-                    break;
-                case Mapping::TYPE_STRING:
-                default:
-                    $filtered[] = $field;
-            }
-        }
-        return $filtered;
-    }
-
     public function getConceptPathIndexField()
     {
         return sprintf('concept_path.%s', $this->name);

lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Tag.php

@@ -5,7 +5,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Structure;
 use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
 use Assert\Assertion;
 
-class Tag
+class Tag implements Typed
 {
     private $name;
     private $type;
@@ -36,11 +36,6 @@ class Tag
         return $this->analyzable;
     }
 
-    public function isValueCompatible()
-    {
-        return true;
-    }
-
     public function getIndexField($raw = false)
     {
         return sprintf(

lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Typed.php

@@ -0,0 +1,13 @@
+<?php
+
+namespace Alchemy\Phrasea\SearchEngine\Elastic\Structure;
+
+interface Typed
+{
+    /**
+     * Get the type of the object
+     *
+     * @return string One of Mapping::TYPE_* constants
+     */
+    public function getType();
+}

lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/ValueChecker.php

@@ -0,0 +1,48 @@
+<?php
+
+namespace Alchemy\Phrasea\SearchEngine\Elastic\Structure;
+
+use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
+use Alchemy\Phrasea\SearchEngine\Elastic\RecordHelper;
+use Assert\Assertion;
+
+class ValueChecker
+{
+    private function __construct() {}
+
+    public static function isValueCompatible(Typed $typed, $value)
+    {
+        return count(self::filterByValueCompatibility([$typed], $value)) > 0;
+    }
+
+    public static function filterByValueCompatibility(array $list, $value)
+    {
+        Assertion::allIsInstanceOf($list, Typed::class);
+        $is_numeric = is_numeric($value);
+        $is_valid_date = RecordHelper::validateDate($value);
+        $filtered = [];
+        foreach ($list as $item) {
+            switch ($item->getType()) {
+                case Mapping::TYPE_FLOAT:
+                case Mapping::TYPE_DOUBLE:
+                case Mapping::TYPE_INTEGER:
+                case Mapping::TYPE_LONG:
+                case Mapping::TYPE_SHORT:
+                case Mapping::TYPE_BYTE:
+                    if ($is_numeric) {
+                        $filtered[] = $item;
+                    }
+                    break;
+                case Mapping::TYPE_DATE:
+                    if ($is_valid_date) {
+                        $filtered[] = $item;
+                    }
+                    break;
+                case Mapping::TYPE_STRING:
+                default:
+                    $filtered[] = $item;
+            }
+        }
+        return $filtered;
+    }
+}

tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php

@@ -12,7 +12,7 @@ use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field as StructureField;
  * @group searchengine
  * @group ast
  */
-class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase
+class EqualExpressionTest extends \PHPUnit_Framework_TestCase
 {
     public function testSerialization()
     {
@@ -33,6 +33,7 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase
         $key = $this->prophesize(Key::class);
         $key->isValueCompatible($value, $query_context)->willReturn($compatible_value);
         $key->getIndexField($query_context, true)->willReturn($index_field);
+        $key->__toString()->willReturn('foo');
         // TODO Test keys implementing QueryPostProcessor
 
         $node = new EqualExpression($key->reveal(), 'bar');
@@ -56,8 +57,22 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase
             ['foo.raw', 'bar', true, false, '{
                 "term": {
                     "foo.raw": "bar" } }'],
-            ['foo.raw', 'bar', false, true, 'null'],
-            ['foo.raw', 'bar', false, false, 'null'],
         ];
     }
+
+    /**
+     * @expectedException Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException
+     * @expectedExceptionMessageRegExp #"foo"#u
+     */
+    public function testQueryBuildWithIncompatibleValue()
+    {
+        $query_context = $this->prophesize(QueryContext::class)->reveal();
+        $key = $this->prophesize(Key::class);
+        $key->isValueCompatible('bar', $query_context)->willReturn(false);
+        $key->getIndexField($query_context, true)->willReturn('foo.raw');
+        $key->__toString()->willReturn('foo');
+
+        $node = new EqualExpression($key->reveal(), 'bar');
+        $node->buildQuery($query_context);
+    }
 }

tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php

@@ -28,6 +28,7 @@ class MatchExpressionTest extends \PHPUnit_Framework_TestCase
     {
         $query_context = $this->prophesize(QueryContext::class)->reveal();
         $key = $this->prophesize(Key::class);
+        $key->isValueCompatible('bar', $query_context)->willReturn(true);
         $key->getIndexField($query_context)->willReturn('foo');
         $node = new MatchExpression($key->reveal(), 'bar');
         $query = $node->buildQuery($query_context);

tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RawNodeTest.php

@@ -24,6 +24,7 @@ class RawNodeTest extends \PHPUnit_Framework_TestCase
     public function testQueryBuildOnSingleField()
     {
         $field = $this->prophesize(Field::class);
+        $field->getType()->willReturn(Mapping::TYPE_STRING);
         $field->getIndexField(true)->willReturn('foo.raw');
 
         $query_context = $this->prophesize(QueryContext::class);
@@ -45,8 +46,10 @@ class RawNodeTest extends \PHPUnit_Framework_TestCase
     public function testQueryBuildOnMultipleFields()
     {
         $field_a = $this->prophesize(Field::class);
+        $field_a->getType()->willReturn(Mapping::TYPE_STRING);
         $field_a->getIndexField(true)->willReturn('foo.raw');
         $field_b = $this->prophesize(Field::class);
+        $field_b->getType()->willReturn(Mapping::TYPE_STRING);
         $field_b->getIndexField(true)->willReturn('bar.raw');
 
         $query_context = $this->prophesize(QueryContext::class);

tests/Alchemy/Tests/Phrasea/SearchEngine/Structure/ValueCheckerTest.php

@@ -0,0 +1,80 @@
+<?php
+
+namespace Alchemy\Tests\Phrasea\SearchEngine\Structure;
+
+use Alchemy\Phrasea\SearchEngine\Elastic\Mapping;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\ValueChecker;
+use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Typed;
+
+/**
+ * @group unit
+ * @group structure
+ */
+class ValueCheckerTest extends \PHPUnit_Framework_TestCase
+{
+    /**
+     * @dataProvider escapeRawProvider
+     */
+    public function testValueCompatibility($subject, $value, $compatible)
+    {
+        $this->assertEquals($compatible, ValueChecker::isValueCompatible($subject, $value));
+    }
+
+    public function escapeRawProvider()
+    {
+        $values = [
+            [Mapping::TYPE_FLOAT  ,  42         , true ],
+            [Mapping::TYPE_FLOAT  , '42'        , true ],
+            [Mapping::TYPE_FLOAT  , '42foo'     , false],
+            [Mapping::TYPE_FLOAT  , 'foo'       , false],
+            [Mapping::TYPE_DOUBLE ,  42         , true ],
+            [Mapping::TYPE_DOUBLE , '42'        , true ],
+            [Mapping::TYPE_DOUBLE , '42foo'     , false],
+            [Mapping::TYPE_DOUBLE , 'foo'       , false],
+            [Mapping::TYPE_INTEGER,  42         , true ],
+            [Mapping::TYPE_INTEGER, '42'        , true ],
+            [Mapping::TYPE_INTEGER, '42foo'     , false],
+            [Mapping::TYPE_INTEGER, 'foo'       , false],
+            [Mapping::TYPE_LONG   ,  42         , true ],
+            [Mapping::TYPE_LONG   , '42'        , true ],
+            [Mapping::TYPE_LONG   , '42foo'     , false],
+            [Mapping::TYPE_LONG   , 'foo'       , false],
+            [Mapping::TYPE_SHORT  ,  42         , true ],
+            [Mapping::TYPE_SHORT  , '42'        , true ],
+            [Mapping::TYPE_SHORT  , '42foo'     , false],
+            [Mapping::TYPE_SHORT  , 'foo'       , false],
+            [Mapping::TYPE_BYTE   ,  42         , true ],
+            [Mapping::TYPE_BYTE   , '42'        , true ],
+            [Mapping::TYPE_BYTE   , '42foo'     , false],
+            [Mapping::TYPE_BYTE   , 'foo'       , false],
+
+            [Mapping::TYPE_STRING , 'foo'       , true ],
+            [Mapping::TYPE_STRING , '42'        , true ],
+            [Mapping::TYPE_STRING ,  42         , true ],
+
+            [Mapping::TYPE_BOOLEAN, true        , true ],
+            [Mapping::TYPE_BOOLEAN, false       , true ],
+            [Mapping::TYPE_BOOLEAN, 'yes'       , true ],
+            [Mapping::TYPE_BOOLEAN, 'no'        , true ],
+            [Mapping::TYPE_BOOLEAN, 'foo'       , true ],
+            [Mapping::TYPE_BOOLEAN, 42          , true ],
+
+            [Mapping::TYPE_DATE   , '2015/01/01'         , true ],
+            [Mapping::TYPE_DATE   , '2015/01/01 00:00:00', false],
+            [Mapping::TYPE_DATE   , 'foo'                , false],
+        ];
+
+        foreach ($values as &$value) {
+            $value[0] = $this->createTypedMock($value[0]);
+        }
+
+        return $values;
+    }
+
+    private function createTypedMock($type)
+    {
+        $typed = $this->prophesize(Typed::class);
+        $typed->getType()->willReturn($type)->shouldBeCalled();
+        return $typed->reveal();
+    }
+}