From ee893124f51c0560a3a2d3b65574b91909a565e7 Mon Sep 17 00:00:00 2001 From: Tanguy Leroux Date: Thu, 26 Apr 2018 17:47:16 +0200 Subject: [PATCH] Fix TermsSetQueryBuilder.doEquals() method (#29629) Closes #29620 --- .../index/query/TermsSetQueryBuilder.java | 14 +++-- .../query/TermsSetQueryBuilderTests.java | 57 ++++++++++++++++--- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/query/TermsSetQueryBuilder.java b/server/src/main/java/org/elasticsearch/index/query/TermsSetQueryBuilder.java index b8afe967b05ff..5caabd445b32e 100644 --- a/server/src/main/java/org/elasticsearch/index/query/TermsSetQueryBuilder.java +++ b/server/src/main/java/org/elasticsearch/index/query/TermsSetQueryBuilder.java @@ -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; @@ -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; } @@ -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 diff --git a/server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java index c84449e18761c..ec4a7e73eb643 100644 --- a/server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/TermsSetQueryBuilderTests.java @@ -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; @@ -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 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; } @@ -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 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("")); @@ -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); @@ -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 values = new ArrayList<>(numValues); + + for (int i = 0; i < numValues; i++) { + values.add(getRandomValueForFieldName(fieldName)); + } + return values; + } + public static class CustomScriptPlugin extends MockScriptPlugin { @Override