From 0a526be3440406e2a1e6bad4b86f4e93f5467029 Mon Sep 17 00:00:00 2001 From: javanna Date: Tue, 16 Jun 2015 10:41:57 +0200 Subject: [PATCH] Query DSL: fix support for _name in some queries Some of our Java api builders had wrong logic when it comes to serializing the query in json format, resulting in missing fields like _name. Also, regexp parser was ignoring the _name field. Closes #11694 --- .../index/query/FuzzyQueryBuilder.java | 50 ++++---- .../index/query/PrefixQueryBuilder.java | 2 +- .../index/query/RegexpQueryBuilder.java | 38 +++--- .../index/query/RegexpQueryParser.java | 3 +- .../index/query/SpanFirstQueryBuilder.java | 2 +- .../index/query/WildcardQueryBuilder.java | 2 +- .../matchedqueries/MatchedQueriesTests.java | 111 ++++++++++++++++++ 7 files changed, 156 insertions(+), 52 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java index d1143f9a5ce..ce17e66d361 100644 --- a/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/FuzzyQueryBuilder.java @@ -105,34 +105,30 @@ public class FuzzyQueryBuilder extends MultiTermQueryBuilder implements Boostabl @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(FuzzyQueryParser.NAME); - if (boost == -1 && fuzziness == null && prefixLength == null && queryName != null) { - builder.field(name, value); - } else { - builder.startObject(name); - builder.field("value", value); - if (boost != -1) { - builder.field("boost", boost); - } - if (transpositions != null) { - builder.field("transpositions", transpositions); - } - if (fuzziness != null) { - fuzziness.toXContent(builder, params); - } - if (prefixLength != null) { - builder.field("prefix_length", prefixLength); - } - if (maxExpansions != null) { - builder.field("max_expansions", maxExpansions); - } - if (rewrite != null) { - builder.field("rewrite", rewrite); - } - if (queryName != null) { - builder.field("_name", queryName); - } - builder.endObject(); + builder.startObject(name); + builder.field("value", value); + if (boost != -1) { + builder.field("boost", boost); } + if (transpositions != null) { + builder.field("transpositions", transpositions); + } + if (fuzziness != null) { + fuzziness.toXContent(builder, params); + } + if (prefixLength != null) { + builder.field("prefix_length", prefixLength); + } + if (maxExpansions != null) { + builder.field("max_expansions", maxExpansions); + } + if (rewrite != null) { + builder.field("rewrite", rewrite); + } + if (queryName != null) { + builder.field("_name", queryName); + } + builder.endObject(); builder.endObject(); } } \ No newline at end of file diff --git a/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java index c890d60ee7c..e0e5b2f243f 100644 --- a/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/PrefixQueryBuilder.java @@ -75,7 +75,7 @@ public class PrefixQueryBuilder extends MultiTermQueryBuilder implements Boostab @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(PrefixQueryParser.NAME); - if (boost == -1 && rewrite == null && queryName != null) { + if (boost == -1 && rewrite == null && queryName == null) { builder.field(name, prefix); } else { builder.startObject(name); diff --git a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java index 43b5b2f77ef..fcb41717aa8 100644 --- a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryBuilder.java @@ -98,28 +98,24 @@ public class RegexpQueryBuilder extends MultiTermQueryBuilder implements Boostab @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(RegexpQueryParser.NAME); - if (boost == -1 && rewrite == null && queryName != null) { - builder.field(name, regexp); - } else { - builder.startObject(name); - builder.field("value", regexp); - if (flags != -1) { - builder.field("flags_value", flags); - } - if (maxDetermizedStatesSet) { - builder.field("max_determinized_states", maxDeterminizedStates); - } - if (boost != -1) { - builder.field("boost", boost); - } - if (rewrite != null) { - builder.field("rewrite", rewrite); - } - if (queryName != null) { - builder.field("name", queryName); - } - builder.endObject(); + builder.startObject(name); + builder.field("value", regexp); + if (flags != -1) { + builder.field("flags_value", flags); } + if (maxDetermizedStatesSet) { + builder.field("max_determinized_states", maxDeterminizedStates); + } + if (boost != -1) { + builder.field("boost", boost); + } + if (rewrite != null) { + builder.field("rewrite", rewrite); + } + if (queryName != null) { + builder.field("_name", queryName); + } + builder.endObject(); builder.endObject(); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryParser.java b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryParser.java index 652ac68ba73..7be5b798eac 100644 --- a/core/src/main/java/org/elasticsearch/index/query/RegexpQueryParser.java +++ b/core/src/main/java/org/elasticsearch/index/query/RegexpQueryParser.java @@ -27,7 +27,6 @@ import org.apache.lucene.util.automaton.Operations; import org.elasticsearch.common.inject.Inject; import org.elasticsearch.common.lucene.BytesRefs; import org.elasticsearch.common.xcontent.XContentParser; -import org.elasticsearch.index.mapper.FieldMapper; import org.elasticsearch.index.mapper.MappedFieldType; import org.elasticsearch.index.query.support.QueryParsers; @@ -87,6 +86,8 @@ public class RegexpQueryParser implements QueryParser { maxDeterminizedStates = parser.intValue(); } else if ("flags_value".equals(currentFieldName)) { flagsValue = parser.intValue(); + } else if ("_name".equals(currentFieldName)) { + queryName = parser.text(); } else { throw new QueryParsingException(parseContext, "[regexp] query does not support [" + currentFieldName + "]"); } diff --git a/core/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java index 2e2c1fd5e74..f967a1c1c07 100644 --- a/core/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/SpanFirstQueryBuilder.java @@ -62,7 +62,7 @@ public class SpanFirstQueryBuilder extends SpanQueryBuilder implements Boostable builder.field("boost", boost); } if (queryName != null) { - builder.field("name", queryName); + builder.field("_name", queryName); } builder.endObject(); } diff --git a/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java index 002d408dd77..654f14ee509 100644 --- a/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java @@ -85,7 +85,7 @@ public class WildcardQueryBuilder extends MultiTermQueryBuilder implements Boost @Override public void doXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(WildcardQueryParser.NAME); - if (boost == -1 && rewrite == null && queryName != null) { + if (boost == -1 && rewrite == null && queryName == null) { builder.field(name, wildcard); } else { builder.startObject(name); diff --git a/core/src/test/java/org/elasticsearch/search/matchedqueries/MatchedQueriesTests.java b/core/src/test/java/org/elasticsearch/search/matchedqueries/MatchedQueriesTests.java index b079b2f6e6b..6b0a7a74ee6 100644 --- a/core/src/test/java/org/elasticsearch/search/matchedqueries/MatchedQueriesTests.java +++ b/core/src/test/java/org/elasticsearch/search/matchedqueries/MatchedQueriesTests.java @@ -21,6 +21,7 @@ package org.elasticsearch.search.matchedqueries; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.SearchHit; import org.elasticsearch.test.ElasticsearchIntegrationTest; import org.junit.Test; @@ -204,6 +205,116 @@ public class MatchedQueriesTests extends ElasticsearchIntegrationTest { } } + @Test + public void testRegExpQuerySupportsName() { + createIndex("test1"); + ensureGreen(); + + client().prepareIndex("test1", "type1", "1").setSource("title", "title1").get(); + refresh(); + + SearchResponse searchResponse = client().prepareSearch() + .setQuery(QueryBuilders.regexpQuery("title", "title1").queryName("regex")).get(); + assertHitCount(searchResponse, 1l); + + for (SearchHit hit : searchResponse.getHits()) { + if (hit.id().equals("1")) { + assertThat(hit.matchedQueries().length, equalTo(1)); + assertThat(hit.matchedQueries(), hasItemInArray("regex")); + } else { + fail("Unexpected document returned with id " + hit.id()); + } + } + } + + @Test + public void testPrefixQuerySupportsName() { + createIndex("test1"); + ensureGreen(); + + client().prepareIndex("test1", "type1", "1").setSource("title", "title1").get(); + refresh(); + + SearchResponse searchResponse = client().prepareSearch() + .setQuery(QueryBuilders.prefixQuery("title", "title").queryName("prefix")).get(); + assertHitCount(searchResponse, 1l); + + for (SearchHit hit : searchResponse.getHits()) { + if (hit.id().equals("1")) { + assertThat(hit.matchedQueries().length, equalTo(1)); + assertThat(hit.matchedQueries(), hasItemInArray("prefix")); + } else { + fail("Unexpected document returned with id " + hit.id()); + } + } + } + + @Test + public void testFuzzyQuerySupportsName() { + createIndex("test1"); + ensureGreen(); + + client().prepareIndex("test1", "type1", "1").setSource("title", "title1").get(); + refresh(); + + SearchResponse searchResponse = client().prepareSearch() + .setQuery(QueryBuilders.fuzzyQuery("title", "titel1").queryName("fuzzy")).get(); + assertHitCount(searchResponse, 1l); + + for (SearchHit hit : searchResponse.getHits()) { + if (hit.id().equals("1")) { + assertThat(hit.matchedQueries().length, equalTo(1)); + assertThat(hit.matchedQueries(), hasItemInArray("fuzzy")); + } else { + fail("Unexpected document returned with id " + hit.id()); + } + } + } + + @Test + public void testWildcardQuerySupportsName() { + createIndex("test1"); + ensureGreen(); + + client().prepareIndex("test1", "type1", "1").setSource("title", "title1").get(); + refresh(); + + SearchResponse searchResponse = client().prepareSearch() + .setQuery(QueryBuilders.wildcardQuery("title", "titl*").queryName("wildcard")).get(); + assertHitCount(searchResponse, 1l); + + for (SearchHit hit : searchResponse.getHits()) { + if (hit.id().equals("1")) { + assertThat(hit.matchedQueries().length, equalTo(1)); + assertThat(hit.matchedQueries(), hasItemInArray("wildcard")); + } else { + fail("Unexpected document returned with id " + hit.id()); + } + } + } + + @Test + public void testSpanFirstQuerySupportsName() { + createIndex("test1"); + ensureGreen(); + + client().prepareIndex("test1", "type1", "1").setSource("title", "title1 title2").get(); + refresh(); + + SearchResponse searchResponse = client().prepareSearch() + .setQuery(QueryBuilders.spanFirstQuery(QueryBuilders.spanTermQuery("title", "title1"), 10).queryName("span")).get(); + assertHitCount(searchResponse, 1l); + + for (SearchHit hit : searchResponse.getHits()) { + if (hit.id().equals("1")) { + assertThat(hit.matchedQueries().length, equalTo(1)); + assertThat(hit.matchedQueries(), hasItemInArray("span")); + } else { + fail("Unexpected document returned with id " + hit.id()); + } + } + } + /** * Test case for issue #4361: https://github.com/elasticsearch/elasticsearch/issues/4361 */