Skip to content

Commit

Permalink
Fix TermsSetQueryBuilder.doEquals() method (#29629)
Browse files Browse the repository at this point in the history 
Closes #29620
  • Loading branch information
@tlrx
tlrx committed Apr 26, 2018
1 parent ea0c431 commit e954938
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 12 deletions.
14 changes: 9 additions & 5 deletions server/src/main/java/org/elasticsearch/index/query/TermsSetQueryBuilder.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@
*/
package org.elasticsearch.index.query;

import org.apache.lucene.index.DocValues;
import org.apache.lucene.index.LeafReaderContext;
import org.apache.lucene.index.NumericDocValues;
import org.apache.lucene.index.SortedNumericDocValues;
import org.apache.lucene.index.Term;
import org.apache.lucene.search.BooleanQuery;
Expand Down Expand Up @@ -86,6 +84,11 @@ protected void doWriteTo(StreamOutput out) throws IOException {
out.writeOptionalWriteable(minimumShouldMatchScript);
}

// package protected for testing purpose
String getFieldName() {
return fieldName;
}

public List<?> getValues() {
return values;
}
Expand Down Expand Up @@ -116,9 +119,10 @@ public TermsSetQueryBuilder setMinimumShouldMatchScript(Script minimumShouldMatc

@Override
protected boolean doEquals(TermsSetQueryBuilder other) {
return Objects.equals(fieldName, this.fieldName) && Objects.equals(values, this.values) &&
Objects.equals(minimumShouldMatchField, this.minimumShouldMatchField) &&
Objects.equals(minimumShouldMatchScript, this.minimumShouldMatchScript);
return Objects.equals(fieldName, other.fieldName)
&& Objects.equals(values, other.values)
&& Objects.equals(minimumShouldMatchField, other.minimumShouldMatchField)
&& Objects.equals(minimumShouldMatchScript, other.minimumShouldMatchScript);
}

@Override
Expand Down
57 changes: 50 additions & 7 deletions server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,9 @@
import java.util.List;
import java.util.Map;
import java.util.function.Function;
import java.util.function.Predicate;

import static java.util.Collections.emptyMap;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.instanceOf;
Expand All @@ -85,17 +87,13 @@ protected TermsSetQueryBuilder doCreateTestQueryBuilder() {
do {
fieldName = randomFrom(MAPPED_FIELD_NAMES);
} while (fieldName.equals(GEO_POINT_FIELD_NAME) || fieldName.equals(GEO_SHAPE_FIELD_NAME));
int numValues = randomIntBetween(0, 10);
List<Object> randomTerms = new ArrayList<>(numValues);
for (int i = 0; i < numValues; i++) {
randomTerms.add(getRandomValueForFieldName(fieldName));
}
List<?> randomTerms = randomValues(fieldName);
TermsSetQueryBuilder queryBuilder = new TermsSetQueryBuilder(STRING_FIELD_NAME, randomTerms);
if (randomBoolean()) {
queryBuilder.setMinimumShouldMatchField("m_s_m");
} else {
queryBuilder.setMinimumShouldMatchScript(
new Script(ScriptType.INLINE, MockScriptEngine.NAME, "_script", Collections.emptyMap()));
new Script(ScriptType.INLINE, MockScriptEngine.NAME, "_script", emptyMap()));
}
return queryBuilder;
}
Expand All @@ -122,6 +120,41 @@ protected boolean builderGeneratesCacheableQueries() {
return false;
}

@Override
public TermsSetQueryBuilder mutateInstance(final TermsSetQueryBuilder instance) throws IOException {
String fieldName = instance.getFieldName();
List<?> values = instance.getValues();
String minimumShouldMatchField = null;
Script minimumShouldMatchScript = null;

switch (randomIntBetween(0, 3)) {
case 0:
Predicate<String> predicate = s -> s.equals(instance.getFieldName()) == false && s.equals(GEO_POINT_FIELD_NAME) == false
&& s.equals(GEO_SHAPE_FIELD_NAME) == false;
fieldName = randomValueOtherThanMany(predicate, () -> randomFrom(MAPPED_FIELD_NAMES));
values = randomValues(fieldName);
break;
case 1:
values = randomValues(fieldName);
break;
case 2:
minimumShouldMatchField = randomAlphaOfLengthBetween(1, 10);
break;
case 3:
minimumShouldMatchScript = new Script(ScriptType.INLINE, MockScriptEngine.NAME, randomAlphaOfLength(10), emptyMap());
break;
}

TermsSetQueryBuilder newInstance = new TermsSetQueryBuilder(fieldName, values);
if (minimumShouldMatchField != null) {
newInstance.setMinimumShouldMatchField(minimumShouldMatchField);
}
if (minimumShouldMatchScript != null) {
newInstance.setMinimumShouldMatchScript(minimumShouldMatchScript);
}
return newInstance;
}

public void testBothFieldAndScriptSpecified() {
TermsSetQueryBuilder queryBuilder = new TermsSetQueryBuilder("_field", Collections.emptyList());
queryBuilder.setMinimumShouldMatchScript(new Script(""));
Expand Down Expand Up @@ -215,7 +248,7 @@ public void testDoToQuery_msmScriptField() throws Exception {

try (IndexReader ir = DirectoryReader.open(directory)) {
QueryShardContext context = createShardContext();
Script script = new Script(ScriptType.INLINE, MockScriptEngine.NAME, "_script", Collections.emptyMap());
Script script = new Script(ScriptType.INLINE, MockScriptEngine.NAME, "_script", emptyMap());
Query query = new TermsSetQueryBuilder("message", Arrays.asList("a", "b", "c", "d"))
.setMinimumShouldMatchScript(script).doToQuery(context);
IndexSearcher searcher = new IndexSearcher(ir);
Expand All @@ -228,6 +261,16 @@ public void testDoToQuery_msmScriptField() throws Exception {
}
}

private static List<?> randomValues(final String fieldName) {
final int numValues = randomIntBetween(0, 10);
final List<Object> values = new ArrayList<>(numValues);

for (int i = 0; i < numValues; i++) {
values.add(getRandomValueForFieldName(fieldName));
}
return values;
}

public static class CustomScriptPlugin extends MockScriptPlugin {

@Override
Expand Down

0 comments on commit e954938

Please sign in to comment.