From ae4caff20f36c124bfd6847bdf82ee92a452ca85 Mon Sep 17 00:00:00 2001 From: javanna Date: Sat, 10 Dec 2016 23:55:19 +0100 Subject: [PATCH 1/2] 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 | 39 +++++++------- .../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, 172 insertions(+), 230 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/ParseField.java b/core/src/main/java/org/elasticsearch/common/ParseField.java index 7121be7d1d880..fc9377eeb2f20 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 ParseField withAllDeprecated(String allReplacedWith) { /** * @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 @@ boolean match(String fieldName, boolean strict) { // 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 9866694a230ef..a7d412398e5b7 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 @@ 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 475848989d443..0eb21be486240 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.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 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 static void parseField(FieldMapper.Builder builder, String name, Map 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 1f348000ee4cc..6ae7b3c230fdf 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 @@ 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 void testParse() { 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 void testAllDeprecated() { 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 46c0ba3572342..43c421c5f0e89 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.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.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 void setTest(int test) { } } - public void testDeprecationFail() throws IOException { - XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"old_test\" : \"foo\"}"); - class TestStruct { - public String test; - } - ObjectParser objectParser = new ObjectParser<>("foo"); - TestStruct s = new TestStruct(); + 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); + 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"); + 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); } - 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 49266ebe9fd04..e1757756e35d6 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 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 void testToQuery() throws IOException { public void testExceptionOnMissingTypes() throws IOException { assumeTrue("test runs only when at least a type is registered", getCurrentTypes().length == 0); - QueryShardException e = expectThrows(QueryShardException.class, () -> 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 void testFromJson() throws IOException { assertEquals(json, GeoExecType.MEMORY, parsed.type()); } - public void testFromJsonCoerceFails() throws IOException { + public void testFromJsonCoerceIsDeprecated() throws IOException { String json = "{\n" + " \"geo_bounding_box\" : {\n" + @@ -426,11 +420,12 @@ public void testFromJsonCoerceFails() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testFromJsonIgnoreMalformedFails() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 6c92fde684315..7b317ae85aba1 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 void testFromJson() throws IOException { assertEquals(json, 12000.0, parsed.distance(), 0.0001); } - public void testOptimizeBboxFails() throws IOException { + public void testOptimizeBboxIsDeprecated() throws IOException { String json = "{\n" + " \"geo_distance\" : {\n" + @@ -329,11 +329,12 @@ public void testOptimizeBboxFails() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testFromCoerceFails() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testFromJsonIgnoreMalformedFails() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 b77ff3bbdef88..5aff43291e20f 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 void testDeprecatedXContent() throws IOException { builder.field("normalize", true); // deprecated builder.endObject(); builder.endObject(); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testFromJsonIgnoreMalformedDeprecated() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testFromJsonCoerceDeprecated() throws IOException { " \"boost\" : 1.0\n" + " }\n" + "}"; - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 ca402782e8c50..9250467e61859 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 void testDeprecatedXContent() throws IOException { builder.field("type", "foo"); // deprecated builder.endObject(); builder.endObject(); - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 5913a03866155..f711c7fb8f179 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 void testFromJsonDeprecatedSyntax() throws IOException { 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 void testFromJsonDeprecatedSyntax() throws IOException { 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 a32eafd850c08..6e19ad4094090 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 void testLegacyMatchPhrasePrefixQuery() throws IOException { assertSerialization(qb); - checkWarningHeaders("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", + assertWarningHeaders("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", "Deprecated field [slop] used, replaced by [match_phrase query]"); - - // Now check with strict parsing an exception is thrown - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testLegacyMatchPhraseQuery() throws IOException { checkGeneratedJson(json, qb); assertEquals(json, expectedQB, qb); - assertSerialization(qb); - - checkWarningHeaders("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", + assertWarningHeaders("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", "Deprecated field [slop] used, replaced by [match_phrase query]"); - - // Now check with strict parsing an exception is thrown - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 09627d00d76d9..ede35d35021a1 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.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.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 void testNamedQueryParsing() throws IOException { " }\n" + "}"; - // non strict parsing should accept "_name" on top level - assertNotNull(parseQuery(json, ParseFieldMatcher.EMPTY)); - - // with strict parsing, ParseField will throw exception - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> 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 d545a082b558b..c3b1b22ea4a18 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.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.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 static void afterClass() throws Exception { 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 @@ protected static QueryBuilder randomNestedFilter() { @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 131d19f600db8..977b4b3cad4c1 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 void testCoerceIsDeprecated() throws IOException { " } ],\n" + " \"unit\" : \"m\",\n" + " \"distance_type\" : \"sloppy_arc\",\n" + - " \"mode\" : \"SUM\",\n" + + " \"mode\" : \"MAX\",\n" + " \"coerce\" : true\n" + "}"; XContentParser itemParser = createParser(JsonXContent.jsonXContent, json); itemParser.nextToken(); - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.STRICT); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testIgnoreMalformedIsDeprecated() throws IOException { " } ],\n" + " \"unit\" : \"m\",\n" + " \"distance_type\" : \"sloppy_arc\",\n" + - " \"mode\" : \"SUM\",\n" + + " \"mode\" : \"MAX\",\n" + " \"ignore_malformed\" : true\n" + "}"; XContentParser itemParser = createParser(JsonXContent.jsonXContent, json); itemParser.nextToken(); - QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.STRICT); - - IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> 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 void testGeoDistanceSortDeprecatedSortModeException() throws Exception { sortBuilder.field("unit", "km"); sortBuilder.field("sort_mode", "max"); sortBuilder.endObject(); - IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> 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 6a936260ca575..701ee8d984fd3 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 void testSimple() throws IOException { + "\"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 void testParseTemplateAsSingleStringWithConditionalClause() throws IOExce + " }" + "}"; 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 34e06410a7808..5f2ad37c2a6f5 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 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 IndexFieldDataService fieldData() { } @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 @@ protected Query rewrite(Query query) throws IOException { 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); From 26867c12e4f1995326447e556865e131e0880bb9 Mon Sep 17 00:00:00 2001 From: javanna Date: Thu, 15 Dec 2016 10:13:03 +0100 Subject: [PATCH 2/2] [TEST] add warnings check to ESTestCase We are currenlty checking that no deprecation warnings are emitted in our query tests. That can be moved to ESTestCase (disabled in ESIntegTestCase) as it allows us to easily catch where our tests use deprecated features and assert on the expected warnings. --- .../common/logging/DeprecationLogger.java | 5 +- .../indices/template/BWCTemplateTests.java | 2 + .../elasticsearch/common/ParseFieldTests.java | 36 ++++------- .../logging/DeprecationLoggerTests.java | 14 ++-- .../common/xcontent/ObjectParserTests.java | 33 +++------- .../index/analysis/AnalysisRegistryTests.java | 8 ++- .../index/mapper/DynamicTemplateTests.java | 3 +- .../index/mapper/MapperServiceTests.java | 7 +- .../GeoBoundingBoxQueryBuilderTests.java | 4 +- .../query/GeoDistanceQueryBuilderTests.java | 6 +- .../query/GeoPolygonQueryBuilderTests.java | 6 +- .../query/HasParentQueryBuilderTests.java | 3 +- .../index/query/IdsQueryBuilderTests.java | 4 +- .../index/query/MatchQueryBuilderTests.java | 4 +- .../index/query/RangeQueryBuilderTests.java | 2 +- .../indices/analysis/AnalysisModuleTests.java | 16 +++++ .../builder/SearchSourceBuilderTests.java | 1 + .../search/sort/AbstractSortTestCase.java | 42 ------------ .../sort/GeoDistanceSortBuilderTests.java | 6 +- .../script/mustache/SearchTemplateIT.java | 64 +++++++++---------- .../mustache/TemplateQueryBuilderTests.java | 9 +-- .../file/FileBasedDiscoveryPluginTests.java | 10 ++- .../repositories/s3/S3RepositoryTests.java | 9 ++- .../common/logging/EvilLoggerTests.java | 1 + .../test/AbstractQueryTestCase.java | 35 ---------- .../elasticsearch/test/ESIntegTestCase.java | 7 ++ .../org/elasticsearch/test/ESTestCase.java | 51 +++++++++++++++ 27 files changed, 190 insertions(+), 198 deletions(-) diff --git a/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java index ace0569a14a4e..55f89ce84ad5f 100644 --- a/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java +++ b/core/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java @@ -21,7 +21,6 @@ import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.message.ParameterizedMessage; import org.elasticsearch.common.SuppressLoggerChecks; import org.elasticsearch.common.util.concurrent.ThreadContext; @@ -42,7 +41,7 @@ public class DeprecationLogger { * * https://tools.ietf.org/html/rfc7234#section-5.5 */ - public static final String DEPRECATION_HEADER = "Warning"; + public static final String WARNING_HEADER = "Warning"; /** * This is set once by the {@code Node} constructor, but it uses {@link CopyOnWriteArraySet} to ensure that tests can run in parallel. @@ -128,7 +127,7 @@ void deprecated(Set threadContexts, String message, Object... par while (iterator.hasNext()) { try { - iterator.next().addResponseHeader(DEPRECATION_HEADER, formattedMessage); + iterator.next().addResponseHeader(WARNING_HEADER, formattedMessage); } catch (IllegalStateException e) { // ignored; it should be removed shortly } diff --git a/core/src/test/java/org/elasticsearch/action/admin/indices/template/BWCTemplateTests.java b/core/src/test/java/org/elasticsearch/action/admin/indices/template/BWCTemplateTests.java index 69c6731aa1537..7ea103313fe83 100644 --- a/core/src/test/java/org/elasticsearch/action/admin/indices/template/BWCTemplateTests.java +++ b/core/src/test/java/org/elasticsearch/action/admin/indices/template/BWCTemplateTests.java @@ -42,12 +42,14 @@ public void testBeatsTemplatesBWC() throws Exception { client().prepareIndex("packetbeat-foo", "doc", "1").setSource("message", "foo").get(); client().prepareIndex("filebeat-foo", "doc", "1").setSource("message", "foo").get(); client().prepareIndex("winlogbeat-foo", "doc", "1").setSource("message", "foo").get(); + assertWarnings("Deprecated field [template] used, replaced by [index_patterns]"); } public void testLogstashTemplatesBWC() throws Exception { String ls5x = copyToStringFromClasspath("/org/elasticsearch/action/admin/indices/template/logstash-5.0.template.json"); client().admin().indices().preparePutTemplate("logstash-5x").setSource(ls5x).get(); client().prepareIndex("logstash-foo", "doc", "1").setSource("message", "foo").get(); + assertWarnings("Deprecated field [template] used, replaced by [index_patterns]"); } } diff --git a/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java b/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java index 6ae7b3c230fdf..59b37f9e9e696 100644 --- a/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java +++ b/core/src/test/java/org/elasticsearch/common/ParseFieldTests.java @@ -20,13 +20,15 @@ import org.elasticsearch.test.ESTestCase; +import java.io.IOException; + import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.sameInstance; import static org.hamcrest.collection.IsArrayContainingInAnyOrder.arrayContainingInAnyOrder; public class ParseFieldTests extends ESTestCase { - public void testParse() { + public void testParse() throws IOException { String name = "foo_bar"; ParseField field = new ParseField(name); String[] deprecated = new String[]{"barFoo", "bar_foo", "Foobar"}; @@ -42,33 +44,21 @@ public void testParse() { assertThat(withDeprecations.match("foo bar"), is(false)); for (String deprecatedName : deprecated) { assertThat(withDeprecations.match(deprecatedName), is(true)); + assertWarnings("Deprecated field [" + deprecatedName + "] used, expected [foo_bar] instead"); } } - public void testAllDeprecated() { + public void testAllDeprecated() throws IOException { String name = "like_text"; - - boolean withDeprecatedNames = randomBoolean(); String[] deprecated = new String[]{"text", "same_as_text"}; - String[] allValues; - if (withDeprecatedNames) { - String[] newArray = new String[1 + deprecated.length]; - newArray[0] = name; - System.arraycopy(deprecated, 0, newArray, 1, deprecated.length); - allValues = newArray; - } else { - allValues = new String[] {name}; - } - - ParseField field; - if (withDeprecatedNames) { - field = new ParseField(name).withDeprecation(deprecated).withAllDeprecated("like"); - } else { - field = new ParseField(name).withAllDeprecated("like"); - } - - assertThat(field.match(randomFrom(allValues)), is(true)); - assertThat(field.match("not a field name"), is(false)); + ParseField field = new ParseField(name).withDeprecation(deprecated).withAllDeprecated("like"); + assertFalse(field.match("not a field name")); + assertTrue(field.match("text")); + assertWarnings("Deprecated field [text] used, replaced by [like]"); + assertTrue(field.match("same_as_text")); + assertWarnings("Deprecated field [same_as_text] used, replaced by [like]"); + assertTrue(field.match("like_text")); + assertWarnings("Deprecated field [like_text] used, replaced by [like]"); } public void testGetAllNamesIncludedDeprecated() { diff --git a/core/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java b/core/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java index f75e73ced2c8e..d0e1b807baf63 100644 --- a/core/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java +++ b/core/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java @@ -41,6 +41,12 @@ public class DeprecationLoggerTests extends ESTestCase { private final DeprecationLogger logger = new DeprecationLogger(Loggers.getLogger(getClass())); + @Override + protected boolean enableWarningsCheck() { + //this is a low level test for the deprecation logger, setup and checks are done manually + return false; + } + public void testAddsHeaderWithThreadContext() throws IOException { String msg = "A simple message [{}]"; String param = randomAsciiOfLengthBetween(1, 5); @@ -54,7 +60,7 @@ public void testAddsHeaderWithThreadContext() throws IOException { Map> responseHeaders = threadContext.getResponseHeaders(); assertEquals(1, responseHeaders.size()); - assertEquals(formatted, responseHeaders.get(DeprecationLogger.DEPRECATION_HEADER).get(0)); + assertEquals(formatted, responseHeaders.get(DeprecationLogger.WARNING_HEADER).get(0)); } } @@ -74,7 +80,7 @@ public void testAddsCombinedHeaderWithThreadContext() throws IOException { assertEquals(1, responseHeaders.size()); - List responses = responseHeaders.get(DeprecationLogger.DEPRECATION_HEADER); + List responses = responseHeaders.get(DeprecationLogger.WARNING_HEADER); assertEquals(2, responses.size()); assertEquals(formatted, responses.get(0)); @@ -93,7 +99,7 @@ public void testCanRemoveThreadContext() throws IOException { logger.deprecated(expected); Map> responseHeaders = threadContext.getResponseHeaders(); - List responses = responseHeaders.get(DeprecationLogger.DEPRECATION_HEADER); + List responses = responseHeaders.get(DeprecationLogger.WARNING_HEADER); // ensure it works (note: concurrent tests may be adding to it, but in different threads, so it should have no impact) assertThat(responses, hasSize(atLeast(1))); @@ -104,7 +110,7 @@ public void testCanRemoveThreadContext() throws IOException { logger.deprecated(unexpected); responseHeaders = threadContext.getResponseHeaders(); - responses = responseHeaders.get(DeprecationLogger.DEPRECATION_HEADER); + responses = responseHeaders.get(DeprecationLogger.WARNING_HEADER); assertThat(responses, hasSize(atLeast(1))); assertThat(responses, hasItem(expected)); 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 43c421c5f0e89..48860e008cdb0 100644 --- a/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -22,9 +22,6 @@ 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; @@ -38,8 +35,6 @@ 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 { @@ -224,26 +219,16 @@ public void setTest(int 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); + class TestStruct { + public String test; } + ObjectParser objectParser = new ObjectParser<>("foo"); + TestStruct s = new TestStruct(); + XContentParser parser = createParser(XContentType.JSON.xContent(), "{\"old_test\" : \"foo\"}"); + objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); + objectParser.parse(parser, s, () -> ParseFieldMatcher.EMPTY); + assertEquals("foo", s.test); + assertWarnings("Deprecated field [old_test] used, expected [test] instead"); } public void testFailOnValueType() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java b/core/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java index 4a5a0b9567206..dedd478e3bfc8 100644 --- a/core/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java +++ b/core/src/test/java/org/elasticsearch/index/analysis/AnalysisRegistryTests.java @@ -104,7 +104,7 @@ public void testOverrideDefaultIndexAnalyzerIsUnsupported() { assertTrue(e.getMessage().contains("[index.analysis.analyzer.default_index] is not supported")); } - public void testBackCompatOverrideDefaultIndexAnalyzer() { + public void testBackCompatOverrideDefaultIndexAnalyzer() throws IOException { Version version = VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), VersionUtils.getPreviousVersion(Version.V_5_0_0_alpha1)); Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build(); @@ -113,6 +113,8 @@ public void testBackCompatOverrideDefaultIndexAnalyzer() { assertThat(indexAnalyzers.getDefaultIndexAnalyzer().analyzer(), instanceOf(EnglishAnalyzer.class)); assertThat(indexAnalyzers.getDefaultSearchAnalyzer().analyzer(), instanceOf(StandardAnalyzer.class)); assertThat(indexAnalyzers.getDefaultSearchQuoteAnalyzer().analyzer(), instanceOf(StandardAnalyzer.class)); + assertWarnings("setting [index.analysis.analyzer.default_index] is deprecated, use [index.analysis.analyzer.default] " + + "instead for index [index]"); } public void testOverrideDefaultSearchAnalyzer() { @@ -125,7 +127,7 @@ public void testOverrideDefaultSearchAnalyzer() { assertThat(indexAnalyzers.getDefaultSearchQuoteAnalyzer().analyzer(), instanceOf(EnglishAnalyzer.class)); } - public void testBackCompatOverrideDefaultIndexAndSearchAnalyzer() { + public void testBackCompatOverrideDefaultIndexAndSearchAnalyzer() throws IOException { Version version = VersionUtils.randomVersionBetween(random(), VersionUtils.getFirstVersion(), VersionUtils.getPreviousVersion(Version.V_5_0_0_alpha1)); Settings settings = Settings.builder().put(IndexMetaData.SETTING_VERSION_CREATED, version).build(); @@ -137,6 +139,8 @@ public void testBackCompatOverrideDefaultIndexAndSearchAnalyzer() { assertThat(indexAnalyzers.getDefaultIndexAnalyzer().analyzer(), instanceOf(EnglishAnalyzer.class)); assertThat(indexAnalyzers.getDefaultSearchAnalyzer().analyzer(), instanceOf(EnglishAnalyzer.class)); assertThat(indexAnalyzers.getDefaultSearchQuoteAnalyzer().analyzer(), instanceOf(EnglishAnalyzer.class)); + assertWarnings("setting [index.analysis.analyzer.default_index] is deprecated, use [index.analysis.analyzer.default] " + + "instead for index [index]"); } public void testConfigureCamelCaseTokenFilter() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/index/mapper/DynamicTemplateTests.java b/core/src/test/java/org/elasticsearch/index/mapper/DynamicTemplateTests.java index f41b36068ad9f..24023281f5e7e 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/DynamicTemplateTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/DynamicTemplateTests.java @@ -26,6 +26,7 @@ import org.elasticsearch.index.mapper.DynamicTemplate.XContentFieldType; import org.elasticsearch.test.ESTestCase; +import java.io.IOException; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -43,7 +44,7 @@ public void testParseUnknownParam() throws Exception { assertEquals("Illegal dynamic template parameter: [random_param]", e.getMessage()); } - public void testParseUnknownMatchType() { + public void testParseUnknownMatchType() throws IOException { Map templateDef = new HashMap<>(); templateDef.put("match_mapping_type", "short"); templateDef.put("mapping", Collections.singletonMap("store", true)); diff --git a/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java b/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java index 42e880151693d..59bda1c877976 100644 --- a/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java +++ b/core/src/test/java/org/elasticsearch/index/mapper/MapperServiceTests.java @@ -160,12 +160,13 @@ public void testMappingDepthExceedsLimit() throws Throwable { assertThat(e.getMessage(), containsString("Limit of mapping depth [1] in index [test1] has been exceeded")); } - public void testUnmappedFieldType() { + public void testUnmappedFieldType() throws IOException { MapperService mapperService = createIndex("index").mapperService(); assertThat(mapperService.unmappedFieldType("keyword"), instanceOf(KeywordFieldType.class)); assertThat(mapperService.unmappedFieldType("long"), instanceOf(NumberFieldType.class)); // back compat assertThat(mapperService.unmappedFieldType("string"), instanceOf(KeywordFieldType.class)); + assertWarnings("[unmapped_type:string] should be replaced with [unmapped_type:keyword]"); } public void testMergeWithMap() throws Throwable { @@ -206,9 +207,7 @@ public void testOtherDocumentMappersOnlyUpdatedWhenChangingFieldType() throws IO .startObject("properties") .startObject("field") .field("type", "text") - .startObject("norms") - .field("enabled", false) - .endObject() + .field("norms", false) .endObject() .endObject().endObject().bytes()); 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 e1757756e35d6..b2d1b957befcb 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilderTests.java @@ -422,7 +422,7 @@ public void testFromJsonCoerceIsDeprecated() throws IOException { "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); + assertWarnings("Deprecated field [coerce] used, replaced by [validation_method]"); } public void testFromJsonIgnoreMalformedIsDeprecated() throws IOException { @@ -440,7 +440,7 @@ public void testFromJsonIgnoreMalformedIsDeprecated() throws IOException { " }\n" + "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); + assertWarnings("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 7b317ae85aba1..dec1ee0a7df22 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoDistanceQueryBuilderTests.java @@ -330,7 +330,7 @@ public void testOptimizeBboxIsDeprecated() throws IOException { " }\n" + "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [optimize_bbox] used, replaced by [no replacement: " + + assertWarnings("Deprecated field [optimize_bbox] used, replaced by [no replacement: " + "`optimize_bbox` is no longer supported due to recent improvements]"); } @@ -347,7 +347,7 @@ public void testFromCoerceIsDeprecated() throws IOException { " }\n" + "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); + assertWarnings("Deprecated field [coerce] used, replaced by [validation_method]"); } public void testFromJsonIgnoreMalformedIsDeprecated() throws IOException { @@ -363,7 +363,7 @@ public void testFromJsonIgnoreMalformedIsDeprecated() throws IOException { " }\n" + "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); + assertWarnings("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 5aff43291e20f..6df61d8cb451d 100644 --- a/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/GeoPolygonQueryBuilderTests.java @@ -140,7 +140,7 @@ public void testDeprecatedXContent() throws IOException { builder.endObject(); builder.endObject(); parseQuery(builder.string()); - assertWarningHeaders("Deprecated field [normalize] used, replaced by [validation_method]"); + assertWarnings("Deprecated field [normalize] used, replaced by [validation_method]"); } public void testParsingAndToQueryParsingExceptions() throws IOException { @@ -266,7 +266,7 @@ public void testFromJsonIgnoreMalformedDeprecated() throws IOException { " }\n" + "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); + assertWarnings("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); } public void testFromJsonCoerceDeprecated() throws IOException { @@ -282,7 +282,7 @@ public void testFromJsonCoerceDeprecated() throws IOException { " }\n" + "}"; parseQuery(json); - assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); + assertWarnings("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 9250467e61859..cf4b0617ea98e 100644 --- a/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/HasParentQueryBuilderTests.java @@ -23,7 +23,6 @@ import org.apache.lucene.search.Query; import org.apache.lucene.search.join.ScoreMode; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingRequest; -import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.compress.CompressedXContent; import org.elasticsearch.common.xcontent.ToXContent; import org.elasticsearch.common.xcontent.XContentBuilder; @@ -151,7 +150,7 @@ public void testDeprecatedXContent() throws IOException { builder.endObject(); HasParentQueryBuilder queryBuilder = (HasParentQueryBuilder) parseQuery(builder.string()); assertEquals("foo", queryBuilder.type()); - assertWarningHeaders("Deprecated field [type] used, expected [parent_type] instead"); + assertWarnings("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 f711c7fb8f179..de7762246ece3 100644 --- a/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/IdsQueryBuilderTests.java @@ -165,7 +165,7 @@ public void testFromJsonDeprecatedSyntax() throws IOException { assertEquals(testQuery, parsed); parseQuery(contentString); - assertWarningHeaders("Deprecated field [_type] used, expected [type] instead"); + assertWarnings("Deprecated field [_type] used, expected [type] instead"); //array of types can also be called types rather than type final String contentString2 = "{\n" + @@ -178,6 +178,6 @@ public void testFromJsonDeprecatedSyntax() throws IOException { assertEquals(testQuery, parsed); parseQuery(contentString2); - assertWarningHeaders("Deprecated field [types] used, expected [type] instead"); + assertWarnings("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 6e19ad4094090..a5684863e51dd 100644 --- a/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/MatchQueryBuilderTests.java @@ -318,7 +318,7 @@ public void testLegacyMatchPhrasePrefixQuery() throws IOException { assertSerialization(qb); - assertWarningHeaders("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", + assertWarnings("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", "Deprecated field [slop] used, replaced by [match_phrase query]"); } @@ -347,7 +347,7 @@ public void testLegacyMatchPhraseQuery() throws IOException { assertEquals(json, expectedQB, qb); assertSerialization(qb); - assertWarningHeaders("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", + assertWarnings("Deprecated field [type] used, replaced by [match_phrase and match_phrase_prefix query]", "Deprecated field [slop] used, replaced by [match_phrase query]"); } 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 ede35d35021a1..b48179e0a733a 100644 --- a/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/index/query/RangeQueryBuilderTests.java @@ -387,7 +387,7 @@ public void testNamedQueryParsing() throws IOException { "}"; assertNotNull(parseQuery(deprecatedJson)); - assertWarningHeaders("Deprecated field [_name] used, replaced by [query name is not supported in short version of range query]"); + assertWarnings("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/indices/analysis/AnalysisModuleTests.java b/core/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java index 6f379e48baefb..b568ae9276d43 100644 --- a/core/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java +++ b/core/src/test/java/org/elasticsearch/indices/analysis/AnalysisModuleTests.java @@ -136,6 +136,10 @@ public void testAnalyzerAlias() throws IOException { IndexAnalyzers indexAnalyzers = getIndexAnalyzers(newRegistry, settings); assertThat(indexAnalyzers.get("default").analyzer(), is(instanceOf(KeywordAnalyzer.class))); assertThat(indexAnalyzers.get("default_search").analyzer(), is(instanceOf(EnglishAnalyzer.class))); + assertWarnings("setting [index.analysis.analyzer.foobar.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices.", + "setting [index.analysis.analyzer.foobar_search.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices."); } public void testAnalyzerAliasReferencesAlias() throws IOException { @@ -154,6 +158,10 @@ public void testAnalyzerAliasReferencesAlias() throws IOException { assertThat(indexAnalyzers.get("default").analyzer(), is(instanceOf(GermanAnalyzer.class))); // analyzer types are bound early before we resolve aliases assertThat(indexAnalyzers.get("default_search").analyzer(), is(instanceOf(StandardAnalyzer.class))); + assertWarnings("setting [index.analysis.analyzer.foobar.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices.", + "setting [index.analysis.analyzer.foobar_search.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices."); } public void testAnalyzerAliasDefault() throws IOException { @@ -168,6 +176,8 @@ public void testAnalyzerAliasDefault() throws IOException { IndexAnalyzers indexAnalyzers = getIndexAnalyzers(newRegistry, settings); assertThat(indexAnalyzers.get("default").analyzer(), is(instanceOf(KeywordAnalyzer.class))); assertThat(indexAnalyzers.get("default_search").analyzer(), is(instanceOf(KeywordAnalyzer.class))); + assertWarnings("setting [index.analysis.analyzer.foobar.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices."); } public void testAnalyzerAliasMoreThanOnce() throws IOException { @@ -183,6 +193,10 @@ public void testAnalyzerAliasMoreThanOnce() throws IOException { AnalysisRegistry newRegistry = getNewRegistry(settings); IllegalStateException ise = expectThrows(IllegalStateException.class, () -> getIndexAnalyzers(newRegistry, settings)); assertEquals("alias [default] is already used by [foobar]", ise.getMessage()); + assertWarnings("setting [index.analysis.analyzer.foobar.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices.", + "setting [index.analysis.analyzer.foobar1.alias] is only allowed on index [test] because it was created before " + + "5.x; analyzer aliases can no longer be created on new indices."); } public void testAnalyzerAliasNotAllowedPost5x() throws IOException { @@ -353,6 +367,8 @@ public void testUnderscoreInAnalyzerNameAlias() throws IOException { } catch (IllegalArgumentException e) { assertThat(e.getMessage(), equalTo("analyzer name must not start with '_'. got \"_invalid_name\"")); } + assertWarnings("setting [index.analysis.analyzer.valid_name.alias] is only allowed on index [test] because it was " + + "created before 5.x; analyzer aliases can no longer be created on new indices."); } public void testDeprecatedPositionOffsetGap() throws IOException { diff --git a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java index 8ec873704d617..7817a9ed32a4e 100644 --- a/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/builder/SearchSourceBuilderTests.java @@ -324,6 +324,7 @@ public void testParseIndicesBoost() throws IOException { assertEquals(2, searchSourceBuilder.indexBoosts().size()); assertEquals(new SearchSourceBuilder.IndexBoost("foo", 1.0f), searchSourceBuilder.indexBoosts().get(0)); assertEquals(new SearchSourceBuilder.IndexBoost("bar", 2.0f), searchSourceBuilder.indexBoosts().get(1)); + assertWarnings("Object format in indices_boost is deprecated, please use array format instead"); } } 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 c3b1b22ea4a18..452d914bb866a 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java +++ b/core/src/test/java/org/elasticsearch/search/sort/AbstractSortTestCase.java @@ -25,9 +25,7 @@ import org.elasticsearch.cluster.metadata.IndexMetaData; import org.elasticsearch.common.ParseFieldMatcher; import org.elasticsearch.common.io.stream.NamedWriteableRegistry; -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; @@ -67,22 +65,16 @@ 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 { @@ -91,7 +83,6 @@ 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 { @@ -123,39 +114,6 @@ public static void afterClass() throws Exception { 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(); 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 977b4b3cad4c1..ec3d2a01754ca 100644 --- a/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java +++ b/core/src/test/java/org/elasticsearch/search/sort/GeoDistanceSortBuilderTests.java @@ -275,7 +275,7 @@ public void testCoerceIsDeprecated() throws IOException { QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.EMPTY); GeoDistanceSortBuilder.fromXContent(context, ""); - assertWarningHeaders("Deprecated field [coerce] used, replaced by [validation_method]"); + assertWarnings("Deprecated field [coerce] used, replaced by [validation_method]"); } public void testIgnoreMalformedIsDeprecated() throws IOException { @@ -294,7 +294,7 @@ public void testIgnoreMalformedIsDeprecated() throws IOException { QueryParseContext context = new QueryParseContext(indicesQueriesRegistry, itemParser, ParseFieldMatcher.EMPTY); GeoDistanceSortBuilder.fromXContent(context, ""); - assertWarningHeaders("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); + assertWarnings("Deprecated field [ignore_malformed] used, replaced by [validation_method]"); } public void testSortModeSumIsRejectedInJSON() throws IOException { @@ -452,7 +452,7 @@ public void testGeoDistanceSortDeprecatedSortModeException() throws Exception { sortBuilder.field("sort_mode", "max"); sortBuilder.endObject(); parse(sortBuilder); - assertWarningHeaders("Deprecated field [sort_mode] used, expected [mode] instead"); + assertWarnings("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/SearchTemplateIT.java b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java index 5bed13c78dce9..09e160f34f713 100644 --- a/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java +++ b/modules/lang-mustache/src/test/java/org/elasticsearch/script/mustache/SearchTemplateIT.java @@ -294,6 +294,7 @@ public void testIndexedTemplate() throws Exception { SearchResponse sr = client().prepareSearch().setQuery(builder) .execute().actionGet(); assertHitCount(sr, 1); + assertWarnings("[template] query is deprecated, use search template api instead"); } // Relates to #10397 @@ -306,39 +307,36 @@ public void testIndexedTemplateOverwrite() throws Exception { .get(); client().admin().indices().prepareRefresh().get(); - int iterations = randomIntBetween(2, 11); - for (int i = 1; i < iterations; i++) { - assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) - .setId("git01") - .setSource(new BytesArray("{\"template\":{\"query\": {\"match\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\"," + - "\"type\": \"ooophrase_prefix\"}}}}}"))); - - GetStoredScriptResponse getResponse = client().admin().cluster() - .prepareGetStoredScript(MustacheScriptEngineService.NAME, "git01").get(); - assertNotNull(getResponse.getStoredScript()); - - Map templateParams = new HashMap<>(); - templateParams.put("P_Keyword1", "dev"); - - ParsingException e = expectThrows(ParsingException.class, () -> new SearchTemplateRequestBuilder(client()) - .setRequest(new SearchRequest("testindex").types("test")) - .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) - .get()); - assertThat(e.getMessage(), containsString("[match] query does not support type ooophrase_prefix")); - - assertAcked(client().admin().cluster().preparePutStoredScript() - .setScriptLang(MustacheScriptEngineService.NAME) - .setId("git01") - .setSource(new BytesArray("{\"query\": {\"match\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\"," + - "\"type\": \"phrase_prefix\"}}}}"))); - - SearchTemplateResponse searchResponse = new SearchTemplateRequestBuilder(client()) - .setRequest(new SearchRequest("testindex").types("test")) - .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) - .get(); - assertHitCount(searchResponse.getResponse(), 1); - } + assertAcked(client().admin().cluster().preparePutStoredScript() + .setScriptLang(MustacheScriptEngineService.NAME) + .setId("git01") + .setSource(new BytesArray("{\"template\":{\"query\": {\"match_phrase_prefix\": " + + "{\"searchtext\": {\"query\": \"{{P_Keyword1}}\"," + + "\"unsupported\": \"unsupported\"}}}}}"))); + + GetStoredScriptResponse getResponse = client().admin().cluster() + .prepareGetStoredScript(MustacheScriptEngineService.NAME, "git01").get(); + assertNotNull(getResponse.getStoredScript()); + + Map templateParams = new HashMap<>(); + templateParams.put("P_Keyword1", "dev"); + + ParsingException e = expectThrows(ParsingException.class, () -> new SearchTemplateRequestBuilder(client()) + .setRequest(new SearchRequest("testindex").types("test")) + .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) + .get()); + assertThat(e.getMessage(), containsString("[match_phrase_prefix] query does not support [unsupported]")); + + assertAcked(client().admin().cluster().preparePutStoredScript() + .setScriptLang(MustacheScriptEngineService.NAME) + .setId("git01") + .setSource(new BytesArray("{\"query\": {\"match_phrase_prefix\": {\"searchtext\": {\"query\": \"{{P_Keyword1}}\"}}}}"))); + + SearchTemplateResponse searchResponse = new SearchTemplateRequestBuilder(client()) + .setRequest(new SearchRequest("testindex").types("test")) + .setScript("git01").setScriptType(ScriptType.STORED).setScriptParams(templateParams) + .get(); + assertHitCount(searchResponse.getResponse(), 1); } public void testIndexedTemplateWithArray() throws Exception { 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 5f2ad37c2a6f5..4334de090c20e 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 @@ -59,11 +59,12 @@ public class TemplateQueryBuilderTests extends AbstractQueryTestCase> extends ESTestCase { @@ -156,11 +152,6 @@ public abstract class AbstractQueryTestCase> private static String[] currentTypes; private static String[] randomTypes; - /** - * used to check warning headers of the deprecation logger - */ - private ThreadContext threadContext; - protected static Index getIndex() { return index; } @@ -213,8 +204,6 @@ public void beforeTest() throws IOException { serviceHolder = new ServiceHolder(nodeSettings, indexSettings, getPlugins(), this); } serviceHolder.clientInvocationHandler.delegate = this; - this.threadContext = new ThreadContext(Settings.EMPTY); - DeprecationLogger.setThreadContext(threadContext); } private static SearchContext getSearchContext(String[] types, QueryShardContext context) { @@ -235,13 +224,6 @@ public IndexFieldDataService fieldData() { @After 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; } @@ -1020,23 +1002,6 @@ protected Query rewrite(Query query) throws IOException { return query; } - 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); - } - private static class ServiceHolder implements Closeable { private final IndicesQueriesRegistry indicesQueriesRegistry; diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index cb31ac6028bbf..45ae84d7ee3f6 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -339,6 +339,13 @@ public static void beforeClass() throws Exception { initializeSuiteScope(); } + @Override + protected final boolean enableWarningsCheck() { + //In an integ test it doesn't make sense to keep track of warnings: if the cluster is external the warnings are in another jvm, + //if the cluster is internal the deprecation logger is shared across all nodes + return false; + } + protected final void beforeInternal() throws Exception { final Scope currentClusterScope = getCurrentClusterScope(); switch (currentClusterScope) { diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java index 9a4085ff2e4a3..1002d66c5d0cc 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -57,12 +57,14 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.Writeable; +import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.common.logging.Loggers; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.transport.TransportAddress; import org.elasticsearch.common.util.MockBigArrays; import org.elasticsearch.common.util.MockPageCacheRecycler; import org.elasticsearch.common.xcontent.XContent; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; import org.elasticsearch.common.xcontent.XContentParser; @@ -129,6 +131,8 @@ import static org.elasticsearch.common.util.CollectionUtils.arrayAsArrayList; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; /** * Base testcase for randomized unit testing with Elasticsearch @@ -173,6 +177,7 @@ public static void resetPortCounter() { } protected final Logger logger = Loggers.getLogger(getClass()); + private ThreadContext threadContext; // ----------------------------------------------------------------- // Suite and test case setup/cleanup. @@ -251,16 +256,62 @@ public static void restoreContentType() { @Before public final void before() { logger.info("[{}]: before test", getTestName()); + if (enableWarningsCheck()) { + this.threadContext = new ThreadContext(Settings.EMPTY); + DeprecationLogger.setThreadContext(threadContext); + } + } + + /** + * Whether or not we check after each test whether it has left warnings behind. That happens if any deprecated feature or syntax + * was used by the test and the test didn't assert on it using {@link #assertWarnings(String...)}. + */ + protected boolean enableWarningsCheck() { + return true; } @After public final void after() throws Exception { checkStaticState(); + if (enableWarningsCheck()) { + ensureNoWarnings(); + } ensureAllSearchContextsReleased(); ensureCheckIndexPassed(); logger.info("[{}]: after test", getTestName()); } + private void ensureNoWarnings() throws IOException { + //Check that there are no unaccounted warning headers. These should be checked with {@link #checkWarningHeaders(String...)} in the + //appropriate test + try { + final List warnings = threadContext.getResponseHeaders().get(DeprecationLogger.WARNING_HEADER); + assertNull("unexpected warning headers", warnings); + } finally { + DeprecationLogger.removeThreadContext(this.threadContext); + this.threadContext.close(); + } + } + + protected final void assertWarnings(String... expectedWarnings) throws IOException { + if (enableWarningsCheck() == false) { + throw new IllegalStateException("unable to check warning headers if the test is not set to do so"); + } + try { + final List actualWarnings = threadContext.getResponseHeaders().get(DeprecationLogger.WARNING_HEADER); + assertThat(actualWarnings, hasSize(expectedWarnings.length)); + for (String msg : expectedWarnings) { + assertThat(actualWarnings, hasItem(equalTo(msg))); + } + } finally { + // "clear" current warning headers by setting a new ThreadContext + DeprecationLogger.removeThreadContext(this.threadContext); + this.threadContext.close(); + this.threadContext = new ThreadContext(Settings.EMPTY); + DeprecationLogger.setThreadContext(this.threadContext); + } + } + private static final List statusData = new ArrayList<>(); static { // ensure that the status logger is set to the warn level so we do not miss any warnings with our Log4j usage