From 713442c7b487cdcec4409ec308536cd34b95dbcf Mon Sep 17 00:00:00 2001 From: Mathieu Darse Date: Thu, 5 Nov 2015 16:52:01 +0100 Subject: [PATCH] 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 )