From 55c8e805320c1ab51288396e005cffaf14adc008 Mon Sep 17 00:00:00 2001 From: Colin Goodheart-Smithe Date: Fri, 6 Apr 2018 11:45:34 +0100 Subject: [PATCH] Fixes query_string query equals timezone check (#29406) * Fixes query_string query equals timezone check This change fixes a bug where two `QueryStringQueryBuilder`s were found to be equal if they had the same timezone set even if the query string in the builders were different Closes #29403 * Adds mutate function to QueryStringQueryBuilderTests * iter --- .../index/query/QueryStringQueryBuilder.java | 26 ++- .../query/QueryStringQueryBuilderTests.java | 212 ++++++++++++++++++ .../test/AbstractQueryTestCase.java | 10 +- 3 files changed, 240 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java index 4ce8aae52c1..3920b730d7a 100644 --- a/server/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java @@ -42,7 +42,6 @@ import org.joda.time.DateTimeZone; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Locale; import java.util.Map; @@ -358,7 +357,7 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder fields = instance.fields(); + Operator operator = instance.defaultOperator(); + Fuzziness fuzziness = instance.fuzziness(); + String analyzer = instance.analyzer(); + String quoteAnalyzer = instance.quoteAnalyzer(); + Boolean allowLeadingWildCard = instance.allowLeadingWildcard(); + Boolean analyzeWildcard = instance.analyzeWildcard(); + int maxDeterminizedStates = instance.maxDeterminizedStates(); + boolean enablePositionIncrements = instance.enablePositionIncrements(); + boolean escape = instance.escape(); + int phraseSlop = instance.phraseSlop(); + int fuzzyMaxExpansions = instance.fuzzyMaxExpansions(); + int fuzzyPrefixLength = instance.fuzzyPrefixLength(); + String fuzzyRewrite = instance.fuzzyRewrite(); + String rewrite = instance.rewrite(); + String quoteFieldSuffix = instance.quoteFieldSuffix(); + Float tieBreaker = instance.tieBreaker(); + String minimumShouldMatch = instance.minimumShouldMatch(); + String timeZone = instance.timeZone() == null ? null : instance.timeZone().getID(); + boolean autoGenerateSynonymsPhraseQuery = instance.autoGenerateSynonymsPhraseQuery(); + boolean fuzzyTranspositions = instance.fuzzyTranspositions(); + + switch (between(0, 23)) { + case 0: + query = query + " foo"; + break; + case 1: + if (defaultField == null) { + defaultField = randomAlphaOfLengthBetween(1, 10); + } else { + defaultField = defaultField + randomAlphaOfLength(5); + } + break; + case 2: + fields = new HashMap<>(fields); + fields.put(randomAlphaOfLength(10), 1.0f); + break; + case 3: + operator = randomValueOtherThan(operator, () -> randomFrom(Operator.values())); + break; + case 4: + fuzziness = randomValueOtherThan(fuzziness, () -> randomFrom(Fuzziness.AUTO, Fuzziness.ZERO, Fuzziness.ONE, Fuzziness.TWO)); + break; + case 5: + if (analyzer == null) { + analyzer = randomAnalyzer(); + } else { + analyzer = null; + } + break; + case 6: + if (quoteAnalyzer == null) { + quoteAnalyzer = randomAnalyzer(); + } else { + quoteAnalyzer = null; + } + break; + case 7: + if (allowLeadingWildCard == null) { + allowLeadingWildCard = randomBoolean(); + } else { + allowLeadingWildCard = randomBoolean() ? null : (allowLeadingWildCard == false); + } + break; + case 8: + if (analyzeWildcard == null) { + analyzeWildcard = randomBoolean(); + } else { + analyzeWildcard = randomBoolean() ? null : (analyzeWildcard == false); + } + break; + case 9: + maxDeterminizedStates += 5; + break; + case 10: + enablePositionIncrements = (enablePositionIncrements == false); + break; + case 11: + escape = (escape == false); + break; + case 12: + phraseSlop += 5; + break; + case 13: + fuzzyMaxExpansions += 5; + break; + case 14: + fuzzyPrefixLength += 5; + break; + case 15: + if (fuzzyRewrite == null) { + fuzzyRewrite = getRandomRewriteMethod(); + } else { + fuzzyRewrite = null; + } + break; + case 16: + if (rewrite == null) { + rewrite = getRandomRewriteMethod(); + } else { + rewrite = null; + } + break; + case 17: + if (quoteFieldSuffix == null) { + quoteFieldSuffix = randomAlphaOfLengthBetween(1, 3); + } else { + quoteFieldSuffix = quoteFieldSuffix + randomAlphaOfLength(1); + } + break; + case 18: + if (tieBreaker == null) { + tieBreaker = randomFloat(); + } else { + tieBreaker += 0.05f; + } + break; + case 19: + if (minimumShouldMatch == null) { + minimumShouldMatch = randomMinimumShouldMatch(); + } else { + minimumShouldMatch = null; + } + break; + case 20: + if (timeZone == null) { + timeZone = randomDateTimeZone().getID(); + } else { + if (randomBoolean()) { + timeZone = null; + } else { + timeZone = randomValueOtherThan(timeZone, () -> randomDateTimeZone().getID()); + } + } + break; + case 21: + autoGenerateSynonymsPhraseQuery = (autoGenerateSynonymsPhraseQuery == false); + break; + case 22: + fuzzyTranspositions = (fuzzyTranspositions == false); + break; + case 23: + return changeNameOrBoost(instance); + default: + throw new AssertionError("Illegal randomisation branch"); + } + + QueryStringQueryBuilder newInstance = new QueryStringQueryBuilder(query); + if (defaultField != null) { + newInstance.defaultField(defaultField); + } + newInstance.fields(fields); + newInstance.defaultOperator(operator); + newInstance.fuzziness(fuzziness); + if (analyzer != null) { + newInstance.analyzer(analyzer); + } + if (quoteAnalyzer != null) { + newInstance.quoteAnalyzer(quoteAnalyzer); + } + if (allowLeadingWildCard != null) { + newInstance.allowLeadingWildcard(allowLeadingWildCard); + } + if (analyzeWildcard != null) { + newInstance.analyzeWildcard(analyzeWildcard); + } + newInstance.maxDeterminizedStates(maxDeterminizedStates); + newInstance.enablePositionIncrements(enablePositionIncrements); + newInstance.escape(escape); + newInstance.phraseSlop(phraseSlop); + newInstance.fuzzyMaxExpansions(fuzzyMaxExpansions); + newInstance.fuzzyPrefixLength(fuzzyPrefixLength); + if (fuzzyRewrite != null) { + newInstance.fuzzyRewrite(fuzzyRewrite); + } + if (rewrite != null) { + newInstance.rewrite(rewrite); + } + if (quoteFieldSuffix != null) { + newInstance.quoteFieldSuffix(quoteFieldSuffix); + } + if (tieBreaker != null) { + newInstance.tieBreaker(tieBreaker); + } + if (minimumShouldMatch != null) { + newInstance.minimumShouldMatch(minimumShouldMatch); + } + if (timeZone != null) { + newInstance.timeZone(timeZone); + } + newInstance.autoGenerateSynonymsPhraseQuery(autoGenerateSynonymsPhraseQuery); + newInstance.fuzzyTranspositions(fuzzyTranspositions); + + return newInstance; + } + @Override protected void doAssertLuceneQuery(QueryStringQueryBuilder queryBuilder, Query query, SearchContext context) throws IOException { @@ -182,6 +384,16 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase new QueryStringQueryBuilder((String) null)); } diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java index 04ac1d6cda0..cc1e0d715af 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java @@ -20,6 +20,7 @@ package org.elasticsearch.test; import com.fasterxml.jackson.core.io.JsonStringEncoder; + import org.apache.lucene.search.BoostQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermQuery; @@ -55,7 +56,6 @@ import org.elasticsearch.common.unit.Fuzziness; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.ToXContent; -import org.elasticsearch.common.xcontent.XContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentGenerator; @@ -742,10 +742,14 @@ public abstract class AbstractQueryTestCase> for (int runs = 0; runs < NUMBER_OF_TESTQUERIES; runs++) { // TODO we only change name and boost, we should extend by any sub-test supplying a "mutate" method that randomly changes one // aspect of the object under test - checkEqualsAndHashCode(createTestQueryBuilder(), this::copyQuery, this::changeNameOrBoost); + checkEqualsAndHashCode(createTestQueryBuilder(), this::copyQuery, this::mutateInstance); } } + public QB mutateInstance(QB instance) throws IOException { + return changeNameOrBoost(instance); + } + /** * Generic test that checks that the Strings.toString() method * renders the XContent correctly. @@ -761,7 +765,7 @@ public abstract class AbstractQueryTestCase> } } - private QB changeNameOrBoost(QB original) throws IOException { + protected QB changeNameOrBoost(QB original) throws IOException { QB secondQuery = copyQuery(original); if (randomBoolean()) { secondQuery.queryName(secondQuery.queryName() == null ? randomAlphaOfLengthBetween(1, 30) : secondQuery.queryName()