From dff2e39a8fe0164736b4a6717c147818e2a8bbc0 Mon Sep 17 00:00:00 2001 From: Jim Ferenczi Date: Mon, 6 May 2019 18:45:35 +0200 Subject: [PATCH] Fix full text queries test that start with now (#41854) Full text queries that start with now are not cacheable if they target a date field. However we assume in the query builder tests that all queries are cacheable and this assumption fails when the random generated query string starts with "now". This fails twice in several years since the probability that a random string starts with "now" is low but this commit ensures that isCacheable is correctly checked for full text queries that fall into this edge case. Closes #41847 --- .../index/query/FullTextQueryTestCase.java | 60 +++++++++++++++++++ .../index/query/MatchQueryBuilderTests.java | 9 ++- .../query/MultiMatchQueryBuilderTests.java | 10 +++- .../query/QueryStringQueryBuilderTests.java | 8 ++- .../query/SimpleQueryStringBuilderTests.java | 12 ++-- .../test/AbstractBuilderTestCase.java | 4 ++ 6 files changed, 89 insertions(+), 14 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java diff --git a/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java b/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java new file mode 100644 index 0000000000000..7f9d85e3e26b7 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/index/query/FullTextQueryTestCase.java @@ -0,0 +1,60 @@ +/* + * Licensed to Elasticsearch under one or more contributor + * license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright + * ownership. Elasticsearch licenses this file to you under + * the Apache License, Version 2.0 (the "License"); you may + * not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.elasticsearch.index.query; + +import org.elasticsearch.index.mapper.DateFieldMapper; +import org.elasticsearch.index.mapper.MappedFieldType; +import org.elasticsearch.test.AbstractQueryTestCase; + +import java.util.Collection; +import java.util.HashSet; +import java.util.Set; + +public abstract class FullTextQueryTestCase> extends AbstractQueryTestCase { + protected abstract boolean isCacheable(QB queryBuilder); + + /** + * Full text queries that start with "now" are not cacheable if they + * target a {@link DateFieldMapper.DateFieldType} field. + */ + protected final boolean isCacheable(Collection fields, String value) { + if (value.length() < 3 + || value.substring(0, 3).equalsIgnoreCase("now") == false) { + return true; + } + Set dateFields = new HashSet<>(); + getMapping().forEach(ft -> { + if (ft instanceof DateFieldMapper.DateFieldType) { + dateFields.add(ft.name()); + } + }); + for (MappedFieldType ft : getMapping()) { + if (ft instanceof DateFieldMapper.DateFieldType) { + dateFields.add(ft.name()); + } + } + if (fields.isEmpty()) { + // special case: all fields are requested + return dateFields.isEmpty(); + } + return fields.stream() + .anyMatch(dateFields::contains) == false; + } +} diff --git a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index e9f2b447da133..a7aad3dbc3e42 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -46,7 +46,6 @@ import org.elasticsearch.index.search.MatchQuery.Type; import org.elasticsearch.index.search.MatchQuery.ZeroTermsQuery; import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; import org.hamcrest.Matcher; import java.io.IOException; @@ -56,13 +55,19 @@ import java.util.Locale; import java.util.Map; +import static java.util.Collections.singletonList; import static org.hamcrest.CoreMatchers.either; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.notNullValue; -public class MatchQueryBuilderTests extends AbstractQueryTestCase { +public class MatchQueryBuilderTests extends FullTextQueryTestCase { + @Override + protected boolean isCacheable(MatchQueryBuilder queryBuilder) { + return queryBuilder.fuzziness() != null + || isCacheable(singletonList(queryBuilder.fieldName()), queryBuilder.value().toString()); + } @Override protected MatchQueryBuilder doCreateTestQueryBuilder() { diff --git a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java index 7ca722fc31139..ab9b3c732135d 100644 --- a/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/MultiMatchQueryBuilderTests.java @@ -42,7 +42,6 @@ import org.elasticsearch.index.query.MultiMatchQueryBuilder.Type; import org.elasticsearch.index.search.MatchQuery; import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; import java.util.Arrays; @@ -59,11 +58,16 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.collection.IsCollectionWithSize.hasSize; -public class MultiMatchQueryBuilderTests extends AbstractQueryTestCase { - +public class MultiMatchQueryBuilderTests extends FullTextQueryTestCase { private static final String MISSING_WILDCARD_FIELD_NAME = "missing_*"; private static final String MISSING_FIELD_NAME = "missing"; + @Override + protected boolean isCacheable(MultiMatchQueryBuilder queryBuilder) { + return queryBuilder.fuzziness() != null + || isCacheable(queryBuilder.fields().keySet(), queryBuilder.value().toString()); + } + @Override protected MultiMatchQueryBuilder doCreateTestQueryBuilder() { String fieldName = randomFrom(STRING_FIELD_NAME, INT_FIELD_NAME, DOUBLE_FIELD_NAME, BOOLEAN_FIELD_NAME, DATE_FIELD_NAME, diff --git a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java index 608eb5d9d6cd2..5ace39c0890df 100644 --- a/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/QueryStringQueryBuilderTests.java @@ -62,7 +62,6 @@ import org.elasticsearch.index.mapper.MapperService; import org.elasticsearch.index.search.QueryStringQueryParser; import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; import org.hamcrest.CoreMatchers; import org.hamcrest.Matchers; @@ -84,7 +83,12 @@ import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.instanceOf; -public class QueryStringQueryBuilderTests extends AbstractQueryTestCase { +public class QueryStringQueryBuilderTests extends FullTextQueryTestCase { + @Override + protected boolean isCacheable(QueryStringQueryBuilder queryBuilder) { + return queryBuilder.fuzziness() != null + || isCacheable(queryBuilder.fields().keySet(), queryBuilder.queryString()); + } @Override protected void initializeAdditionalMappings(MapperService mapperService) throws IOException { diff --git a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java index 2bb289ddc11fa..daed696f02fd9 100644 --- a/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/SimpleQueryStringBuilderTests.java @@ -44,7 +44,6 @@ import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.search.SimpleQueryStringQueryParser; import org.elasticsearch.search.internal.SearchContext; -import org.elasticsearch.test.AbstractQueryTestCase; import java.io.IOException; import java.util.ArrayList; @@ -65,7 +64,11 @@ import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; -public class SimpleQueryStringBuilderTests extends AbstractQueryTestCase { +public class SimpleQueryStringBuilderTests extends FullTextQueryTestCase { + @Override + protected boolean isCacheable(SimpleQueryStringBuilder queryBuilder) { + return isCacheable(queryBuilder.fields().keySet(), queryBuilder.value()); + } @Override protected SimpleQueryStringBuilder doCreateTestQueryBuilder() { @@ -107,11 +110,6 @@ protected SimpleQueryStringBuilder doCreateTestQueryBuilder() { fields.put(STRING_FIELD_NAME_2, 2.0f / randomIntBetween(1, 20)); } } - // special handling if query start with "now" and no field specified. This hits the "mapped_date" field which leads to the query not - // being cacheable and trigger later test failures (see https://github.com/elastic/elasticsearch/issues/35183) - if (fieldCount == 0 && queryText.length() >= 3 && queryText.substring(0,3).equalsIgnoreCase("now")) { - fields.put(STRING_FIELD_NAME_2, 2.0f / randomIntBetween(1, 20)); - } result.fields(fields); if (randomBoolean()) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java index d607d73336826..61298ce0e8418 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/AbstractBuilderTestCase.java @@ -193,6 +193,10 @@ protected static String expectedFieldName(String builderFieldName) { return ALIAS_TO_CONCRETE_FIELD_NAME.getOrDefault(builderFieldName, builderFieldName); } + protected Iterable getMapping() { + return serviceHolder.mapperService.fieldTypes(); + } + @AfterClass public static void afterClass() throws Exception { IOUtils.close(serviceHolder);