honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-09 14:34:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fixes query_string query equals timezone check (#29406)

Browse Source 
* 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
This commit is contained in:
Colin Goodheart-Smithe 2018-04-06 11:45:34 +01:00 committed by GitHub
parent d649449d37
commit 55c8e80532
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 240 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

26
server/src/main/java/org/elasticsearch/index/query/QueryStringQueryBuilder.java
View File

@ -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<QueryStringQue
return this;
}
public float tieBreaker() {
public Float tieBreaker() {
return this.tieBreaker;
}
@ -389,6 +388,22 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder<QueryStringQue
this.analyzer = analyzer;
return this;
}
/**
* The optional analyzer used to analyze the query string. Note, if a field has search analyzer
* defined for it, then it will be used automatically. Defaults to the smart search analyzer.
*/
public String analyzer() {
return analyzer;
}
/**
* The optional analyzer used to analyze the query string for phrase searches. Note, if a field has search (quote) analyzer
* defined for it, then it will be used automatically. Defaults to the smart search analyzer.
*/
public String quoteAnalyzer() {
return quoteAnalyzer;
}
/**
* The optional analyzer used to analyze the query string for phrase searches. Note, if a field has search (quote) analyzer
@ -884,9 +899,10 @@ public class QueryStringQueryBuilder extends AbstractQueryBuilder<QueryStringQue
Objects.equals(tieBreaker, other.tieBreaker) &&
Objects.equals(rewrite, other.rewrite) &&
Objects.equals(minimumShouldMatch, other.minimumShouldMatch) &&
Objects.equals(lenient, other.lenient) &&
timeZone == null ? other.timeZone == null : other.timeZone != null &&
Objects.equals(timeZone.getID(), other.timeZone.getID()) &&
Objects.equals(lenient, other.lenient) &&
Objects.equals(
timeZone == null ? null : timeZone.getID(),
other.timeZone == null ? null : other.timeZone.getID()) &&
Objects.equals(escape, other.escape) &&
Objects.equals(maxDeterminizedStates, other.maxDeterminizedStates) &&
Objects.equals(autoGenerateSynonymsPhraseQuery, other.autoGenerateSynonymsPhraseQuery) &&

212
server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java
View File

@ -66,7 +66,9 @@ import org.joda.time.DateTimeZone;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import static org.elasticsearch.index.query.AbstractQueryBuilder.parseInnerQueryBuilder;
import static org.elasticsearch.index.query.QueryBuilders.queryStringQuery;
@ -172,6 +174,206 @@ public class QueryStringQueryBuilderTests extends AbstractQueryTestCase<QueryStr
return queryStringQueryBuilder;
}
@Override
public QueryStringQueryBuilder mutateInstance(QueryStringQueryBuilder instance) throws IOException {
String query = instance.queryString();
String defaultField = instance.defaultField();
Map<String, Float> 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<QueryStr
.or(instanceOf(MatchNoDocsQuery.class)));
}
// Tests fix for https://github.com/elastic/elasticsearch/issues/29403
public void testTimezoneEquals() {
QueryStringQueryBuilder builder1 = new QueryStringQueryBuilder("bar");
QueryStringQueryBuilder builder2 = new QueryStringQueryBuilder("foo");
assertNotEquals(builder1, builder2);
builder1.timeZone("Europe/London");
builder2.timeZone("Europe/London");
assertNotEquals(builder1, builder2);
}
public void testIllegalArguments() {
expectThrows(IllegalArgumentException.class, () -> new QueryStringQueryBuilder((String) null));
}

10
test/framework/src/main/java/org/elasticsearch/test/AbstractQueryTestCase.java
View File

@ -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<QB extends AbstractQueryBuilder<QB>>
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 <code>Strings.toString()</code> method
* renders the XContent correctly.
@ -761,7 +765,7 @@ public abstract class AbstractQueryTestCase<QB extends AbstractQueryBuilder<QB>>
}
}
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()