From 6a27628f129773da5bad65a99b7394273e632198 Mon Sep 17 00:00:00 2001 From: javanna Date: Sat, 10 Dec 2016 23:55:19 +0100 Subject: [PATCH] Remove support for strict parsing mode We return deprecation warnings as response headers, besides logging them. Strict parsing mode stayed around, but was only used in query tests, though we also introduced checks for deprecation warnings there that don't need strict parsing anymore (see #20993). We can then safely remove support for strict parsing mode. The final goal is to remove the ParseFieldMatcher class, but there are many many users of it. This commit prepares the field for the removal, by deprecating ParseFieldMatcher and making it effectively not needed. Strict parsing is removed from ParseFieldMatcher, and strict parsing is replaced in tests where needed with deprecation warnings checks. Note that the setting to enable strict parsing was never ported to the new settings infra hance it cannot be set in production. It is really only used in our own tests. Relates to #19552 --- .../org/elasticsearch/common/ParseField.java | 12 +---- .../common/ParseFieldMatcher.java | 31 ++++------- .../index/mapper/TypeParsers.java | 51 +++++++------------ .../query/GeoBoundingBoxQueryBuilder.java | 4 +- .../index/query/GeoDistanceQueryBuilder.java | 6 +-- .../index/query/GeoPolygonQueryBuilder.java | 6 +-- .../search/sort/GeoDistanceSortBuilder.java | 6 +-- .../elasticsearch/common/ParseFieldTests.java | 38 +++----------- .../common/xcontent/ObjectParserTests.java | 45 ++++++++-------- .../GeoBoundingBoxQueryBuilderTests.java | 21 +++----- .../query/GeoDistanceQueryBuilderTests.java | 19 +++---- .../query/GeoPolygonQueryBuilderTests.java | 13 +++-- .../query/HasParentQueryBuilderTests.java | 7 +-- .../index/query/IdsQueryBuilderTests.java | 14 ++--- .../index/query/MatchQueryBuilderTests.java | 16 +----- .../index/query/RangeQueryBuilderTests.java | 12 +---- .../search/sort/AbstractSortTestCase.java | 45 +++++++++++++++- .../sort/GeoDistanceSortBuilderTests.java | 24 ++++----- .../mustache/MustacheScriptEngineTests.java | 4 +- .../mustache/TemplateQueryBuilderTests.java | 2 +- .../test/AbstractQueryTestCase.java | 32 +++++------- 21 files changed, 175 insertions(+), 233 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/ParseField.java b/core/src/main/java/org/elasticsearch/common/ParseField.java index 7121be7d1d8..fc9377eeb2f 100644 --- a/core/src/main/java/org/elasticsearch/common/ParseField.java +++ b/core/src/main/java/org/elasticsearch/common/ParseField.java @@ -101,14 +101,10 @@ public class ParseField { /** * @param fieldName * the field name to match against this {@link ParseField} - * @param strict - * if true an exception will be thrown if a deprecated field name - * is given. If false the deprecated name will be matched but a - * message will also be logged to the {@link DeprecationLogger} * @return true if fieldName matches any of the acceptable * names for this {@link ParseField}. */ - boolean match(String fieldName, boolean strict) { + public boolean match(String fieldName) { Objects.requireNonNull(fieldName, "fieldName cannot be null"); // if this parse field has not been completely deprecated then try to // match the preferred name @@ -128,11 +124,7 @@ public class ParseField { // message to indicate what should be used instead msg = "Deprecated field [" + fieldName + "] used, replaced by [" + allReplacedWith + "]"; } - if (strict) { - throw new IllegalArgumentException(msg); - } else { - DEPRECATION_LOGGER.deprecated(msg); - } + DEPRECATION_LOGGER.deprecated(msg); return true; } } diff --git a/core/src/main/java/org/elasticsearch/common/ParseFieldMatcher.java b/core/src/main/java/org/elasticsearch/common/ParseFieldMatcher.java index 9866694a230..a7d412398e5 100644 --- a/core/src/main/java/org/elasticsearch/common/ParseFieldMatcher.java +++ b/core/src/main/java/org/elasticsearch/common/ParseFieldMatcher.java @@ -22,38 +22,29 @@ package org.elasticsearch.common; import org.elasticsearch.common.settings.Settings; /** - * Matcher to use in combination with {@link ParseField} while parsing requests. Matches a {@link ParseField} - * against a field name and throw deprecation exception depending on the current value of the {@link #PARSE_STRICT} setting. + * Matcher to use in combination with {@link ParseField} while parsing requests. + * + * @deprecated This class used to be useful to parse in strict mode and emit errors rather than deprecation warnings. Now that we return + * warnings as response headers all the time, it is no longer useful and will soon be removed. The removal is in progress and there is + * already no strict mode in fact. Use {@link ParseField} directly. */ +@Deprecated public class ParseFieldMatcher { - public static final String PARSE_STRICT = "index.query.parse.strict"; - public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(false); - public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(true); - - private final boolean strict; + public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(Settings.EMPTY); + public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(Settings.EMPTY); public ParseFieldMatcher(Settings settings) { - this(settings.getAsBoolean(PARSE_STRICT, false)); - } - - public ParseFieldMatcher(boolean strict) { - this.strict = strict; - } - - /** Should deprecated settings be rejected? */ - public boolean isStrict() { - return strict; + //we don't do anything with the settings argument, this whole class will be soon removed } /** - * Matches a {@link ParseField} against a field name, and throws deprecation exception depending on the current - * value of the {@link #PARSE_STRICT} setting. + * Matches a {@link ParseField} against a field name, * @param fieldName the field name found in the request while parsing * @param parseField the parse field that we are looking for * @throws IllegalArgumentException whenever we are in strict mode and the request contained a deprecated field * @return true whenever the parse field that we are looking for was found, false otherwise */ public boolean match(String fieldName, ParseField parseField) { - return parseField.match(fieldName, strict); + return parseField.match(fieldName); } } diff --git a/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java b/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java index 475848989d4..0eb21be4862 100644 --- a/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java +++ b/core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java @@ -25,7 +25,6 @@ import org.elasticsearch.Version; import org.elasticsearch.common.joda.FormatDateTimeFormatter; import org.elasticsearch.common.joda.Joda; import org.elasticsearch.common.logging.DeprecationLogger; -import org.elasticsearch.common.logging.ESLoggerFactory; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.xcontent.support.XContentMapValues; import org.elasticsearch.index.analysis.NamedAnalyzer; @@ -43,7 +42,6 @@ import java.util.Set; import static org.elasticsearch.common.xcontent.support.XContentMapValues.isArray; import static org.elasticsearch.common.xcontent.support.XContentMapValues.lenientNodeBooleanValue; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeFloatValue; -import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeIntegerValue; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeMapValue; import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeStringValue; @@ -59,16 +57,11 @@ public class TypeParsers { private static final Set BOOLEAN_STRINGS = new HashSet<>(Arrays.asList("true", "false")); public static boolean nodeBooleanValue(String name, Object node, Mapper.TypeParser.ParserContext parserContext) { - // Hook onto ParseFieldMatcher so that parsing becomes strict when setting index.query.parse.strict - if (parserContext.parseFieldMatcher().isStrict()) { - return XContentMapValues.nodeBooleanValue(node); - } else { - // TODO: remove this leniency in 6.0 - if (BOOLEAN_STRINGS.contains(node.toString()) == false) { - DEPRECATION_LOGGER.deprecated("Expected a boolean for property [{}] but got [{}]", name, node); - } - return XContentMapValues.lenientNodeBooleanValue(node); + // TODO: remove this leniency in 6.0 + if (BOOLEAN_STRINGS.contains(node.toString()) == false) { + DEPRECATION_LOGGER.deprecated("Expected a boolean for property [{}] but got [{}]", name, node); } + return XContentMapValues.lenientNodeBooleanValue(node); } private static void parseAnalyzersAndTermVectors(FieldMapper.Builder builder, String name, Map fieldNode, Mapper.TypeParser.ParserContext parserContext) { @@ -211,10 +204,10 @@ public class TypeParsers { throw new MapperParsingException("[" + propName + "] must not have a [null] value"); } if (propName.equals("store")) { - builder.store(parseStore(name, propNode.toString(), parserContext)); + builder.store(parseStore(propNode.toString())); iterator.remove(); } else if (propName.equals("index")) { - builder.index(parseIndex(name, propNode.toString(), parserContext)); + builder.index(parseIndex(name, propNode.toString())); iterator.remove(); } else if (propName.equals(DOC_VALUES)) { builder.docValues(nodeBooleanValue(DOC_VALUES, propNode, parserContext)); @@ -346,7 +339,7 @@ public class TypeParsers { } } - public static boolean parseIndex(String fieldName, String index, Mapper.TypeParser.ParserContext parserContext) throws MapperParsingException { + private static boolean parseIndex(String fieldName, String index) throws MapperParsingException { switch (index) { case "true": return true; @@ -355,31 +348,23 @@ public class TypeParsers { case "not_analyzed": case "analyzed": case "no": - if (parserContext.parseFieldMatcher().isStrict() == false) { - DEPRECATION_LOGGER.deprecated("Expected a boolean for property [index] but got [{}]", index); - return "no".equals(index) == false; - } else { - throw new IllegalArgumentException("Can't parse [index] value [" + index + "] for field [" + fieldName + "], expected [true] or [false]"); - } + DEPRECATION_LOGGER.deprecated("Expected a boolean for property [index] but got [{}]", index); + return "no".equals(index) == false; default: throw new IllegalArgumentException("Can't parse [index] value [" + index + "] for field [" + fieldName + "], expected [true] or [false]"); } } - public static boolean parseStore(String fieldName, String store, Mapper.TypeParser.ParserContext parserContext) throws MapperParsingException { - if (parserContext.parseFieldMatcher().isStrict()) { - return XContentMapValues.nodeBooleanValue(store); + private static boolean parseStore(String store) throws MapperParsingException { + if (BOOLEAN_STRINGS.contains(store) == false) { + DEPRECATION_LOGGER.deprecated("Expected a boolean for property [store] but got [{}]", store); + } + if ("no".equals(store)) { + return false; + } else if ("yes".equals(store)) { + return true; } else { - if (BOOLEAN_STRINGS.contains(store) == false) { - DEPRECATION_LOGGER.deprecated("Expected a boolean for property [store] but got [{}]", store); - } - if ("no".equals(store)) { - return false; - } else if ("yes".equals(store)) { - return true; - } else { - return lenientNodeBooleanValue(store); - } + return lenientNodeBooleanValue(store); } } diff --git a/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java b/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java index ca61a66066c..07b39ba12c1 100644 --- a/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java +++ b/core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java @@ -62,9 +62,9 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder private static final ParseField UNIT_FIELD = new ParseField("unit"); private static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type"); private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method"); - private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed") - .withAllDeprecated("use validation_method instead"); - private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize") - .withAllDeprecated("use validation_method instead"); + private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed").withAllDeprecated("validation_method"); + private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize").withAllDeprecated("validation_method"); private static final ParseField SORTMODE_FIELD = new ParseField("mode", "sort_mode"); private final String fieldName; diff --git a/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java b/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java index 1f348000ee4..6ae7b3c230f 100644 --- a/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java +++ b/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java @@ -20,7 +20,6 @@ package org.elasticsearch.common; import org.elasticsearch.test.ESTestCase; -import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.sameInstance; @@ -33,32 +32,16 @@ public class ParseFieldTests extends ESTestCase { String[] deprecated = new String[]{"barFoo", "bar_foo", "Foobar"}; ParseField withDeprecations = field.withDeprecation(deprecated); assertThat(field, not(sameInstance(withDeprecations))); - assertThat(field.match(name, false), is(true)); - assertThat(field.match("foo bar", false), is(false)); + assertThat(field.match(name), is(true)); + assertThat(field.match("foo bar"), is(false)); for (String deprecatedName : deprecated) { - assertThat(field.match(deprecatedName, false), is(false)); + assertThat(field.match(deprecatedName), is(false)); } - assertThat(withDeprecations.match(name, false), is(true)); - assertThat(withDeprecations.match("foo bar", false), is(false)); + assertThat(withDeprecations.match(name), is(true)); + assertThat(withDeprecations.match("foo bar"), is(false)); for (String deprecatedName : deprecated) { - assertThat(withDeprecations.match(deprecatedName, false), is(true)); - } - - // now with strict mode - assertThat(field.match(name, true), is(true)); - assertThat(field.match("foo bar", true), is(false)); - for (String deprecatedName : deprecated) { - assertThat(field.match(deprecatedName, true), is(false)); - } - - assertThat(withDeprecations.match(name, true), is(true)); - assertThat(withDeprecations.match("foo bar", true), is(false)); - for (String deprecatedName : deprecated) { - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> { - withDeprecations.match(deprecatedName, true); - }); - assertThat(e.getMessage(), containsString("used, expected [foo_bar] instead")); + assertThat(withDeprecations.match(deprecatedName), is(true)); } } @@ -84,13 +67,8 @@ public class ParseFieldTests extends ESTestCase { field = new ParseField(name).withAllDeprecated("like"); } - // strict mode off - assertThat(field.match(randomFrom(allValues), false), is(true)); - assertThat(field.match("not a field name", false), is(false)); - - // now with strict mode - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> field.match(randomFrom(allValues), true)); - assertThat(e.getMessage(), containsString(" used, replaced by [like]")); + assertThat(field.match(randomFrom(allValues)), is(true)); + assertThat(field.match("not a field name"), is(false)); } public void testGetAllNamesIncludedDeprecated() { diff --git a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index 46c0ba35723..43c421c5f0e 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -22,6 +22,9 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParseFieldMatcherSupplier; import org.elasticsearch.common.ParsingException; +import org.elasticsearch.common.logging.DeprecationLogger; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.AbstractObjectParser.NoContextParser; import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser; import org.elasticsearch.common.xcontent.ObjectParser.ValueType; @@ -35,6 +38,8 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; public class ObjectParserTests extends ESTestCase { @@ -218,27 +223,27 @@ public class ObjectParserTests extends ESTestCase { } } - public void testDeprecationFail() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"old_test\" : \"foo\"}"); - class TestStruct { - public String test; + public void testDeprecationWarnings() throws IOException { + try (ThreadContext threadContext = new ThreadContext(Settings.EMPTY)) { + DeprecationLogger.setThreadContext(threadContext); + class TestStruct { + public String test; + } + ObjectParser objectParser = new ObjectParser<>("foo"); + TestStruct s = new TestStruct(); + + objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); + + XContentParser parser = createParser(XContentType.JSON.xContent(), "{\"old_test\" : \"foo\"}"); + objectParser.parse(parser, s, () -> ParseFieldMatcher.EMPTY); + + assertEquals("foo", s.test); + + final List warnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); + assertThat(warnings, hasSize(1)); + assertThat(warnings, hasItem(equalTo("Deprecated field [old_test] used, expected [test] instead"))); + DeprecationLogger.removeThreadContext(threadContext); } - ObjectParser objectParser = new ObjectParser<>("foo"); - TestStruct s = new TestStruct(); - - objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); - - try { - objectParser.parse(parser, s, STRICT_PARSING); - fail("deprecated value"); - } catch (IllegalArgumentException ex) { - assertEquals(ex.getMessage(), "Deprecated field [old_test] used, expected [test] instead"); - - } - assertNull(s.test); - parser = createParser(JsonXContent.jsonXContent, "{\"old_test\" : \"foo\"}"); - objectParser.parse(parser, s, () -> ParseFieldMatcher.EMPTY); - assertEquals("foo", s.test); } public void testFailOnValueType() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java index 49266ebe9fd..e1757756e35 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java @@ -19,13 +19,8 @@ package org.elasticsearch.index.query; -import org.apache.lucene.search.BooleanClause; -import org.apache.lucene.search.BooleanQuery; -import org.apache.lucene.search.ConstantScoreQuery; import org.apache.lucene.search.MatchNoDocsQuery; import org.apache.lucene.search.Query; -import org.apache.lucene.spatial.geopoint.search.GeoPointInBBoxQuery; -import org.elasticsearch.Version; import org.elasticsearch.common.geo.GeoPoint; import org.elasticsearch.common.geo.GeoUtils; import org.elasticsearch.index.mapper.MappedFieldType; @@ -40,7 +35,6 @@ import java.io.IOException; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.notNullValue; -import static org.hamcrest.Matchers.equalTo; public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase { /** Randomly generate either NaN or one of the two infinity values. */ @@ -118,7 +112,7 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase super.testToQuery()); + QueryShardException e = expectThrows(QueryShardException.class, super::testToQuery); assertEquals("failed to find geo_point field [mapped_geo_point]", e.getMessage()); } @@ -412,7 +406,7 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); + + parseQuery(json); + assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); } - public void testFromJsonIgnoreMalformedFails() throws IOException { + public void testFromJsonIgnoreMalformedIsDeprecated() throws IOException { String json = "{\n" + " \"geo_bounding_box\" : {\n" + @@ -444,8 +439,8 @@ public class GeoBoundingBoxQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); + parseQuery(json); + assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java index 6c92fde6843..7b317ae85ab 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java @@ -316,7 +316,7 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); + parseQuery(json); + assertWarningHeaders("Deprecated field [optimize_bbox] used, replaced by [no replacement: " + + "`optimize_bbox` is no longer supported due to recent improvements]"); } - public void testFromCoerceFails() throws IOException { + public void testFromCoerceIsDeprecated() throws IOException { String json = "{\n" + " \"geo_distance\" : {\n" + @@ -345,11 +346,11 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); + parseQuery(json); + assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); } - public void testFromJsonIgnoreMalformedFails() throws IOException { + public void testFromJsonIgnoreMalformedIsDeprecated() throws IOException { String json = "{\n" + " \"geo_distance\" : {\n" + @@ -361,8 +362,8 @@ public class GeoDistanceQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); + parseQuery(json); + assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java index b77ff3bbdef..5aff43291e2 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java @@ -139,8 +139,8 @@ public class GeoPolygonQueryBuilderTests extends AbstractQueryTestCase parseQuery(builder.string())); - assertEquals("Deprecated field [normalize] used, replaced by [use validation_method instead]", e.getMessage()); + parseQuery(builder.string()); + assertWarningHeaders("Deprecated field [normalize] used, replaced by [validation_method]"); } public void testParsingAndToQueryParsingExceptions() throws IOException { @@ -265,9 +265,8 @@ public class GeoPolygonQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); - + parseQuery(json); + assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); } public void testFromJsonCoerceDeprecated() throws IOException { @@ -282,8 +281,8 @@ public class GeoPolygonQueryBuilderTests extends AbstractQueryTestCase parseQuery(json)); - assertTrue(e.getMessage().startsWith("Deprecated field ")); + parseQuery(json); + assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); } @Override diff --git a/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java index ca402782e8c..9250467e618 100644 --- a/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java @@ -149,12 +149,9 @@ public class HasParentQueryBuilderTests extends AbstractQueryTestCase parseQuery(builder.string())); - assertEquals("Deprecated field [type] used, expected [parent_type] instead", e.getMessage()); - - HasParentQueryBuilder queryBuilder = (HasParentQueryBuilder) parseQuery(builder.string(), ParseFieldMatcher.EMPTY); + HasParentQueryBuilder queryBuilder = (HasParentQueryBuilder) parseQuery(builder.string()); assertEquals("foo", queryBuilder.type()); - checkWarningHeaders("Deprecated field [type] used, expected [parent_type] instead"); + assertWarningHeaders("Deprecated field [type] used, expected [parent_type] instead"); } public void testToQueryInnerQueryType() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java index 5913a038661..f711c7fb8f1 100644 --- a/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java @@ -164,11 +164,8 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase IdsQueryBuilder parsed = (IdsQueryBuilder) parseQuery(contentString, ParseFieldMatcher.EMPTY); assertEquals(testQuery, parsed); - ParsingException e = expectThrows(ParsingException.class, () -> parseQuery(contentString)); - checkWarningHeaders("Deprecated field [_type] used, expected [type] instead"); - assertEquals("Deprecated field [_type] used, expected [type] instead", e.getMessage()); - assertEquals(3, e.getLineNumber()); - assertEquals(19, e.getColumnNumber()); + parseQuery(contentString); + assertWarningHeaders("Deprecated field [_type] used, expected [type] instead"); //array of types can also be called types rather than type final String contentString2 = "{\n" + @@ -180,10 +177,7 @@ public class IdsQueryBuilderTests extends AbstractQueryTestCase parsed = (IdsQueryBuilder) parseQuery(contentString2, ParseFieldMatcher.EMPTY); assertEquals(testQuery, parsed); - e = expectThrows(ParsingException.class, () -> parseQuery(contentString2)); - checkWarningHeaders("Deprecated field [types] used, expected [type] instead"); - assertEquals("Deprecated field [types] used, expected [type] instead", e.getMessage()); - assertEquals(3, e.getLineNumber()); - assertEquals(19, e.getColumnNumber()); + parseQuery(contentString2); + assertWarningHeaders("Deprecated field [types] used, expected [type] instead"); } } diff --git a/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java index a32eafd850c..6e19ad40940 100644 --- a/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -318,13 +318,8 @@ public class MatchQueryBuilderTests extends AbstractQueryTestCase parseQuery(json, ParseFieldMatcher.STRICT)); - assertThat(e.getMessage(), - containsString("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]")); } public void testLegacyMatchPhraseQuery() throws IOException { @@ -351,16 +346,9 @@ public class MatchQueryBuilderTests extends AbstractQueryTestCase parseQuery(json, ParseFieldMatcher.STRICT)); - assertThat(e.getMessage(), - containsString("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]")); } public void testFuzzinessOnNonStringField() throws Exception { diff --git a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java index 09627d00d76..ede35d35021 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java @@ -26,7 +26,6 @@ import org.apache.lucene.search.PointRangeQuery; import org.apache.lucene.search.Query; import org.apache.lucene.search.TermRangeQuery; import org.elasticsearch.ElasticsearchParseException; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.ParsingException; import org.elasticsearch.common.geo.ShapeRelation; import org.elasticsearch.common.lucene.BytesRefs; @@ -39,7 +38,6 @@ import org.elasticsearch.test.AbstractQueryTestCase; import org.joda.time.DateTime; import org.joda.time.DateTimeZone; import org.joda.time.chrono.ISOChronology; -import org.locationtech.spatial4j.shape.SpatialRelation; import java.io.IOException; import java.util.HashMap; @@ -388,14 +386,8 @@ public class RangeQueryBuilderTests extends AbstractQueryTestCase parseQuery(deprecatedJson, ParseFieldMatcher.STRICT)); - assertEquals("Deprecated field [_name] used, replaced by [query name is not supported in short version of range query]", - e.getMessage()); + assertNotNull(parseQuery(deprecatedJson)); + assertWarningHeaders("Deprecated field [_name] used, replaced by [query name is not supported in short version of range query]"); } public void testRewriteDateToMatchAll() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java index d545a082b55..c3b1b22ea4a 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -25,8 +25,9 @@ import org.elasticsearch.Version; import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -66,16 +67,22 @@ import org.elasticsearch.search.SearchModule; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.IndexSettingsModule; import org.elasticsearch.watcher.ResourceWatcherService; +import org.junit.After; import org.junit.AfterClass; +import org.junit.Before; import org.junit.BeforeClass; import java.io.IOException; import java.nio.file.Path; import java.util.Collections; +import java.util.List; import java.util.Map; import static java.util.Collections.emptyList; import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; public abstract class AbstractSortTestCase> extends ESTestCase { @@ -84,6 +91,7 @@ public abstract class AbstractSortTestCase> extends EST private static final int NUMBER_OF_TESTBUILDERS = 20; static IndicesQueriesRegistry indicesQueriesRegistry; private static ScriptService scriptService; + private ThreadContext threadContext; @BeforeClass public static void init() throws IOException { @@ -115,6 +123,39 @@ public abstract class AbstractSortTestCase> extends EST indicesQueriesRegistry = null; } + @Before + public void beforeTest() { + this.threadContext = new ThreadContext(Settings.EMPTY); + DeprecationLogger.setThreadContext(threadContext); + } + + @After + public void afterTest() throws IOException { + //Check that there are no unaccounted warning headers. These should be checked with assertWarningHeaders(String...) in the + //appropriate test + final List warnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); + assertNull("unexpected warning headers", warnings); + DeprecationLogger.removeThreadContext(this.threadContext); + this.threadContext.close(); + } + + protected void assertWarningHeaders(String... expectedWarnings) { + final List actualWarnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); + assertThat(actualWarnings, hasSize(expectedWarnings.length)); + for (String msg : expectedWarnings) { + assertThat(actualWarnings, hasItem(equalTo(msg))); + } + // "clear" current warning headers by setting a new ThreadContext + DeprecationLogger.removeThreadContext(this.threadContext); + try { + this.threadContext.close(); + } catch (IOException e) { + throw new RuntimeException(e); + } + this.threadContext = new ThreadContext(Settings.EMPTY); + DeprecationLogger.setThreadContext(this.threadContext); + } + /** Returns random sort that is put under test */ protected abstract T createTestItem(); @@ -251,6 +292,6 @@ public abstract class AbstractSortTestCase> extends EST @SuppressWarnings("unchecked") private T copy(T original) throws IOException { return copyWriteable(original, namedWriteableRegistry, - (Writeable.Reader) namedWriteableRegistry.getReader(SortBuilder.class, original.getWriteableName())); + namedWriteableRegistry.getReader(SortBuilder.class, original.getWriteableName())); } } diff --git a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java index 131d19f600d..977b4b3cad4 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -267,17 +267,15 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase GeoDistanceSortBuilder.fromXContent(context, "")); - assertTrue(e.getMessage().startsWith("Deprecated field ")); - + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.EMPTY); + GeoDistanceSortBuilder.fromXContent(context, ""); + assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); } public void testIgnoreMalformedIsDeprecated() throws IOException { @@ -288,17 +286,15 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase GeoDistanceSortBuilder.fromXContent(context, "")); - assertTrue(e.getMessage().startsWith("Deprecated field ")); - + QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.EMPTY); + GeoDistanceSortBuilder.fromXContent(context, ""); + assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); } public void testSortModeSumIsRejectedInJSON() throws IOException { @@ -455,8 +451,8 @@ public class GeoDistanceSortBuilderTests extends AbstractSortTestCase parse(sortBuilder)); - assertEquals("Deprecated field [sort_mode] used, expected [mode] instead", ex.getMessage()); + parse(sortBuilder); + assertWarningHeaders("Deprecated field [sort_mode] used, expected [mode] instead"); } private GeoDistanceSortBuilder parse(XContentBuilder sortBuilder) throws Exception { diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java index 6a936260ca5..701ee8d984f 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/MustacheScriptEngineTests.java @@ -87,7 +87,7 @@ public class MustacheScriptEngineTests extends ESTestCase { + "\"params\":{\"template\":\"all\"}" + "}"; XContentParser parser = createParser(JsonXContent.jsonXContent, templateString); - Script script = Script.parse(parser, new ParseFieldMatcher(false)); + Script script = Script.parse(parser, ParseFieldMatcher.EMPTY); CompiledScript compiledScript = new CompiledScript(ScriptType.INLINE, null, "mustache", qe.compile(null, script.getIdOrCode(), Collections.emptyMap())); ExecutableScript executableScript = qe.executable(compiledScript, script.getParams()); @@ -103,7 +103,7 @@ public class MustacheScriptEngineTests extends ESTestCase { + " }" + "}"; XContentParser parser = createParser(JsonXContent.jsonXContent, templateString); - Script script = Script.parse(parser, new ParseFieldMatcher(false)); + Script script = Script.parse(parser, ParseFieldMatcher.EMPTY); CompiledScript compiledScript = new CompiledScript(ScriptType.INLINE, null, "mustache", qe.compile(null, script.getIdOrCode(), Collections.emptyMap())); ExecutableScript executableScript = qe.executable(compiledScript, script.getParams()); diff --git a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/TemplateQueryBuilderTests.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/TemplateQueryBuilderTests.java index 34e06410a78..5f2ad37c2a6 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/TemplateQueryBuilderTests.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/TemplateQueryBuilderTests.java @@ -63,7 +63,7 @@ public class TemplateQueryBuilderTests extends AbstractQueryTestCase> .put(ScriptService.SCRIPT_AUTO_RELOAD_ENABLED_SETTING.getKey(), false) .build(); indexSettings = Settings.builder() - .put(ParseFieldMatcher.PARSE_STRICT, true) .put(IndexMetaData.SETTING_VERSION_CREATED, indexVersionCreated).build(); index = new Index(randomAsciiOfLengthBetween(1, 10), "_na_"); @@ -218,18 +217,6 @@ public abstract class AbstractQueryTestCase> DeprecationLogger.setThreadContext(threadContext); } - /** - * Check that there are no unaccounted warning headers. These should be checked with {@link #checkWarningHeaders(String...)} in the - * appropriate test - */ - @After - public void teardown() throws IOException { - final List warnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); - assertNull("unexpected warning headers", warnings); - DeprecationLogger.removeThreadContext(this.threadContext); - this.threadContext.close(); - } - private static SearchContext getSearchContext(String[] types, QueryShardContext context) { TestSearchContext testSearchContext = new TestSearchContext(context) { @Override @@ -247,7 +234,14 @@ public abstract class AbstractQueryTestCase> } @After - public void afterTest() { + public void afterTest() throws IOException { + //Check that there are no unaccounted warning headers. These should be checked with {@link #checkWarningHeaders(String...)} in the + //appropriate test + final List warnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); + assertNull("unexpected warning headers", warnings); + DeprecationLogger.removeThreadContext(this.threadContext); + this.threadContext.close(); + serviceHolder.clientInvocationHandler.delegate = null; } @@ -1026,11 +1020,11 @@ public abstract class AbstractQueryTestCase> return query; } - protected void checkWarningHeaders(String... messages) { - final List warnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); - assertThat(warnings, hasSize(messages.length)); - for (String msg : messages) { - assertThat(warnings, hasItem(equalTo(msg))); + protected void assertWarningHeaders(String... expectedWarnings) { + final List actualWarnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER); + assertThat(actualWarnings, hasSize(expectedWarnings.length)); + for (String msg : expectedWarnings) { + assertThat(actualWarnings, hasItem(equalTo(msg))); } // "clear" current warning headers by setting a new ThreadContext DeprecationLogger.removeThreadContext(this.threadContext);