From 13186209f77b3d6e5e26eea8a36ee00594ac871e Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Wed, 4 Nov 2015 19:39:15 +0100 Subject: [PATCH 01/22] Handle matching on file metadata --- grammar/query.pp | 11 +++-- .../Elastic/AST/MetadataMatchStatement.php | 45 +++++++++++++++++++ .../SearchEngine/Elastic/Search/NodeTypes.php | 2 + .../Elastic/Search/QueryHelper.php | 31 +++++++++++++ .../Elastic/Search/QueryVisitor.php | 16 +++++++ .../SearchEngine/resources/queries.csv | 3 ++ 6 files changed, 105 insertions(+), 3 deletions(-) create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php diff --git a/grammar/query.pp b/grammar/query.pp index c926084eee..75e44603b4 100644 --- a/grammar/query.pp +++ b/grammar/query.pp @@ -32,6 +32,7 @@ %token id id|recordid %token field_prefix field. %token flag_prefix flag. +%token meta_prefix meta. %token true true|1 %token false false|0 %token word [^\s\(\)\[\]:<>≤≥=]+ @@ -68,6 +69,7 @@ quaternary: key_value_pair: native_key() ::colon:: ::space::? value() #native_key_value + | ::meta_prefix:: meta_key() ::colon:: ::space::? value() #meta_statement | ::flag_prefix:: flag() ::colon:: ::space::? boolean() #flag_statement | ::field_prefix:: field() ::colon:: ::space::? term() #field_statement | field() ::colon:: ::space::? term() #field_statement @@ -77,15 +79,18 @@ key_value_pair: | field() ::space::? ::gte:: ::space::? value() #greater_than_or_equal_to | field() ::space::? ::equal:: ::space::? value() #equal_to -#flag: - word_or_keyword()+ - #native_key: | | | +#meta_key: + word_or_keyword()+ + +#flag: + word_or_keyword()+ + #field: word_or_keyword()+ | quoted_string() diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php new file mode 100644 index 0000000000..11c0c4d58a --- /dev/null +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php @@ -0,0 +1,45 @@ +name = $name; + $this->value = $value; + } + + public function buildQuery(QueryContext $context) + { + if (!QueryHelper::isValidMetadataName($this->name)) { + throw new QueryException(sprintf('"%s" is not a valid metadata name', $this->name)); + } + $field = sprintf('exif.%s', $this->name); + return [ + 'term' => [ + $field => $this->value + ] + ]; + } + + public function getTermNodes() + { + return array(); + } + + public function __toString() + { + return sprintf('', $this->name, $this->value); + } +} diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php index c74df1a906..f6a8ae9f76 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php @@ -21,6 +21,8 @@ class NodeTypes const TERM = '#thesaurus_term'; const TEXT = '#text'; const CONTEXT = '#context'; + const METADATA_STATEMENT = '#meta_statement'; + const METADATA_KEY = '#meta_key'; const FLAG_STATEMENT = '#flag_statement'; const FLAG = '#flag'; const NATIVE_KEY_VALUE = '#native_key_value'; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php index 9d652f86c7..95d30fc685 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php @@ -3,6 +3,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Search; use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field; +use media_subdef; class QueryHelper { @@ -107,4 +108,34 @@ class QueryHelper return $query; } } + + public static function isValidMetadataName($name) + { + return in_array($name, [ + // To keep in sync with RecordIndexer::getExifMapping() + media_subdef::TC_DATA_WIDTH, + media_subdef::TC_DATA_HEIGHT, + media_subdef::TC_DATA_COLORSPACE, + media_subdef::TC_DATA_CHANNELS, + media_subdef::TC_DATA_ORIENTATION, + media_subdef::TC_DATA_COLORDEPTH, + media_subdef::TC_DATA_DURATION, + media_subdef::TC_DATA_AUDIOCODEC, + media_subdef::TC_DATA_AUDIOSAMPLERATE, + media_subdef::TC_DATA_VIDEOCODEC, + media_subdef::TC_DATA_FRAMERATE, + media_subdef::TC_DATA_MIMETYPE, + media_subdef::TC_DATA_FILESIZE, + media_subdef::TC_DATA_LONGITUDE, + media_subdef::TC_DATA_LATITUDE, + media_subdef::TC_DATA_FOCALLENGTH, + media_subdef::TC_DATA_CAMERAMODEL, + media_subdef::TC_DATA_FLASHFIRED, + media_subdef::TC_DATA_APERTURE, + media_subdef::TC_DATA_SHUTTERSPEED, + media_subdef::TC_DATA_HYPERFOCALDISTANCE, + media_subdef::TC_DATA_ISO, + media_subdef::TC_DATA_LIGHTVALUE + ]); + } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 336724cd99..94d8f81343 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -83,6 +83,12 @@ class QueryVisitor implements Visit case NodeTypes::FIELD: return new AST\Field($this->visitString($element)); + case NodeTypes::METADATA_STATEMENT: + return $this->visitMetadataStatementNode($element); + + case NodeTypes::METADATA_KEY: + return $this->visitString($element); + case NodeTypes::FLAG_STATEMENT: return $this->visitFlagStatementNode($element); @@ -298,6 +304,16 @@ class QueryVisitor implements Visit } } + private function visitMetadataStatementNode(TreeNode $node) + { + if ($node->getChildrenNumber() !== 2) { + throw new Exception('Flag statement can only have 2 childs.'); + } + $name = $this->visit($node->getChild(0)); + $value = $this->visit($node->getChild(1)); + return new AST\MetadataMatchStatement($name, $value); + } + private function visitNativeKeyValueNode(TreeNode $node) { if ($node->getChildrenNumber() !== 2) { diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv index 3c89dd5483..ff207a8f82 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv @@ -102,6 +102,9 @@ flag.true:true| flag.foo bar:true|( AND ( MATCHES )) true| +# Metadata (EXIF or anything else) matcher +meta.MimeType:image/jpeg| + # Matcher on unknown name --> fulltext foo:bar|( MATCHES ) foo:bar AND baz|(( MATCHES ) AND ) From 79142bff100795bd0fc717db0d3e0ff4ee4d59ec Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Thu, 5 Nov 2015 11:33:51 +0100 Subject: [PATCH 02/22] Allow literal "meta." inside strings --- grammar/query.pp | 1 + 1 file changed, 1 insertion(+) diff --git a/grammar/query.pp b/grammar/query.pp index 75e44603b4..a4ef731ea4 100644 --- a/grammar/query.pp +++ b/grammar/query.pp @@ -154,6 +154,7 @@ keyword: | | | + | | | From f94b4c6b809f80dbdb08ad211ed2750eb5956f6a Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Thu, 5 Nov 2015 11:42:43 +0100 Subject: [PATCH 03/22] =?UTF-8?q?Escape=20trailing=20=C2=AB=C2=A0.=C2=A0?= =?UTF-8?q?=C2=BB=20on=20key=20prefixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- grammar/query.pp | 6 +++--- .../Tests/Phrasea/SearchEngine/resources/queries.csv | 5 +++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/grammar/query.pp b/grammar/query.pp index a4ef731ea4..9511c417fe 100644 --- a/grammar/query.pp +++ b/grammar/query.pp @@ -30,9 +30,9 @@ %token collection collection %token type type %token id id|recordid -%token field_prefix field. -%token flag_prefix flag. -%token meta_prefix meta. +%token field_prefix field\. +%token flag_prefix flag\. +%token meta_prefix meta\. %token true true|1 %token false false|0 %token word [^\s\(\)\[\]:<>≤≥=]+ diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv index ff207a8f82..b29461a7c2 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv @@ -105,6 +105,11 @@ true| # Metadata (EXIF or anything else) matcher meta.MimeType:image/jpeg| +# Unescaped "." issue on key prefixes +fieldOne:foo|( MATCHES ) +flagged:true|( MATCHES ) +metadata:foo|( MATCHES ) + # Matcher on unknown name --> fulltext foo:bar|( MATCHES ) foo:bar AND baz|(( MATCHES ) AND ) From 803a9010a58cad935839eef740c2feec0006f54f Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Thu, 5 Nov 2015 15:35:21 +0100 Subject: [PATCH 04/22] Pass metadata key as value object --- .../Elastic/AST/KeyValue/MetadataKey.php | 22 ++++++++++ .../Elastic/AST/MetadataMatchStatement.php | 15 +++---- .../Elastic/Search/QueryVisitor.php | 13 +++++- .../AST/MetadataMatchStatementTest.php | 40 +++++++++++++++++++ 4 files changed, 80 insertions(+), 10 deletions(-) create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php create mode 100644 tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php new file mode 100644 index 0000000000..a156b1068d --- /dev/null +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php @@ -0,0 +1,22 @@ +name = $name; + } + + public function __toString() + { + return $this->name; + } +} diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php index 11c0c4d58a..fe516d6f65 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php @@ -3,29 +3,26 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST; use Assert\Assertion; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\MetadataKey; use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper; class MetadataMatchStatement extends Node { - private $name; + private $key; private $value; - public function __construct($name, $value) + public function __construct(MetadataKey $key, $value) { - Assertion::string($name); Assertion::string($value); - $this->name = $name; + $this->key = $key; $this->value = $value; } public function buildQuery(QueryContext $context) { - if (!QueryHelper::isValidMetadataName($this->name)) { - throw new QueryException(sprintf('"%s" is not a valid metadata name', $this->name)); - } - $field = sprintf('exif.%s', $this->name); + $field = sprintf('exif.%s', $this->key); return [ 'term' => [ $field => $this->value @@ -40,6 +37,6 @@ class MetadataMatchStatement extends Node public function __toString() { - return sprintf('', $this->name, $this->value); + return sprintf('', $this->key, $this->value); } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 94d8f81343..1e0625e6ce 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -4,6 +4,8 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Search; use Alchemy\Phrasea\SearchEngine\Elastic\AST; use Alchemy\Phrasea\SearchEngine\Elastic\Exception\Exception; +use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException; +use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper; use Hoa\Compiler\Llk\TreeNode; use Hoa\Visitor\Element; use Hoa\Visitor\Visit; @@ -87,7 +89,7 @@ class QueryVisitor implements Visit return $this->visitMetadataStatementNode($element); case NodeTypes::METADATA_KEY: - return $this->visitString($element); + return $this->visitMetadataKeyNode($element); case NodeTypes::FLAG_STATEMENT: return $this->visitFlagStatementNode($element); @@ -304,6 +306,15 @@ class QueryVisitor implements Visit } } + private function visitMetadataKeyNode(TreeNode $node) + { + $name = $this->visitString($node); + if (!QueryHelper::isValidMetadataName($name)) { + throw new QueryException(sprintf('"%s" is not a valid metadata name', $name)); + } + return new AST\KeyValue\MetadataKey($name); + } + private function visitMetadataStatementNode(TreeNode $node) { if ($node->getChildrenNumber() !== 2) { diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php new file mode 100644 index 0000000000..439a646b8f --- /dev/null +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php @@ -0,0 +1,40 @@ +assertTrue(method_exists(MetadataMatchStatement::class, '__toString'), 'Class does not have method __toString'); + $key = new MetadataKey('foo'); + $node = new MetadataMatchStatement($key, 'bar'); + $this->assertEquals('', (string) $node); + } + + public function testQueryBuild() + { + $query_context = $this->prophesize(QueryContext::class); + + $key = new MetadataKey('foo'); + $node = new MetadataMatchStatement($key, 'bar'); + $query = $node->buildQuery($query_context->reveal()); + + $expected = '{ + "term": { + "exif.foo": "bar" + } + }'; + + $this->assertEquals(json_decode($expected, true), $query); + } +} From 713442c7b487cdcec4409ec308536cd34b95dbcf Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Thu, 5 Nov 2015 16:52:01 +0100 Subject: [PATCH 05/22] Merge metadata & native key expressions Do not delegates query building to Key instance anymore. --- .../Elastic/AST/KeyValue/Expression.php | 12 ++-- .../SearchEngine/Elastic/AST/KeyValue/Key.php | 4 +- .../Elastic/AST/KeyValue/MetadataKey.php | 9 ++- .../Elastic/AST/KeyValue/NativeKey.php | 12 +--- .../Elastic/AST/MetadataMatchStatement.php | 42 ------------ .../Elastic/Search/QueryVisitor.php | 24 ++----- .../AST/KeyValueExpressionTest.php | 27 ++++++-- .../AST/MetadataMatchStatementTest.php | 40 ----------- .../SearchEngine/AST/NativeKeyTest.php | 66 ++++--------------- .../SearchEngine/resources/queries.csv | 28 ++++---- 10 files changed, 70 insertions(+), 194 deletions(-) delete mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php delete mode 100644 tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php index acb3bb3e30..d33f68d5e3 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php @@ -8,8 +8,8 @@ use Assert\Assertion; class Expression extends Node { - protected $key; - protected $value; + private $key; + private $value; public function __construct(Key $key, $value) { @@ -20,7 +20,11 @@ class Expression extends Node public function buildQuery(QueryContext $context) { - return $this->key->buildQueryForValue($this->value, $context); + return [ + 'term' => [ + $this->key->getIndexField() => $this->value + ] + ]; } public function getTermNodes() @@ -30,6 +34,6 @@ class Expression extends Node public function __toString() { - return sprintf('<%s:%s>', $this->key, $this->value); + return sprintf('<%s:"%s">', $this->key, $this->value); } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php index d27303a568..934a4b8d47 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php @@ -2,10 +2,8 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; -use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; - interface Key { - public function buildQueryForValue($value, QueryContext $context); + public function getIndexField(); public function __toString(); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php index a156b1068d..bb0e01dd14 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php @@ -5,7 +5,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Assert\Assertion; -class MetadataKey +class MetadataKey implements Key { private $name; @@ -15,8 +15,13 @@ class MetadataKey $this->name = $name; } + public function getIndexField() + { + return sprintf('exif.%s', $this->name); + } + public function __toString() { - return $this->name; + return sprintf('metadata.%s', $this->name); } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php index 2d7271af4c..a1966aac7d 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php @@ -2,9 +2,6 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; -use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; -use Assert\Assertion; - class NativeKey implements Key { const TYPE_DATABASE = 'database'; @@ -41,14 +38,9 @@ class NativeKey implements Key $this->key = $key; } - public function buildQueryForValue($value, QueryContext $context) + public function getIndexField() { - Assertion::string($value); - return [ - 'term' => [ - $this->key => $value - ] - ]; + return $this->key; } public function __toString() diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php deleted file mode 100644 index fe516d6f65..0000000000 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/MetadataMatchStatement.php +++ /dev/null @@ -1,42 +0,0 @@ -key = $key; - $this->value = $value; - } - - public function buildQuery(QueryContext $context) - { - $field = sprintf('exif.%s', $this->key); - return [ - 'term' => [ - $field => $this->value - ] - ]; - } - - public function getTermNodes() - { - return array(); - } - - public function __toString() - { - return sprintf('', $this->key, $this->value); - } -} diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 1e0625e6ce..4b87db3a11 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -85,12 +85,6 @@ class QueryVisitor implements Visit case NodeTypes::FIELD: return new AST\Field($this->visitString($element)); - case NodeTypes::METADATA_STATEMENT: - return $this->visitMetadataStatementNode($element); - - case NodeTypes::METADATA_KEY: - return $this->visitMetadataKeyNode($element); - case NodeTypes::FLAG_STATEMENT: return $this->visitFlagStatementNode($element); @@ -98,11 +92,15 @@ class QueryVisitor implements Visit return new AST\Flag($this->visitString($element)); case NodeTypes::NATIVE_KEY_VALUE: - return $this->visitNativeKeyValueNode($element); + case NodeTypes::METADATA_STATEMENT: + return $this->visitKeyValueNode($element); case NodeTypes::NATIVE_KEY: return $this->visitNativeKeyNode($element); + case NodeTypes::METADATA_KEY: + return $this->visitMetadataKeyNode($element); + default: throw new Exception(sprintf('Unknown node type "%s".', $element->getId())); } @@ -315,17 +313,7 @@ class QueryVisitor implements Visit return new AST\KeyValue\MetadataKey($name); } - private function visitMetadataStatementNode(TreeNode $node) - { - if ($node->getChildrenNumber() !== 2) { - throw new Exception('Flag statement can only have 2 childs.'); - } - $name = $this->visit($node->getChild(0)); - $value = $this->visit($node->getChild(1)); - return new AST\MetadataMatchStatement($name, $value); - } - - private function visitNativeKeyValueNode(TreeNode $node) + private function visitKeyValueNode(TreeNode $node) { if ($node->getChildrenNumber() !== 2) { throw new Exception('Key value expression can only have 2 childs.'); diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php index cc3b5173ae..0a6060a798 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php @@ -4,6 +4,8 @@ namespace Alchemy\Tests\Phrasea\SearchEngine\AST; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Expression as KeyValueExpression; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\MetadataKey; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\NativeKey; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; /** @@ -19,17 +21,28 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase $key = $this->prophesize(Key::class); $key->__toString()->willReturn('foo'); $node = new KeyValueExpression($key->reveal(), 'bar'); - $this->assertEquals('', (string) $node); + $this->assertEquals('', (string) $node); } - public function testQueryBuild() + /** + * @dataProvider keyProvider + */ + public function testQueryBuild($key, $value, $result) { $query_context = $this->prophesize(QueryContext::class); - $key = $this->prophesize(Key::class); - $key->buildQueryForValue('bar', $query_context->reveal())->willReturn('baz'); - - $node = new KeyValueExpression($key->reveal(), 'bar'); + $node = new KeyValueExpression($key, $value); $query = $node->buildQuery($query_context->reveal()); - $this->assertEquals('baz', $query); + $this->assertEquals(json_decode($result, true), $query); + } + + public function keyProvider() + { + return [ + [NativeKey::database(), 'foo', '{"term":{"databox_name": "foo"}}'], + [NativeKey::collection(), 'bar', '{"term":{"collection_name": "bar"}}'], + [NativeKey::mediaType(), 'baz', '{"term":{"type": "baz"}}'], + [NativeKey::recordIdentifier(), 'qux', '{"term":{"record_id": "qux"}}'], + [new MetadataKey('foo'), 'bar', '{"term":{"exif.foo": "bar"}}'], + ]; } } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php deleted file mode 100644 index 439a646b8f..0000000000 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MetadataMatchStatementTest.php +++ /dev/null @@ -1,40 +0,0 @@ -assertTrue(method_exists(MetadataMatchStatement::class, '__toString'), 'Class does not have method __toString'); - $key = new MetadataKey('foo'); - $node = new MetadataMatchStatement($key, 'bar'); - $this->assertEquals('', (string) $node); - } - - public function testQueryBuild() - { - $query_context = $this->prophesize(QueryContext::class); - - $key = new MetadataKey('foo'); - $node = new MetadataMatchStatement($key, 'bar'); - $query = $node->buildQuery($query_context->reveal()); - - $expected = '{ - "term": { - "exif.foo": "bar" - } - }'; - - $this->assertEquals(json_decode($expected, true), $query); - } -} diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/NativeKeyTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/NativeKeyTest.php index 7d8f1de010..e42155a652 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/NativeKeyTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/NativeKeyTest.php @@ -21,63 +21,21 @@ class NativeKeyTest extends \PHPUnit_Framework_TestCase $this->assertEquals('record_identifier', (string) NativeKey::recordIdentifier()); } - public function testDatabaseQuery() + /** + * @dataProvider keyProvider + */ + public function testGetIndexField($key, $field) { - $query_context = $this->prophesize(QueryContext::class); - $key = NativeKey::database(); - $query = $key->buildQueryForValue('bar', $query_context->reveal()); - - $expected = '{ - "term": { - "databox_name": "bar" - } - }'; - - $this->assertEquals(json_decode($expected, true), $query); + $this->assertEquals($key->getIndexField(), $field); } - public function testCollectionQuery() + public function keyProvider() { - $query_context = $this->prophesize(QueryContext::class); - $key = NativeKey::collection(); - $query = $key->buildQueryForValue('bar', $query_context->reveal()); - - $expected = '{ - "term": { - "collection_name": "bar" - } - }'; - - $this->assertEquals(json_decode($expected, true), $query); - } - - public function testMediaTypeQuery() - { - $query_context = $this->prophesize(QueryContext::class); - $key = NativeKey::mediaType(); - $query = $key->buildQueryForValue('bar', $query_context->reveal()); - - $expected = '{ - "term": { - "type": "bar" - } - }'; - - $this->assertEquals(json_decode($expected, true), $query); - } - - public function testRecordIdentifierQuery() - { - $query_context = $this->prophesize(QueryContext::class); - $key = NativeKey::recordIdentifier(); - $query = $key->buildQueryForValue('bar', $query_context->reveal()); - - $expected = '{ - "term": { - "record_id": "bar" - } - }'; - - $this->assertEquals(json_decode($expected, true), $query); + return [ + [NativeKey::database(), 'databox_name'], + [NativeKey::collection(), 'collection_name'], + [NativeKey::mediaType(), 'type'], + [NativeKey::recordIdentifier(), 'record_id'] + ]; } } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv index b29461a7c2..0b6ec954d2 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv @@ -79,19 +79,19 @@ field.type:foo|( MATCHES ) field.id:foo|( MATCHES ) # Matchers -collection:foo| -collection:foo AND bar|( AND ) -collection:foo bar|( AND ) -database:foo| -database:foo AND bar|( AND ) -database:foo bar|( AND ) -type:foo| -type:foo AND bar|( AND ) -type:foo bar|( AND ) -id:90| -id:90 AND foo|( AND ) -id:90 foo|( AND ) -recordid:90| +collection:foo| +collection:foo AND bar|( AND ) +collection:foo bar|( AND ) +database:foo| +database:foo AND bar|( AND ) +database:foo bar|( AND ) +type:foo| +type:foo AND bar|( AND ) +type:foo bar|( AND ) +id:90| +id:90 AND foo|( AND ) +id:90 foo|( AND ) +recordid:90| # Flag matcher flag.foo:true| @@ -103,7 +103,7 @@ flag.foo bar:true|( AND ( MATCHES )) true| # Metadata (EXIF or anything else) matcher -meta.MimeType:image/jpeg| +meta.MimeType:image/jpeg| # Unescaped "." issue on key prefixes fieldOne:foo|( MATCHES ) From 467e530a9372bd1a2323ec2b62274cf30c87f85d Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Thu, 5 Nov 2015 19:43:03 +0100 Subject: [PATCH 06/22] Handle inequality comparisons with technical data. --- grammar/query.pp | 19 +++- .../Elastic/AST/KeyValue/FieldKey.php | 31 +++++++ .../Elastic/AST/KeyValue/MetadataKey.php | 1 - .../Elastic/AST/RangeExpression.php | 84 ++++++++++++----- .../SearchEngine/Elastic/Search/NodeTypes.php | 1 + .../Elastic/Search/QueryVisitor.php | 15 +-- .../SearchEngine/AST/RangeExpressionTest.php | 93 +++++++++++++++++++ .../SearchEngine/resources/queries.csv | 29 +++--- 8 files changed, 228 insertions(+), 45 deletions(-) create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php create mode 100644 tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php diff --git a/grammar/query.pp b/grammar/query.pp index 9511c417fe..7bf7341d2a 100644 --- a/grammar/query.pp +++ b/grammar/query.pp @@ -32,7 +32,7 @@ %token id id|recordid %token field_prefix field\. %token flag_prefix flag\. -%token meta_prefix meta\. +%token meta_prefix (?:meta|exif)\. %token true true|1 %token false false|0 %token word [^\s\(\)\[\]:<>≤≥=]+ @@ -73,10 +73,10 @@ key_value_pair: | ::flag_prefix:: flag() ::colon:: ::space::? boolean() #flag_statement | ::field_prefix:: field() ::colon:: ::space::? term() #field_statement | field() ::colon:: ::space::? term() #field_statement - | field() ::space::? ::lt:: ::space::? value() #less_than - | field() ::space::? ::gt:: ::space::? value() #greater_than - | field() ::space::? ::lte:: ::space::? value() #less_than_or_equal_to - | field() ::space::? ::gte:: ::space::? value() #greater_than_or_equal_to + | key() ::space::? ::lt:: ::space::? value() #less_than + | key() ::space::? ::gt:: ::space::? value() #greater_than + | key() ::space::? ::lte:: ::space::? value() #less_than_or_equal_to + | key() ::space::? ::gte:: ::space::? value() #greater_than_or_equal_to | field() ::space::? ::equal:: ::space::? value() #equal_to #native_key: @@ -85,9 +85,18 @@ key_value_pair: | | +key: + ::meta_prefix:: meta_key() + | ::field_prefix:: field_key() + | field_key() + #meta_key: word_or_keyword()+ +#field_key: + word_or_keyword()+ + | quoted_string() + #flag: word_or_keyword()+ diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php new file mode 100644 index 0000000000..0041781e0b --- /dev/null +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php @@ -0,0 +1,31 @@ +name = $name; + } + + public function getIndexField() + { + return 'yolo'; + } + + public function getValue() + { + return $this->name; + } + + public function __toString() + { + return sprintf('field.%s', $this->name); + } +} diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php index bb0e01dd14..04772a62c2 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php @@ -2,7 +2,6 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; -use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Assert\Assertion; class MetadataKey implements Key diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php index b5e905b3f4..4c324c3c7a 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php @@ -2,41 +2,49 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST; +use Assert\Assertion; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\FieldKey; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; 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 { - private $field; + private $key; + private $field_cache; private $lower_bound; private $lower_inclusive; private $higher_bound; private $higher_inclusive; - public static function lessThan(Field $field, $bound) + public static function lessThan(Key $key, $bound) { - return new self($field, $bound, false); + return new self($key, $bound, false); } - public static function lessThanOrEqual(Field $field, $bound) + public static function lessThanOrEqual(Key $key, $bound) { - return new self($field, $bound, true); + return new self($key, $bound, true); } - public static function greaterThan(Field $field, $bound) + public static function greaterThan(Key $key, $bound) { - return new self($field, null, null, $bound, false); + return new self($key, null, false, $bound, false); } - public static function greaterThanOrEqual(Field $field, $bound) + public static function greaterThanOrEqual(Key $key, $bound) { - return new self($field, null, null, $bound, true); + return new self($key, null, false, $bound, true); } - public function __construct(Field $field, $lb, $li = false, $hb = null, $hi = false) + public function __construct(Key $key, $lb, $li = false, $hb = null, $hi = false) { - $this->field = $field; + Assertion::nullOrScalar($lb); + Assertion::boolean($li); + Assertion::nullOrScalar($hb); + Assertion::boolean($hi); + $this->key = $key; $this->lower_bound = $lb; $this->lower_inclusive = $li; $this->higher_bound = $hb; @@ -45,14 +53,9 @@ class RangeExpression extends Node public function buildQuery(QueryContext $context) { - $structure_field = $context->get($this->field); - if (!$structure_field) { - throw new QueryException(sprintf('Field "%s" does not exist', $this->field->getValue())); - } - $params = array(); if ($this->lower_bound !== null) { - if (!$structure_field->isValueCompatible($this->lower_bound)) { + if (!$this->isValueCompatible($this->lower_bound, $context)) { return; } if ($this->lower_inclusive) { @@ -62,7 +65,7 @@ class RangeExpression extends Node } } if ($this->higher_bound !== null) { - if (!$structure_field->isValueCompatible($this->higher_bound)) { + if (!$this->isValueCompatible($this->higher_bound, $context)) { return; } if ($this->higher_inclusive) { @@ -73,9 +76,48 @@ class RangeExpression extends Node } $query = []; - $query['range'][$structure_field->getIndexField()] = $params; + $query['range'][$this->getIndexField($context)] = $params; - return QueryHelper::wrapPrivateFieldQuery($structure_field, $query); + return $this->postProcessQuery($query, $context); + } + + private function isValueCompatible($value, QueryContext $context) + { + if ($this->key instanceof FieldKey) { + return $this->getField($context)->isValueCompatible($value); + } else { + return true; + } + } + + private function getIndexField(QueryContext $context) + { + if ($this->key instanceof FieldKey) { + return $this->getField($context)->getIndexField(); + } else { + return $this->key->getIndexField(); + } + } + + private function postProcessQuery($query, QueryContext $context) + { + if ($this->key instanceof FieldKey) { + $field = $this->getField($context); + return QueryHelper::wrapPrivateFieldQuery($field, $query); + } else { + return $query; + } + } + + private function getField(QueryContext $context) + { + if ($this->field_cache === null) { + $this->field_cache = $context->get($this->key->getValue()); + } + if ($this->field_cache === null) { + throw new QueryException(sprintf('Field "%s" does not exist', $this->key->getValue())); + } + return $this->field_cache; } public function getTermNodes() @@ -101,6 +143,6 @@ class RangeExpression extends Node } } - return sprintf('', $this->field->getValue(), $string); + return sprintf('', $this->key, $string); } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php index f6a8ae9f76..aba598fcea 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php @@ -17,6 +17,7 @@ class NodeTypes const EQUAL_EXPR = '#equal_to'; const FIELD_STATEMENT = '#field_statement'; const FIELD = '#field'; + const FIELD_KEY = '#field_key'; const VALUE = '#value'; const TERM = '#thesaurus_term'; const TEXT = '#text'; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 4b87db3a11..3089735efc 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -101,6 +101,9 @@ class QueryVisitor implements Visit case NodeTypes::METADATA_KEY: return $this->visitMetadataKeyNode($element); + case NodeTypes::FIELD_KEY: + return new AST\KeyValue\FieldKey($this->visitString($element)); + default: throw new Exception(sprintf('Unknown node type "%s".', $element->getId())); } @@ -151,18 +154,18 @@ class QueryVisitor implements Visit if ($node->getChildrenNumber() !== 2) { throw new Exception('Comparison operator can only have 2 childs.'); } - $field = $node->getChild(0)->accept($this); - $expression = $node->getChild(1)->accept($this); + $key = $node->getChild(0)->accept($this); + $boundary = $node->getChild(1)->accept($this); switch ($node->getId()) { case NodeTypes::LT_EXPR: - return AST\RangeExpression::lessThan($field, $expression); + return AST\RangeExpression::lessThan($key, $boundary); case NodeTypes::LTE_EXPR: - return AST\RangeExpression::lessThanOrEqual($field, $expression); + return AST\RangeExpression::lessThanOrEqual($key, $boundary); case NodeTypes::GT_EXPR: - return AST\RangeExpression::greaterThan($field, $expression); + return AST\RangeExpression::greaterThan($key, $boundary); case NodeTypes::GTE_EXPR: - return AST\RangeExpression::greaterThanOrEqual($field, $expression); + return AST\RangeExpression::greaterThanOrEqual($key, $boundary); } } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php new file mode 100644 index 0000000000..a18e2fd201 --- /dev/null +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php @@ -0,0 +1,93 @@ +assertTrue(method_exists(RangeExpression::class, '__toString'), 'Class does not have method __toString'); + } + + /** + * @dataProvider serializationProvider + */ + public function testSerialization($expression, $serialization) + { + $this->assertEquals($serialization, (string) $expression); + } + + public function serializationProvider() + { + $key_prophecy = $this->prophesize(Key::class); + $key_prophecy->__toString()->willReturn('foo'); + $key = $key_prophecy->reveal(); + return [ + [RangeExpression::lessThan($key, 42), '' ], + [RangeExpression::lessThanOrEqual($key, 42), ''], + [RangeExpression::greaterThan($key, 42), '' ], + [RangeExpression::greaterThanOrEqual($key, 42), ''], + ]; + } + + /** + * @dataProvider queryProvider + */ + public function testQueryBuild($factory, $key, $value, $result) + { + $query_context = $this->prophesize(QueryContext::class); + $node = RangeExpression::$factory($key, $value); + $query = $node->buildQuery($query_context->reveal()); + $this->assertEquals(json_decode($result, true), $query); + } + + public function queryProvider() + { + $key_prophecy = $this->prophesize(Key::class); + $key_prophecy->getIndexField()->willReturn('foo'); + $key = $key_prophecy->reveal(); + return [ + ['lessThan', $key, 'bar', '{"range":{"foo": {"lt":"bar"}}}'], + ['lessThanOrEqual', $key, 'baz', '{"range":{"foo": {"lte":"baz"}}}'], + ['greaterThan', $key, 'qux', '{"range":{"foo": {"gt":"qux"}}}'], + ['greaterThanOrEqual', $key, 'bla', '{"range":{"foo": {"gte":"bla"}}}'], + ]; + } + + public function testQueryBuildWithFieldKey() + { + $key = $this->prophesize(FieldKey::class); + $key->getValue()->willReturn('foo'); + $node = RangeExpression::lessThan($key->reveal(), 'bar'); + $structure_field = $this->prophesize(Field::class); + $structure_field->isPrivate()->willReturn(false); + $structure_field->isValueCompatible('bar')->willReturn(true); + $structure_field->getIndexField()->willReturn('baz'); + $query_context = $this->prophesize(QueryContext::class); + $query_context->get('foo')->willReturn($structure_field->reveal()); + $query = $node->buildQuery($query_context->reveal()); + + $expected = '{ + "range": { + "baz": { + "lt": "bar" + } + } + }'; + + $this->assertEquals(json_decode($expected, true), $query); + } +} diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv index 0b6ec954d2..fa1b37a72b 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv @@ -49,18 +49,18 @@ foo EXCEPT (bar OR baz)|( EXCEPT ( OR )) foo EXCEPT (bar EXCEPT baz)|( EXCEPT ( EXCEPT )) # Comparison operators -foo < 42| -foo ≤ 42| -foo > 42| -foo ≥ 42| -foo < 2015/01/01| -foo ≤ 2015/01/01| -foo > 2015/01/01| -foo ≥ 2015/01/01| -foo < "2015/01/01"| -foo ≤ "2015/01/01"| -foo > "2015/01/01"| -foo ≥ "2015/01/01"| +foo < 42| +foo ≤ 42| +foo > 42| +foo ≥ 42| +foo < 2015/01/01| +foo ≤ 2015/01/01| +foo > 2015/01/01| +foo ≥ 2015/01/01| +foo < "2015/01/01"| +foo ≤ "2015/01/01"| +foo > "2015/01/01"| +foo ≥ "2015/01/01"| foo = 42|( == ) foo = bar|( == ) foo = "bar"|( == ) @@ -104,6 +104,11 @@ true| # Metadata (EXIF or anything else) matcher meta.MimeType:image/jpeg| +exif.MimeType:image/jpeg| +meta.Duration < 300| +meta.Duration ≤ 300| +meta.Duration > 300| +meta.Duration ≥ 300| # Unescaped "." issue on key prefixes fieldOne:foo|( MATCHES ) From 3d7ab6a2922a8c735039091c3f4295eff495ad70 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Fri, 6 Nov 2015 19:10:26 +0100 Subject: [PATCH 07/22] Move field logic from RangeExpression to FieldKey MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Prevent Demeter’s Law violation --- .../Elastic/AST/KeyValue/Expression.php | 2 +- .../Elastic/AST/KeyValue/FieldKey.php | 38 ++++++++++++-- .../SearchEngine/Elastic/AST/KeyValue/Key.php | 5 +- .../Elastic/AST/KeyValue/MetadataKey.php | 8 ++- .../Elastic/AST/KeyValue/NativeKey.php | 9 +++- .../Elastic/AST/RangeExpression.php | 51 +++---------------- .../Elastic/Search/QueryPostProcessor.php | 10 ++++ .../SearchEngine/AST/NativeKeyTest.php | 3 +- .../SearchEngine/AST/RangeExpressionTest.php | 32 ++++++------ 9 files changed, 88 insertions(+), 70 deletions(-) create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryPostProcessor.php diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php index d33f68d5e3..f90dc5acbe 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php @@ -22,7 +22,7 @@ class Expression extends Node { return [ 'term' => [ - $this->key->getIndexField() => $this->value + $this->key->getIndexField($context) => $this->value ] ]; } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php index 0041781e0b..84ebed4bbf 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php @@ -2,11 +2,16 @@ 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\Search\QueryHelper; +use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryPostProcessor; use Assert\Assertion; -class FieldKey implements Key +class FieldKey implements Key, QueryPostProcessor { private $name; + private $field_cache = []; public function __construct($name) { @@ -14,14 +19,37 @@ class FieldKey implements Key $this->name = $name; } - public function getIndexField() + public function getIndexField(QueryContext $context, ...$other) { - return 'yolo'; + return $this->getField($context)->getIndexField(...$other); } - public function getValue() + public function isValueCompatible($value, QueryContext $context) { - return $this->name; + return $this->getField($context)->isValueCompatible($value); + } + + public function postProcessQuery($query, QueryContext $context) + { + $field = $this->getField($context); + return QueryHelper::wrapPrivateFieldQuery($field, $query); + } + + private function getField(QueryContext $context) + { + $hash = spl_object_hash($context); + if (!isset($this->field_cache[$hash])) { + $this->field_cache[$hash] = $context->get($this->name); + } + if ($field = $this->field_cache[$hash] === null) { + throw new QueryException(sprintf('Field "%s" does not exist', $this->name)); + } + return $field; + } + + public function clearCache() + { + $this->field_cache = []; } public function __toString() diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php index 934a4b8d47..07ba6cbb44 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php @@ -2,8 +2,11 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; +use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; + interface Key { - public function getIndexField(); + public function getIndexField(QueryContext $context); + public function isValueCompatible($value, QueryContext $context); public function __toString(); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php index 04772a62c2..6889fd2356 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php @@ -2,6 +2,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; +use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Assert\Assertion; class MetadataKey implements Key @@ -14,11 +15,16 @@ class MetadataKey implements Key $this->name = $name; } - public function getIndexField() + public function getIndexField(QueryContext $context) { return sprintf('exif.%s', $this->name); } + public function isValueCompatible($value, QueryContext $context) + { + return true; + } + public function __toString() { return sprintf('metadata.%s', $this->name); diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php index a1966aac7d..afffe47d21 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php @@ -2,6 +2,8 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; +use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; + class NativeKey implements Key { const TYPE_DATABASE = 'database'; @@ -38,11 +40,16 @@ class NativeKey implements Key $this->key = $key; } - public function getIndexField() + public function getIndexField(QueryContext $context) { return $this->key; } + public function isValueCompatible($value, QueryContext $context) + { + return true; + } + public function __toString() { return $this->type; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php index 4c324c3c7a..f9ebba9e53 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RangeExpression.php @@ -5,14 +5,12 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST; use Assert\Assertion; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\FieldKey; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; -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 { private $key; - private $field_cache; private $lower_bound; private $lower_inclusive; private $higher_bound; @@ -55,7 +53,7 @@ class RangeExpression extends Node { $params = array(); if ($this->lower_bound !== null) { - if (!$this->isValueCompatible($this->lower_bound, $context)) { + if (!$this->key->isValueCompatible($this->lower_bound, $context)) { return; } if ($this->lower_inclusive) { @@ -65,7 +63,7 @@ class RangeExpression extends Node } } if ($this->higher_bound !== null) { - if (!$this->isValueCompatible($this->higher_bound, $context)) { + if (!$this->key->isValueCompatible($this->higher_bound, $context)) { return; } if ($this->higher_inclusive) { @@ -76,48 +74,13 @@ class RangeExpression extends Node } $query = []; - $query['range'][$this->getIndexField($context)] = $params; + $query['range'][$this->key->getIndexField($context)] = $params; - return $this->postProcessQuery($query, $context); - } + if ($this->key instanceof QueryPostProcessor) { + return $this->key->postProcessQuery($query, $context); + } - private function isValueCompatible($value, QueryContext $context) - { - if ($this->key instanceof FieldKey) { - return $this->getField($context)->isValueCompatible($value); - } else { - return true; - } - } - - private function getIndexField(QueryContext $context) - { - if ($this->key instanceof FieldKey) { - return $this->getField($context)->getIndexField(); - } else { - return $this->key->getIndexField(); - } - } - - private function postProcessQuery($query, QueryContext $context) - { - if ($this->key instanceof FieldKey) { - $field = $this->getField($context); - return QueryHelper::wrapPrivateFieldQuery($field, $query); - } else { - return $query; - } - } - - private function getField(QueryContext $context) - { - if ($this->field_cache === null) { - $this->field_cache = $context->get($this->key->getValue()); - } - if ($this->field_cache === null) { - throw new QueryException(sprintf('Field "%s" does not exist', $this->key->getValue())); - } - return $this->field_cache; + return $query; } public function getTermNodes() diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryPostProcessor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryPostProcessor.php new file mode 100644 index 0000000000..0a2dfc505e --- /dev/null +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryPostProcessor.php @@ -0,0 +1,10 @@ +assertEquals($key->getIndexField(), $field); + $query_context = $this->prophesize(QueryContext::class); + $this->assertEquals($key->getIndexField($query_context->reveal()), $field); } public function keyProvider() diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php index a18e2fd201..4aa666e769 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php @@ -9,6 +9,7 @@ use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\NativeKey; use Alchemy\Phrasea\SearchEngine\Elastic\AST\RangeExpression; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field; +use Prophecy\Argument; /** * @group unit @@ -46,39 +47,38 @@ class RangeExpressionTest extends \PHPUnit_Framework_TestCase /** * @dataProvider queryProvider */ - public function testQueryBuild($factory, $key, $value, $result) + public function testQueryBuild($factory, $query_context, $key, $value, $result) { - $query_context = $this->prophesize(QueryContext::class); $node = RangeExpression::$factory($key, $value); - $query = $node->buildQuery($query_context->reveal()); + $query = $node->buildQuery($query_context); $this->assertEquals(json_decode($result, true), $query); } public function queryProvider() { + $query_context = $this->prophesize(QueryContext::class)->reveal(); $key_prophecy = $this->prophesize(Key::class); - $key_prophecy->getIndexField()->willReturn('foo'); + $key_prophecy->getIndexField($query_context)->willReturn('foo'); + $key_prophecy->isValueCompatible('bar', $query_context)->willReturn(true); $key = $key_prophecy->reveal(); return [ - ['lessThan', $key, 'bar', '{"range":{"foo": {"lt":"bar"}}}'], - ['lessThanOrEqual', $key, 'baz', '{"range":{"foo": {"lte":"baz"}}}'], - ['greaterThan', $key, 'qux', '{"range":{"foo": {"gt":"qux"}}}'], - ['greaterThanOrEqual', $key, 'bla', '{"range":{"foo": {"gte":"bla"}}}'], + ['lessThan', $query_context, $key, 'bar', '{"range":{"foo": {"lt":"bar"}}}'], + ['lessThanOrEqual', $query_context, $key, 'bar', '{"range":{"foo": {"lte":"bar"}}}'], + ['greaterThan', $query_context, $key, 'bar', '{"range":{"foo": {"gt":"bar"}}}'], + ['greaterThanOrEqual', $query_context, $key, 'bar', '{"range":{"foo": {"gte":"bar"}}}'], ]; } public function testQueryBuildWithFieldKey() { + $query_context = $this->prophesize(QueryContext::class)->reveal(); $key = $this->prophesize(FieldKey::class); - $key->getValue()->willReturn('foo'); + $key->getIndexField($query_context)->willReturn('baz'); + $key->isValueCompatible('bar', $query_context)->willReturn(true); + $key->postProcessQuery(Argument::any(), $query_context)->willReturnArgument(0); + $node = RangeExpression::lessThan($key->reveal(), 'bar'); - $structure_field = $this->prophesize(Field::class); - $structure_field->isPrivate()->willReturn(false); - $structure_field->isValueCompatible('bar')->willReturn(true); - $structure_field->getIndexField()->willReturn('baz'); - $query_context = $this->prophesize(QueryContext::class); - $query_context->get('foo')->willReturn($structure_field->reveal()); - $query = $node->buildQuery($query_context->reveal()); + $query = $node->buildQuery($query_context); $expected = '{ "range": { From 77b9faaa4eecb39be98dca3278352b36f7e0d8e3 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Fri, 6 Nov 2015 19:34:09 +0100 Subject: [PATCH 08/22] Handle equality test on metadata Still need some work on testing side --- grammar/query.pp | 10 ++-- .../Elastic/AST/FieldEqualsExpression.php | 25 +++++----- .../Elastic/AST/KeyValue/FieldKey.php | 7 +-- .../AST/FieldEqualsExpressionTest.php | 46 +++++++++---------- .../SearchEngine/resources/queries.csv | 7 +-- 5 files changed, 48 insertions(+), 47 deletions(-) diff --git a/grammar/query.pp b/grammar/query.pp index 7bf7341d2a..098ae65816 100644 --- a/grammar/query.pp +++ b/grammar/query.pp @@ -73,11 +73,11 @@ key_value_pair: | ::flag_prefix:: flag() ::colon:: ::space::? boolean() #flag_statement | ::field_prefix:: field() ::colon:: ::space::? term() #field_statement | field() ::colon:: ::space::? term() #field_statement - | key() ::space::? ::lt:: ::space::? value() #less_than - | key() ::space::? ::gt:: ::space::? value() #greater_than - | key() ::space::? ::lte:: ::space::? value() #less_than_or_equal_to - | key() ::space::? ::gte:: ::space::? value() #greater_than_or_equal_to - | field() ::space::? ::equal:: ::space::? value() #equal_to + | key() ::space::? ::lt:: ::space::? value() #less_than + | key() ::space::? ::gt:: ::space::? value() #greater_than + | key() ::space::? ::lte:: ::space::? value() #less_than_or_equal_to + | key() ::space::? ::gte:: ::space::? value() #greater_than_or_equal_to + | key() ::space::? ::equal:: ::space::? value() #equal_to #native_key: diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php index 8a4b93ca0c..e514fcf312 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php @@ -2,38 +2,41 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\FieldKey; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; 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; class FieldEqualsExpression extends Node { - private $field; + private $key; private $value; - public function __construct(Field $field, $value) + public function __construct(Key $key, $value) { - $this->field = $field; + $this->key = $key; $this->value = $value; } public function buildQuery(QueryContext $context) { - $structure_field = $context->get($this->field); - if (!$structure_field) { - throw new QueryException(sprintf('Field "%s" does not exist', $this->field->getValue())); - } - if (!$structure_field->isValueCompatible($this->value)) { + if (!$this->key->isValueCompatible($this->value, $context)) { return null; } $query = [ 'term' => [ - $structure_field->getIndexField(true) => $this->value + $this->key->getIndexField($context, true) => $this->value ] ]; - return QueryHelper::wrapPrivateFieldQuery($structure_field, $query); + if ($this->key instanceof QueryPostProcessor) { + return $this->key->postProcessQuery($query, $context); + } + + return $query; } public function getTermNodes() @@ -43,6 +46,6 @@ class FieldEqualsExpression extends Node public function __toString() { - return sprintf('(%s == )', $this->field, $this->value); + return sprintf('(<%s> == )', $this->key, $this->value); } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php index 84ebed4bbf..e238b41cb1 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php @@ -19,9 +19,9 @@ class FieldKey implements Key, QueryPostProcessor $this->name = $name; } - public function getIndexField(QueryContext $context, ...$other) + public function getIndexField(QueryContext $context, $raw = false) { - return $this->getField($context)->getIndexField(...$other); + return $this->getField($context)->getIndexField($raw); } public function isValueCompatible($value, QueryContext $context) @@ -41,7 +41,8 @@ class FieldKey implements Key, QueryPostProcessor if (!isset($this->field_cache[$hash])) { $this->field_cache[$hash] = $context->get($this->name); } - if ($field = $this->field_cache[$hash] === null) { + $field = $this->field_cache[$hash]; + if ($field === null) { throw new QueryException(sprintf('Field "%s" does not exist', $this->name)); } return $field; diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php index 99dbd0fb6c..d686efffe3 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php @@ -2,8 +2,8 @@ namespace Alchemy\Tests\Phrasea\SearchEngine\AST; -use Alchemy\Phrasea\SearchEngine\Elastic\AST\Field as ASTField; use Alchemy\Phrasea\SearchEngine\Elastic\AST\FieldEqualsExpression; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field as StructureField; @@ -17,10 +17,10 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase public function testSerialization() { $this->assertTrue(method_exists(FieldEqualsExpression::class, '__toString'), 'Class does not have method __toString'); - $field = $this->prophesize(ASTField::class); - $field->__toString()->willReturn('foo'); - $node = new FieldEqualsExpression($field->reveal(), 'bar'); - $this->assertEquals('(foo == )', (string) $node); + $key = $this->prophesize(Key::class); + $key->__toString()->willReturn('foo'); + $node = new FieldEqualsExpression($key->reveal(), 'bar'); + $this->assertEquals('( == )', (string) $node); } /** @@ -28,20 +28,15 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase */ public function testQueryBuild($index_field, $value, $compatible_value, $private, $expected_json) { - $structure_field = $this->prophesize(StructureField::class); - $structure_field->isValueCompatible($value)->willReturn($compatible_value); - $structure_field->getIndexField(true)->willReturn($index_field); - $structure_field->isPrivate()->willReturn($private); - if ($private) { - $structure_field->getDependantCollections()->willReturn(['baz', 'qux']); - } + $query_context = $this->prophesize(QueryContext::class)->reveal(); - $ast_field = $this->prophesize(ASTField::class); - $query_context = $this->prophesize(QueryContext::class); - $query_context->get($ast_field->reveal())->willReturn($structure_field); + $key = $this->prophesize(Key::class); + $key->isValueCompatible($value, $query_context)->willReturn($compatible_value); + $key->getIndexField($query_context, true)->willReturn($index_field); + // TODO Test keys implementing QueryPostProcessor - $node = new FieldEqualsExpression($ast_field->reveal(), 'bar'); - $query = $node->buildQuery($query_context->reveal()); + $node = new FieldEqualsExpression($key->reveal(), 'bar'); + $query = $node->buildQuery($query_context); $this->assertEquals(json_decode($expected_json, true), $query); } @@ -49,14 +44,15 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase public function queryProvider() { return [ - ['foo.raw', 'bar', true, true, '{ - "filtered": { - "filter": { - "terms": { - "base_id": ["baz","qux"] } }, - "query": { - "term": { - "foo.raw": "bar" } } } }'], + // TODO Put this case in another test case + // ['foo.raw', 'bar', true, true, '{ + // "filtered": { + // "filter": { + // "terms": { + // "base_id": ["baz","qux"] } }, + // "query": { + // "term": { + // "foo.raw": "bar" } } } }'], ['foo.raw', 'bar', true, false, '{ "term": { "foo.raw": "bar" } }'], diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv index fa1b37a72b..53e39de0d9 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/resources/queries.csv @@ -61,9 +61,9 @@ foo < "2015/01/01"| foo ≤ "2015/01/01"| foo > "2015/01/01"| foo ≥ "2015/01/01"| -foo = 42|( == ) -foo = bar|( == ) -foo = "bar"|( == ) +foo = 42|( == ) +foo = bar|( == ) +foo = "bar"|( == ) # Field narrowing foo:bar|( MATCHES ) @@ -109,6 +109,7 @@ meta.Duration < 300| meta.Duration ≤ 300| meta.Duration > 300| meta.Duration ≥ 300| +meta.Duration = 300|( == ) # Unescaped "." issue on key prefixes fieldOne:foo|( MATCHES ) From 9d3dfa56533534b734447f092e41c8912655eb00 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Mon, 9 Nov 2015 19:03:55 +0100 Subject: [PATCH 09/22] Manage metadata tags from structure --- .../Elastic/AST/KeyValue/MetadataKey.php | 27 ++++++++-- .../Elastic/Search/QueryContext.php | 5 ++ .../Elastic/Structure/GlobalStructure.php | 20 ++++++- .../Elastic/Structure/LimitedStructure.php | 10 ++++ .../Elastic/Structure/MetadataHelper.php | 49 +++++++++++++++++ .../Elastic/Structure/Structure.php | 3 ++ .../SearchEngine/Elastic/Structure/Tag.php | 52 +++++++++++++++++++ .../AST/KeyValueExpressionTest.php | 15 +++++- 8 files changed, 174 insertions(+), 7 deletions(-) create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/MetadataHelper.php create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Tag.php diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php index 6889fd2356..81309dcae7 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php @@ -2,12 +2,14 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue; +use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Assert\Assertion; class MetadataKey implements Key { private $name; + private $tag_cache = []; public function __construct($name) { @@ -15,16 +17,35 @@ class MetadataKey implements Key $this->name = $name; } - public function getIndexField(QueryContext $context) + public function getIndexField(QueryContext $context, $raw = false) { - return sprintf('exif.%s', $this->name); + return $this->getTag($context)->getIndexField($raw); } public function isValueCompatible($value, QueryContext $context) { - return true; + return $this->getTag($context)->isValueCompatible($value); } + private function getTag(QueryContext $context) + { + $hash = spl_object_hash($context); + if (!isset($this->tag_cache[$hash])) { + $this->tag_cache[$hash] = $context->getMetadataTag($this->name); + } + $tag = $this->tag_cache[$hash]; + if ($tag === null) { + throw new QueryException(sprintf('Metadata tag "%s" does not exist', $this->name)); + } + return $tag; + } + + public function clearCache() + { + $this->tag_cache = []; + } + + public function __toString() { return sprintf('metadata.%s', $this->name); diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php index a9362624db..3ba553353d 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryContext.php @@ -85,6 +85,11 @@ class QueryContext return $this->structure->getFlagByName($name); } + public function getMetadataTag($name) + { + return $this->structure->getMetadataTagByName($name); + } + /** * @todo Maybe we should put this logic in Field class? */ diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/GlobalStructure.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/GlobalStructure.php index 451536fea6..ab9d6cf5e4 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/GlobalStructure.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/GlobalStructure.php @@ -19,6 +19,7 @@ final class GlobalStructure implements Structure /** @var Field[] */ private $facets = array(); private $flags = array(); + private $metadata_tags = array(); /** * @param \databox[] $databoxes @@ -36,19 +37,23 @@ final class GlobalStructure implements Structure $flags[] = Flag::createFromLegacyStatus($status); } } - return new self($fields, $flags); + return new self($fields, $flags, MetadataHelper::createTags()); } - public function __construct(array $fields = [], array $flags = []) + public function __construct(array $fields = [], array $flags = [], array $metadata_tags = []) { Assertion::allIsInstanceOf($fields, Field::class); Assertion::allIsInstanceOf($flags, Flag::class); + Assertion::allIsInstanceOf($metadata_tags, Tag::class); foreach ($fields as $field) { $this->add($field); } foreach ($flags as $flag) { $this->flags[$flag->getName()] = $flag; } + foreach ($metadata_tags as $tag) { + $this->metadata_tags[$tag->getName()] = $tag; + } } public function add(Field $field) @@ -148,6 +153,17 @@ final class GlobalStructure implements Structure $this->flags[$name] : null; } + public function getMetadataTags() + { + return $this->metadata_tags; + } + + public function getMetadataTagByName($name) + { + return isset($this->metadata_tags[$name]) ? + $this->metadata_tags[$name] : null; + } + /** * Returns an array of collections indexed by field name. * diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/LimitedStructure.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/LimitedStructure.php index aa187102d5..affb103db4 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/LimitedStructure.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/LimitedStructure.php @@ -85,6 +85,16 @@ final class LimitedStructure implements Structure return $this->structure->getFlagByName($name); } + public function getMetadataTags() + { + return $this->structure->getMetadataTags(); + } + + public function getMetadataTagByName($name) + { + return $this->structure->getMetadataTagByName($name); + } + private function limit(array $fields) { $allowed_collections = $this->allowedCollections(); diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/MetadataHelper.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/MetadataHelper.php new file mode 100644 index 0000000000..0e6f481c6f --- /dev/null +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/MetadataHelper.php @@ -0,0 +1,49 @@ +name = $name; + $this->type = $type; + $this->analyzable = $analyzable; + } + + public function getName() + { + return $this->name; + } + + public function getType() + { + return $this->type; + } + + public function isAnalyzable() + { + return $this->analyzable; + } + + public function isValueCompatible() + { + return true; + } + + public function getIndexField($raw = false) + { + return sprintf( + 'exif.%s%s', + $this->name, + $raw && $this->type === Mapping::TYPE_STRING ? '.raw' : '' + ); + } +} diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php index 0a6060a798..784ecd8d19 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php @@ -24,10 +24,22 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase $this->assertEquals('', (string) $node); } + public function testQueryBuild() + { + $query_context = $this->prophesize(QueryContext::class)->reveal(); + $key = $this->prophesize(Key::class); + $key->getIndexField($query_context)->willReturn('foo'); + $node = new KeyValueExpression($key->reveal(), 'bar'); + $query = $node->buildQuery($query_context); + + $result = '{"term":{"foo": "bar"}}'; + $this->assertEquals(json_decode($result, true), $query); + } + /** * @dataProvider keyProvider */ - public function testQueryBuild($key, $value, $result) + public function testNativeQueryBuild($key, $value, $result) { $query_context = $this->prophesize(QueryContext::class); $node = new KeyValueExpression($key, $value); @@ -42,7 +54,6 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase [NativeKey::collection(), 'bar', '{"term":{"collection_name": "bar"}}'], [NativeKey::mediaType(), 'baz', '{"term":{"type": "baz"}}'], [NativeKey::recordIdentifier(), 'qux', '{"term":{"record_id": "qux"}}'], - [new MetadataKey('foo'), 'bar', '{"term":{"exif.foo": "bar"}}'], ]; } } From 5784b158a2f365f25e84bb956765065a43b589ce Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Mon, 9 Nov 2015 19:06:09 +0100 Subject: [PATCH 10/22] Extend Key interface to support raw index fields --- lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php | 2 +- .../Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php index 07ba6cbb44..20ee045a59 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Key.php @@ -6,7 +6,7 @@ use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; interface Key { - public function getIndexField(QueryContext $context); + public function getIndexField(QueryContext $context, $raw = false); public function isValueCompatible($value, QueryContext $context); public function __toString(); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php index afffe47d21..4c25df08e6 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/NativeKey.php @@ -40,7 +40,7 @@ class NativeKey implements Key $this->key = $key; } - public function getIndexField(QueryContext $context) + public function getIndexField(QueryContext $context, $raw = false) { return $this->key; } From aff2819f82ef049c72d316fa064b0148b9690020 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Mon, 9 Nov 2015 19:06:55 +0100 Subject: [PATCH 11/22] Dynamically create metadata tags mapping --- .../Elastic/Indexer/RecordIndexer.php | 43 ++++++------------- 1 file changed, 13 insertions(+), 30 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/RecordIndexer.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/RecordIndexer.php index 50e2830ed7..601dd1cf10 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/RecordIndexer.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/RecordIndexer.php @@ -36,7 +36,6 @@ use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus; use Alchemy\Phrasea\SearchEngine\Elastic\Thesaurus\CandidateTerms; use databox; use Iterator; -use media_subdef; use Psr\Log\LoggerInterface; class RecordIndexer @@ -308,7 +307,7 @@ class RecordIndexer // Thesaurus ->add('concept_path', $this->getThesaurusPathMapping()) // EXIF - ->add('exif', $this->getExifMapping()) + ->add('exif', $this->getMetadataTagMapping()) // Status ->add('flags', $this->getFlagsMapping()) ->add('flags_bitfield', 'integer')->notIndexed() @@ -375,36 +374,20 @@ class RecordIndexer return $mapping; } - // @todo Add call to addAnalyzedVersion ? - private function getExifMapping() + private function getMetadataTagMapping() { $mapping = new Mapping(); - $mapping - ->add(media_subdef::TC_DATA_WIDTH, 'integer') - ->add(media_subdef::TC_DATA_HEIGHT, 'integer') - ->add(media_subdef::TC_DATA_COLORSPACE, 'string')->notAnalyzed() - ->add(media_subdef::TC_DATA_CHANNELS, 'integer') - ->add(media_subdef::TC_DATA_ORIENTATION, 'integer') - ->add(media_subdef::TC_DATA_COLORDEPTH, 'integer') - ->add(media_subdef::TC_DATA_DURATION, 'float') - ->add(media_subdef::TC_DATA_AUDIOCODEC, 'string')->notAnalyzed() - ->add(media_subdef::TC_DATA_AUDIOSAMPLERATE, 'float') - ->add(media_subdef::TC_DATA_VIDEOCODEC, 'string')->notAnalyzed() - ->add(media_subdef::TC_DATA_FRAMERATE, 'float') - ->add(media_subdef::TC_DATA_MIMETYPE, 'string')->notAnalyzed() - ->add(media_subdef::TC_DATA_FILESIZE, 'long') - // TODO use geo point type for lat/long - ->add(media_subdef::TC_DATA_LONGITUDE, 'float') - ->add(media_subdef::TC_DATA_LATITUDE, 'float') - ->add(media_subdef::TC_DATA_FOCALLENGTH, 'float') - ->add(media_subdef::TC_DATA_CAMERAMODEL, 'string') - ->add(media_subdef::TC_DATA_FLASHFIRED, 'boolean') - ->add(media_subdef::TC_DATA_APERTURE, 'float') - ->add(media_subdef::TC_DATA_SHUTTERSPEED, 'float') - ->add(media_subdef::TC_DATA_HYPERFOCALDISTANCE, 'float') - ->add(media_subdef::TC_DATA_ISO, 'integer') - ->add(media_subdef::TC_DATA_LIGHTVALUE, 'float') - ; + foreach ($this->structure->getMetadataTags() as $tag) { + $type = $tag->getType(); + $mapping->add($tag->getName(), $type); + if ($type === Mapping::TYPE_STRING) { + if ($tag->isAnalyzable()) { + $mapping->addRawVersion(); + } else { + $mapping->notAnalyzed(); + } + } + } return $mapping; } From 0bb61bd882d00997608a46b70fbd595b91842341 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Mon, 9 Nov 2015 19:07:31 +0100 Subject: [PATCH 12/22] Remove metadata tag validation from query compiler --- .../Elastic/Search/QueryHelper.php | 31 ------------------- .../Elastic/Search/QueryVisitor.php | 13 +------- 2 files changed, 1 insertion(+), 43 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php index 95d30fc685..9d652f86c7 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryHelper.php @@ -3,7 +3,6 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Search; use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field; -use media_subdef; class QueryHelper { @@ -108,34 +107,4 @@ class QueryHelper return $query; } } - - public static function isValidMetadataName($name) - { - return in_array($name, [ - // To keep in sync with RecordIndexer::getExifMapping() - media_subdef::TC_DATA_WIDTH, - media_subdef::TC_DATA_HEIGHT, - media_subdef::TC_DATA_COLORSPACE, - media_subdef::TC_DATA_CHANNELS, - media_subdef::TC_DATA_ORIENTATION, - media_subdef::TC_DATA_COLORDEPTH, - media_subdef::TC_DATA_DURATION, - media_subdef::TC_DATA_AUDIOCODEC, - media_subdef::TC_DATA_AUDIOSAMPLERATE, - media_subdef::TC_DATA_VIDEOCODEC, - media_subdef::TC_DATA_FRAMERATE, - media_subdef::TC_DATA_MIMETYPE, - media_subdef::TC_DATA_FILESIZE, - media_subdef::TC_DATA_LONGITUDE, - media_subdef::TC_DATA_LATITUDE, - media_subdef::TC_DATA_FOCALLENGTH, - media_subdef::TC_DATA_CAMERAMODEL, - media_subdef::TC_DATA_FLASHFIRED, - media_subdef::TC_DATA_APERTURE, - media_subdef::TC_DATA_SHUTTERSPEED, - media_subdef::TC_DATA_HYPERFOCALDISTANCE, - media_subdef::TC_DATA_ISO, - media_subdef::TC_DATA_LIGHTVALUE - ]); - } } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 3089735efc..d6a743ee22 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -4,8 +4,6 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\Search; use Alchemy\Phrasea\SearchEngine\Elastic\AST; use Alchemy\Phrasea\SearchEngine\Elastic\Exception\Exception; -use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException; -use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper; use Hoa\Compiler\Llk\TreeNode; use Hoa\Visitor\Element; use Hoa\Visitor\Visit; @@ -99,7 +97,7 @@ class QueryVisitor implements Visit return $this->visitNativeKeyNode($element); case NodeTypes::METADATA_KEY: - return $this->visitMetadataKeyNode($element); + return new AST\KeyValue\MetadataKey($this->visitString($element)); case NodeTypes::FIELD_KEY: return new AST\KeyValue\FieldKey($this->visitString($element)); @@ -307,15 +305,6 @@ class QueryVisitor implements Visit } } - private function visitMetadataKeyNode(TreeNode $node) - { - $name = $this->visitString($node); - if (!QueryHelper::isValidMetadataName($name)) { - throw new QueryException(sprintf('"%s" is not a valid metadata name', $name)); - } - return new AST\KeyValue\MetadataKey($name); - } - private function visitKeyValueNode(TreeNode $node) { if ($node->getChildrenNumber() !== 2) { From e953998ca7a6936eb5f7504236e72102f98f1de9 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Mon, 9 Nov 2015 19:08:40 +0100 Subject: [PATCH 13/22] Use analysis on key-value expressions (with colon operator) --- .../SearchEngine/Elastic/AST/KeyValue/Expression.php | 2 +- .../SearchEngine/AST/KeyValueExpressionTest.php | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php index f90dc5acbe..3c58bfb048 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/Expression.php @@ -21,7 +21,7 @@ class Expression extends Node public function buildQuery(QueryContext $context) { return [ - 'term' => [ + 'match' => [ $this->key->getIndexField($context) => $this->value ] ]; diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php index 784ecd8d19..429feddd6a 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php @@ -32,7 +32,7 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase $node = new KeyValueExpression($key->reveal(), 'bar'); $query = $node->buildQuery($query_context); - $result = '{"term":{"foo": "bar"}}'; + $result = '{"match":{"foo": "bar"}}'; $this->assertEquals(json_decode($result, true), $query); } @@ -50,10 +50,10 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase public function keyProvider() { return [ - [NativeKey::database(), 'foo', '{"term":{"databox_name": "foo"}}'], - [NativeKey::collection(), 'bar', '{"term":{"collection_name": "bar"}}'], - [NativeKey::mediaType(), 'baz', '{"term":{"type": "baz"}}'], - [NativeKey::recordIdentifier(), 'qux', '{"term":{"record_id": "qux"}}'], + [NativeKey::database(), 'foo', '{"match":{"databox_name": "foo"}}'], + [NativeKey::collection(), 'bar', '{"match":{"collection_name": "bar"}}'], + [NativeKey::mediaType(), 'baz', '{"match":{"type": "baz"}}'], + [NativeKey::recordIdentifier(), 'qux', '{"match":{"record_id": "qux"}}'], ]; } } From 54cc0132b76fa70dcd88623f8e175b9530a66e6d Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 11:51:57 +0100 Subject: [PATCH 14/22] Cast metadata tags values according to type --- .../Record/Hydrator/MetadataHydrator.php | 46 ++++++++++++------- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php index 54759bdd0c..e3b94e626d 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php @@ -76,23 +76,7 @@ SQL; case 'caption': // Sanitize fields $value = StringHelper::crlfNormalize($value); - switch ($this->structure->typeOf($key)) { - case Mapping::TYPE_DATE: - $value = $this->helper->sanitizeDate($value); - break; - - case Mapping::TYPE_FLOAT: - case Mapping::TYPE_DOUBLE: - $value = (float) $value; - break; - - case Mapping::TYPE_INTEGER: - case Mapping::TYPE_LONG: - case Mapping::TYPE_SHORT: - case Mapping::TYPE_BYTE: - $value = (int) $value; - break; - } + $value = $this->sanitizeValue($value, $this->structure->typeOf($key)); // Private caption fields are kept apart $type = $metadata['private'] ? 'private_caption' : 'caption'; // Caption are multi-valued @@ -110,6 +94,10 @@ SQL; case 'exif': // EXIF data is single-valued + $tag = $this->structure->getMetadataTagByName($key); + if ($tag) { + $value = $this->sanitizeValue($value, $tag->getType()); + } $record['exif'][$key] = $value; break; @@ -119,4 +107,28 @@ SQL; } } } + + private function sanitizeValue($value, $type) + { + switch ($type) { + case Mapping::TYPE_DATE: + return $this->helper->sanitizeDate($value); + + case Mapping::TYPE_FLOAT: + case Mapping::TYPE_DOUBLE: + return (float) $value; + + case Mapping::TYPE_INTEGER: + case Mapping::TYPE_LONG: + case Mapping::TYPE_SHORT: + case Mapping::TYPE_BYTE: + return (int) $value; + + case Mapping::TYPE_BOOLEAN: + return (bool) $value; + + default: + return $value; + } + } } From c31ad07de9c53386c0e7583ea9c431fdff25343b Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 12:29:14 +0100 Subject: [PATCH 15/22] Do not filter out falsy boolean values --- .../Elastic/Indexer/Record/Hydrator/MetadataHydrator.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php index e3b94e626d..34fe519718 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Indexer/Record/Hydrator/MetadataHydrator.php @@ -61,7 +61,7 @@ SQL; $value = $metadata['value']; // Do not keep empty values - if (empty($key) || empty($value)) { + if ($key === '' || $value === '') { continue; } From 407470ab10be157f1dc91b7f7fea844e695d2669 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 12:58:44 +0100 Subject: [PATCH 16/22] Rename key-value expression classes --- .../EqualExpression.php} | 5 +++-- .../KeyValue/{Expression.php => MatchExpression.php} | 2 +- .../Elastic/AST/{ => KeyValue}/RangeExpression.php | 3 ++- .../SearchEngine/Elastic/Search/QueryVisitor.php | 12 ++++++------ ...alsExpressionTest.php => EqualExpressionTest.php} | 8 ++++---- ...lueExpressionTest.php => MatchExpressionTest.php} | 12 ++++++------ .../Phrasea/SearchEngine/AST/RangeExpressionTest.php | 2 +- 7 files changed, 23 insertions(+), 21 deletions(-) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/{FieldEqualsExpression.php => KeyValue/EqualExpression.php} (89%) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/{Expression.php => MatchExpression.php} (95%) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/{ => KeyValue}/RangeExpression.php (96%) rename tests/Alchemy/Tests/Phrasea/SearchEngine/AST/{FieldEqualsExpressionTest.php => EqualExpressionTest.php} (85%) rename tests/Alchemy/Tests/Phrasea/SearchEngine/AST/{KeyValueExpressionTest.php => MatchExpressionTest.php} (79%) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php similarity index 89% rename from lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php rename to lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php index e514fcf312..6559bf5af4 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/FieldEqualsExpression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php @@ -1,15 +1,16 @@ getId()) { case NodeTypes::LT_EXPR: - return AST\RangeExpression::lessThan($key, $boundary); + return AST\KeyValue\RangeExpression::lessThan($key, $boundary); case NodeTypes::LTE_EXPR: - return AST\RangeExpression::lessThanOrEqual($key, $boundary); + return AST\KeyValue\RangeExpression::lessThanOrEqual($key, $boundary); case NodeTypes::GT_EXPR: - return AST\RangeExpression::greaterThan($key, $boundary); + return AST\KeyValue\RangeExpression::greaterThan($key, $boundary); case NodeTypes::GTE_EXPR: - return AST\RangeExpression::greaterThanOrEqual($key, $boundary); + return AST\KeyValue\RangeExpression::greaterThanOrEqual($key, $boundary); } } @@ -184,7 +184,7 @@ class QueryVisitor implements Visit throw new Exception('Equality operator can only have 2 childs.'); } - return new AST\FieldEqualsExpression( + return new AST\KeyValue\EqualExpression( $node->getChild(0)->accept($this), $node->getChild(1)->accept($this) ); @@ -312,7 +312,7 @@ class QueryVisitor implements Visit } $key = $this->visit($node->getChild(0)); $value = $this->visit($node->getChild(1)); - return new AST\KeyValue\Expression($key, $value); + return new AST\KeyValue\MatchExpression($key, $value); } private function visitNativeKeyNode(Element $element) diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php similarity index 85% rename from tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php rename to tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php index d686efffe3..677f2a5880 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/FieldEqualsExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php @@ -2,7 +2,7 @@ namespace Alchemy\Tests\Phrasea\SearchEngine\AST; -use Alchemy\Phrasea\SearchEngine\Elastic\AST\FieldEqualsExpression; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\EqualExpression; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field as StructureField; @@ -16,10 +16,10 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase { public function testSerialization() { - $this->assertTrue(method_exists(FieldEqualsExpression::class, '__toString'), 'Class does not have method __toString'); + $this->assertTrue(method_exists(EqualExpression::class, '__toString'), 'Class does not have method __toString'); $key = $this->prophesize(Key::class); $key->__toString()->willReturn('foo'); - $node = new FieldEqualsExpression($key->reveal(), 'bar'); + $node = new EqualExpression($key->reveal(), 'bar'); $this->assertEquals('( == )', (string) $node); } @@ -35,7 +35,7 @@ class FieldEqualsExpressionTest extends \PHPUnit_Framework_TestCase $key->getIndexField($query_context, true)->willReturn($index_field); // TODO Test keys implementing QueryPostProcessor - $node = new FieldEqualsExpression($key->reveal(), 'bar'); + $node = new EqualExpression($key->reveal(), 'bar'); $query = $node->buildQuery($query_context); $this->assertEquals(json_decode($expected_json, true), $query); diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php similarity index 79% rename from tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php rename to tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php index 429feddd6a..273a2a1b27 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/KeyValueExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php @@ -3,7 +3,7 @@ namespace Alchemy\Tests\Phrasea\SearchEngine\AST; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; -use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Expression as KeyValueExpression; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\MatchExpression; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\MetadataKey; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\NativeKey; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; @@ -13,14 +13,14 @@ use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; * @group searchengine * @group ast */ -class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase +class MatchExpressionTest extends \PHPUnit_Framework_TestCase { public function testSerialization() { - $this->assertTrue(method_exists(KeyValueExpression::class, '__toString'), 'Class does not have method __toString'); + $this->assertTrue(method_exists(MatchExpression::class, '__toString'), 'Class does not have method __toString'); $key = $this->prophesize(Key::class); $key->__toString()->willReturn('foo'); - $node = new KeyValueExpression($key->reveal(), 'bar'); + $node = new MatchExpression($key->reveal(), 'bar'); $this->assertEquals('', (string) $node); } @@ -29,7 +29,7 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase $query_context = $this->prophesize(QueryContext::class)->reveal(); $key = $this->prophesize(Key::class); $key->getIndexField($query_context)->willReturn('foo'); - $node = new KeyValueExpression($key->reveal(), 'bar'); + $node = new MatchExpression($key->reveal(), 'bar'); $query = $node->buildQuery($query_context); $result = '{"match":{"foo": "bar"}}'; @@ -42,7 +42,7 @@ class KeyValueExpressionTest extends \PHPUnit_Framework_TestCase public function testNativeQueryBuild($key, $value, $result) { $query_context = $this->prophesize(QueryContext::class); - $node = new KeyValueExpression($key, $value); + $node = new MatchExpression($key, $value); $query = $node->buildQuery($query_context->reveal()); $this->assertEquals(json_decode($result, true), $query); } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php index 4aa666e769..88a850fe03 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RangeExpressionTest.php @@ -6,7 +6,7 @@ use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\FieldKey; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\MetadataKey; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\NativeKey; -use Alchemy\Phrasea\SearchEngine\Elastic\AST\RangeExpression; +use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\RangeExpression; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Alchemy\Phrasea\SearchEngine\Elastic\Structure\Field; use Prophecy\Argument; From 9635efc1e35abeff4cf2504f0d3947e7c19e0850 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 13:11:45 +0100 Subject: [PATCH 17/22] Refactor children count assertions --- .../Elastic/Search/QueryVisitor.php | 36 ++++++++----------- 1 file changed, 14 insertions(+), 22 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index e7cd7706cd..14c7e53614 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -118,9 +118,7 @@ class QueryVisitor implements Visit private function visitFieldStatementNode(TreeNode $node) { - if ($node->getChildrenNumber() !== 2) { - throw new Exception('Field statement must have 2 childs.'); - } + $this->assertChildrenCount($node, 2); $field = $this->visit($node->getChild(0)); $value = $this->visit($node->getChild(1)); return new AST\FieldMatchExpression($field, $value); @@ -149,9 +147,7 @@ class QueryVisitor implements Visit private function visitRangeNode(TreeNode $node) { - if ($node->getChildrenNumber() !== 2) { - throw new Exception('Comparison operator can only have 2 childs.'); - } + $this->assertChildrenCount($node, 2); $key = $node->getChild(0)->accept($this); $boundary = $node->getChild(1)->accept($this); @@ -169,9 +165,7 @@ class QueryVisitor implements Visit private function handleBinaryOperator(Element $element, \Closure $factory) { - if ($element->getChildrenNumber() !== 2) { - throw new Exception('Binary expression can only have 2 childs.'); - } + $this->assertChildrenCount($element, 2); $left = $element->getChild(0)->accept($this); $right = $element->getChild(1)->accept($this); @@ -180,10 +174,7 @@ class QueryVisitor implements Visit private function visitEqualNode(TreeNode $node) { - if ($node->getChildrenNumber() !== 2) { - throw new Exception('Equality operator can only have 2 childs.'); - } - + $this->assertChildrenCount($node, 2); return new AST\KeyValue\EqualExpression( $node->getChild(0)->accept($this), $node->getChild(1)->accept($this) @@ -274,9 +265,7 @@ class QueryVisitor implements Visit private function visitFlagStatementNode(TreeNode $node) { - if ($node->getChildrenNumber() !== 2) { - throw new Exception('Flag statement can only have 2 childs.'); - } + $this->assertChildrenCount($node, 2); $flag = $node->getChild(0)->accept($this); if (!$flag instanceof AST\Flag) { throw new \Exception('Flag statement key must be a flag node.'); @@ -307,9 +296,7 @@ class QueryVisitor implements Visit private function visitKeyValueNode(TreeNode $node) { - if ($node->getChildrenNumber() !== 2) { - throw new Exception('Key value expression can only have 2 childs.'); - } + $this->assertChildrenCount($node, 2); $key = $this->visit($node->getChild(0)); $value = $this->visit($node->getChild(1)); return new AST\KeyValue\MatchExpression($key, $value); @@ -317,9 +304,7 @@ class QueryVisitor implements Visit private function visitNativeKeyNode(Element $element) { - if ($element->getChildrenNumber() !== 1) { - throw new Exception('Native key node can only have a single child.'); - } + $this->assertChildrenCount($element, 1); $type = $element->getChild(0)->getValue()['token']; switch ($type) { case NodeTypes::TOKEN_DATABASE: @@ -334,4 +319,11 @@ class QueryVisitor implements Visit throw new InvalidArgumentException(sprintf('Unexpected token type "%s" for native key.', $type)); } } + + private function assertChildrenCount(TreeNode $node, $count) + { + if ($node->getChildrenNumber() !== $count) { + throw new Exception(sprintf('Node was expected to have only %s children.', $count)); + } + } } From 7353945d3cfb1b54555b09859500c0ef67300f00 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 13:19:50 +0100 Subject: [PATCH 18/22] Refactor binary expression handling --- .../Elastic/Search/QueryVisitor.php | 30 ++++++++----------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 14c7e53614..9ba5d7ebf5 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -118,29 +118,28 @@ class QueryVisitor implements Visit private function visitFieldStatementNode(TreeNode $node) { - $this->assertChildrenCount($node, 2); - $field = $this->visit($node->getChild(0)); - $value = $this->visit($node->getChild(1)); - return new AST\FieldMatchExpression($field, $value); + return $this->handleBinaryExpression($node, function($left, $right) { + return new AST\FieldMatchExpression($left, $right); + }); } private function visitAndNode(Element $element) { - return $this->handleBinaryOperator($element, function($left, $right) { + return $this->handleBinaryExpression($element, function($left, $right) { return new AST\Boolean\AndOperator($left, $right); }); } private function visitOrNode(Element $element) { - return $this->handleBinaryOperator($element, function($left, $right) { + return $this->handleBinaryExpression($element, function($left, $right) { return new AST\Boolean\OrOperator($left, $right); }); } private function visitExceptNode(Element $element) { - return $this->handleBinaryOperator($element, function($left, $right) { + return $this->handleBinaryExpression($element, function($left, $right) { return new AST\Boolean\ExceptOperator($left, $right); }); } @@ -163,7 +162,7 @@ class QueryVisitor implements Visit } } - private function handleBinaryOperator(Element $element, \Closure $factory) + private function handleBinaryExpression(Element $element, \Closure $factory) { $this->assertChildrenCount($element, 2); $left = $element->getChild(0)->accept($this); @@ -174,11 +173,9 @@ class QueryVisitor implements Visit private function visitEqualNode(TreeNode $node) { - $this->assertChildrenCount($node, 2); - return new AST\KeyValue\EqualExpression( - $node->getChild(0)->accept($this), - $node->getChild(1)->accept($this) - ); + return $this->handleBinaryExpression($node, function($left, $right) { + return new AST\KeyValue\EqualExpression($left, $right); + }); } private function visitTerm(Element $element) @@ -296,10 +293,9 @@ class QueryVisitor implements Visit private function visitKeyValueNode(TreeNode $node) { - $this->assertChildrenCount($node, 2); - $key = $this->visit($node->getChild(0)); - $value = $this->visit($node->getChild(1)); - return new AST\KeyValue\MatchExpression($key, $value); + return $this->handleBinaryExpression($node, function($left, $right) { + return new AST\KeyValue\MatchExpression($left, $right); + }); } private function visitNativeKeyNode(Element $element) From 9f923e0409e1d85eeaa03504fdd6a404b2df0db6 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 13:35:12 +0100 Subject: [PATCH 19/22] Refactor AST for match expression --- grammar/query.pp | 7 +++++-- .../Phrasea/SearchEngine/Elastic/Search/NodeTypes.php | 3 +-- .../Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php | 7 +++---- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/grammar/query.pp b/grammar/query.pp index 098ae65816..9c71566e7d 100644 --- a/grammar/query.pp +++ b/grammar/query.pp @@ -68,8 +68,7 @@ quaternary: // Key value pairs & field level matchers (restricted to a single field) key_value_pair: - native_key() ::colon:: ::space::? value() #native_key_value - | ::meta_prefix:: meta_key() ::colon:: ::space::? value() #meta_statement + match_key() ::colon:: ::space::? value() #match_expression | ::flag_prefix:: flag() ::colon:: ::space::? boolean() #flag_statement | ::field_prefix:: field() ::colon:: ::space::? term() #field_statement | field() ::colon:: ::space::? term() #field_statement @@ -79,6 +78,10 @@ key_value_pair: | key() ::space::? ::gte:: ::space::? value() #greater_than_or_equal_to | key() ::space::? ::equal:: ::space::? value() #equal_to +match_key: + native_key() + | ::meta_prefix:: meta_key() + #native_key: | diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php index aba598fcea..637d5db5b5 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/NodeTypes.php @@ -15,6 +15,7 @@ class NodeTypes const LTE_EXPR = '#less_than_or_equal_to'; const GTE_EXPR = '#greater_than_or_equal_to'; const EQUAL_EXPR = '#equal_to'; + const MATCH_EXPR = '#match_expression'; const FIELD_STATEMENT = '#field_statement'; const FIELD = '#field'; const FIELD_KEY = '#field_key'; @@ -22,11 +23,9 @@ class NodeTypes const TERM = '#thesaurus_term'; const TEXT = '#text'; const CONTEXT = '#context'; - const METADATA_STATEMENT = '#meta_statement'; const METADATA_KEY = '#meta_key'; const FLAG_STATEMENT = '#flag_statement'; const FLAG = '#flag'; - const NATIVE_KEY_VALUE = '#native_key_value'; const NATIVE_KEY = '#native_key'; // Token types for leaf nodes const TOKEN_WORD = 'word'; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 9ba5d7ebf5..2efbc0ed2e 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -89,9 +89,8 @@ class QueryVisitor implements Visit case NodeTypes::FLAG: return new AST\Flag($this->visitString($element)); - case NodeTypes::NATIVE_KEY_VALUE: - case NodeTypes::METADATA_STATEMENT: - return $this->visitKeyValueNode($element); + case NodeTypes::MATCH_EXPR: + return $this->visitMatchExpressionNode($element); case NodeTypes::NATIVE_KEY: return $this->visitNativeKeyNode($element); @@ -291,7 +290,7 @@ class QueryVisitor implements Visit } } - private function visitKeyValueNode(TreeNode $node) + private function visitMatchExpressionNode(TreeNode $node) { return $this->handleBinaryExpression($node, function($left, $right) { return new AST\KeyValue\MatchExpression($left, $right); From c89d48c6f797a121cf4d1343b38cf2b8fbb7f8c6 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 14:56:50 +0100 Subject: [PATCH 20/22] =?UTF-8?q?BooleanOperator=20=E2=86=92=20BooleanExpr?= =?UTF-8?q?ession?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../AST/Boolean/{AndOperator.php => AndExpression.php} | 2 +- .../Boolean/{BinaryOperator.php => BinaryExpression.php} | 2 +- .../Boolean/{ExceptOperator.php => ExceptExpression.php} | 2 +- .../AST/Boolean/{OrOperator.php => OrExpression.php} | 2 +- .../Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php | 8 ++++---- 5 files changed, 8 insertions(+), 8 deletions(-) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/{AndOperator.php => AndExpression.php} (90%) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/{BinaryOperator.php => BinaryExpression.php} (93%) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/{ExceptOperator.php => ExceptExpression.php} (90%) rename lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/{OrOperator.php => OrExpression.php} (91%) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/AndOperator.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/AndExpression.php similarity index 90% rename from lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/AndOperator.php rename to lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/AndExpression.php index 763710a8f0..99602a879f 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/AndOperator.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/AndExpression.php @@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\Boolean; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; -class AndOperator extends BinaryOperator +class AndExpression extends BinaryExpression { protected $operator = 'AND'; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/BinaryOperator.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/BinaryExpression.php similarity index 93% rename from lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/BinaryOperator.php rename to lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/BinaryExpression.php index 34a0acc37b..afcb1f1578 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/BinaryOperator.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/BinaryExpression.php @@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\Boolean; use Alchemy\Phrasea\SearchEngine\Elastic\AST\Node; -abstract class BinaryOperator extends Node +abstract class BinaryExpression extends Node { protected $left; protected $right; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/ExceptOperator.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/ExceptExpression.php similarity index 90% rename from lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/ExceptOperator.php rename to lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/ExceptExpression.php index 8e07bad56f..e0305d5a5e 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/ExceptOperator.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/ExceptExpression.php @@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\Boolean; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; -class ExceptOperator extends BinaryOperator +class ExceptExpression extends BinaryExpression { protected $operator = 'EXCEPT'; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/OrOperator.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/OrExpression.php similarity index 91% rename from lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/OrOperator.php rename to lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/OrExpression.php index f716e0178a..304294c7f8 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/OrOperator.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/Boolean/OrExpression.php @@ -4,7 +4,7 @@ namespace Alchemy\Phrasea\SearchEngine\Elastic\AST\Boolean; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; -class OrOperator extends BinaryOperator +class OrExpression extends BinaryExpression { protected $operator = 'OR'; diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php index 2efbc0ed2e..2e4df86afc 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Search/QueryVisitor.php @@ -125,21 +125,21 @@ class QueryVisitor implements Visit private function visitAndNode(Element $element) { return $this->handleBinaryExpression($element, function($left, $right) { - return new AST\Boolean\AndOperator($left, $right); + return new AST\Boolean\AndExpression($left, $right); }); } private function visitOrNode(Element $element) { return $this->handleBinaryExpression($element, function($left, $right) { - return new AST\Boolean\OrOperator($left, $right); + return new AST\Boolean\OrExpression($left, $right); }); } private function visitExceptNode(Element $element) { return $this->handleBinaryExpression($element, function($left, $right) { - return new AST\Boolean\ExceptOperator($left, $right); + return new AST\Boolean\ExceptExpression($left, $right); }); } @@ -236,7 +236,7 @@ class QueryVisitor implements Visit throw new Exception('Unexpected context after non-contextualizable node'); } } elseif ($node instanceof AST\Node) { - $root = new AST\Boolean\AndOperator($root, $node); + $root = new AST\Boolean\AndExpression($root, $node); } else { throw new Exception('Unexpected node type inside text node.'); } From 8e5d09bd769cfda6810db615cb6ee88d5d9f0746 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 16:26:34 +0100 Subject: [PATCH 21/22] Test value compatibility before creating ES query --- .../Elastic/AST/KeyValue/EqualExpression.php | 2 +- .../Elastic/AST/KeyValue/FieldKey.php | 3 +- .../Elastic/AST/KeyValue/MatchExpression.php | 5 ++ .../Elastic/AST/KeyValue/MetadataKey.php | 3 +- .../Elastic/AST/QuotedTextNode.php | 6 +- .../SearchEngine/Elastic/AST/RawNode.php | 6 +- .../SearchEngine/Elastic/AST/TextNode.php | 4 +- .../SearchEngine/Elastic/Structure/Field.php | 38 +-------- .../SearchEngine/Elastic/Structure/Tag.php | 7 +- .../SearchEngine/Elastic/Structure/Typed.php | 13 +++ .../Elastic/Structure/ValueChecker.php | 48 +++++++++++ .../SearchEngine/AST/EqualExpressionTest.php | 21 ++++- .../SearchEngine/AST/MatchExpressionTest.php | 1 + .../Phrasea/SearchEngine/AST/RawNodeTest.php | 3 + .../Structure/ValueCheckerTest.php | 80 +++++++++++++++++++ 15 files changed, 183 insertions(+), 57 deletions(-) create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Typed.php create mode 100644 lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/ValueChecker.php create mode 100644 tests/Alchemy/Tests/Phrasea/SearchEngine/Structure/ValueCheckerTest.php diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php index 6559bf5af4..827147a1e0 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/EqualExpression.php +++ b/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 = [ diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php index e238b41cb1..89b6376e31 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/FieldKey.php +++ b/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) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MatchExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MatchExpression.php index 9142e62a2f..89043ca310 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MatchExpression.php +++ b/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 diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php index 81309dcae7..292b6e7237 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/MetadataKey.php +++ b/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) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php index fb7ac29409..8d2d5fd9ad 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/QuotedTextNode.php +++ b/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); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php index ac387f0371..784fadcbfa 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/RawNode.php +++ b/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); } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php index 8727b6e227..49b8d49da9 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/TextNode.php +++ b/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; } diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Field.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Field.php index 39060fd81f..06d8730c52 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Field.php +++ b/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); diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Tag.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Tag.php index 91f0d6a1e0..0a4895832e 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Tag.php +++ b/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( diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Typed.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Typed.php new file mode 100644 index 0000000000..bd50d70d07 --- /dev/null +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/Structure/Typed.php @@ -0,0 +1,13 @@ + 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; + } +} diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php index 677f2a5880..6670197fe8 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/EqualExpressionTest.php +++ b/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); + } } diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php index 273a2a1b27..bcb93de7c4 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/MatchExpressionTest.php +++ b/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); diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RawNodeTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RawNodeTest.php index e734e2b01b..4c02032741 100644 --- a/tests/Alchemy/Tests/Phrasea/SearchEngine/AST/RawNodeTest.php +++ b/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); diff --git a/tests/Alchemy/Tests/Phrasea/SearchEngine/Structure/ValueCheckerTest.php b/tests/Alchemy/Tests/Phrasea/SearchEngine/Structure/ValueCheckerTest.php new file mode 100644 index 0000000000..f6ef923329 --- /dev/null +++ b/tests/Alchemy/Tests/Phrasea/SearchEngine/Structure/ValueCheckerTest.php @@ -0,0 +1,80 @@ +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(); + } +} From b8c3ea91c8de8245623c19e7504b939cc47b0af0 Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Tue, 10 Nov 2015 17:08:41 +0100 Subject: [PATCH 22/22] Make range expression throw on invalid value --- .../Elastic/AST/KeyValue/RangeExpression.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/RangeExpression.php b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/RangeExpression.php index ea4066248b..c81b7631b8 100644 --- a/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/RangeExpression.php +++ b/lib/Alchemy/Phrasea/SearchEngine/Elastic/AST/KeyValue/RangeExpression.php @@ -6,6 +6,7 @@ use Assert\Assertion; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\FieldKey; use Alchemy\Phrasea\SearchEngine\Elastic\AST\KeyValue\Key; use Alchemy\Phrasea\SearchEngine\Elastic\AST\Node; +use Alchemy\Phrasea\SearchEngine\Elastic\Exception\QueryException; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryContext; use Alchemy\Phrasea\SearchEngine\Elastic\Search\QueryHelper; @@ -54,9 +55,7 @@ class RangeExpression extends Node { $params = array(); if ($this->lower_bound !== null) { - if (!$this->key->isValueCompatible($this->lower_bound, $context)) { - return; - } + $this->assertValueCompatible($this->lower_bound, $context); if ($this->lower_inclusive) { $params['lte'] = $this->lower_bound; } else { @@ -64,9 +63,7 @@ class RangeExpression extends Node } } if ($this->higher_bound !== null) { - if (!$this->key->isValueCompatible($this->higher_bound, $context)) { - return; - } + $this->assertValueCompatible($this->higher_bound, $context); if ($this->higher_inclusive) { $params['gte'] = $this->higher_bound; } else { @@ -84,6 +81,13 @@ class RangeExpression extends Node return $query; } + private function assertValueCompatible($value, QueryContext $context) + { + if (!$this->key->isValueCompatible($value, $context)) { + throw new QueryException(sprintf('Value "%s" for metadata tag "%s" is not valid.', $value, $this->key)); + } + } + public function getTermNodes() { return array();