From 68eaab5b327ace67f3c5548b0c0d1d467011c71b Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 17:12:39 +0000 Subject: [PATCH 01/12] Report parser name and location in XContent deprecation warnings --- .../org/elasticsearch/common/ParseField.java | 24 ++++++++-- .../common/xcontent/DeprecationHandler.java | 47 +++++++++++++------ .../common/xcontent/ObjectParser.java | 2 +- .../common/xcontent/XContentLocation.java | 5 +- .../elasticsearch/common/ParseFieldTests.java | 2 +- .../common/xcontent/ObjectParserTests.java | 2 +- .../xcontent/LoggingDeprecationHandler.java | 30 +++++++++--- .../index/query/BoolQueryBuilderTests.java | 2 +- .../script/StoredScriptTests.java | 2 +- ...LoggingDeprecationAccumulationHandler.java | 41 +++++++++++----- .../utils/XContentObjectTransformerTests.java | 4 +- .../inference/ingest/InferenceProcessor.java | 2 +- .../xpack/security/authc/ApiKeyService.java | 36 ++++++++++---- 13 files changed, 145 insertions(+), 54 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java b/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java index f4f12ed4f4c81..3872ef4852274 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java @@ -19,11 +19,13 @@ package org.elasticsearch.common; import org.elasticsearch.common.xcontent.DeprecationHandler; +import org.elasticsearch.common.xcontent.XContentLocation; import java.util.Collections; import java.util.HashSet; import java.util.Objects; import java.util.Set; +import java.util.function.Supplier; /** * Holds a field that can be found in a request while parsing and its different @@ -115,6 +117,22 @@ public ParseField withAllDeprecated() { * names for this {@link ParseField}. */ public boolean match(String fieldName, DeprecationHandler deprecationHandler) { + return match(null, () -> XContentLocation.UNKNOWN, fieldName, deprecationHandler); + } + + /** + * Does {@code fieldName} match this field? + * @param parserName + * the name of the parent object holding this field + * @param location + * the XContentLocation of the field + * @param fieldName + * the field name to match against this {@link ParseField} + * @param deprecationHandler called if {@code fieldName} is deprecated + * @return true if fieldName matches any of the acceptable + * names for this {@link ParseField}. + */ + public boolean match(String parserName, Supplier location, String fieldName, DeprecationHandler deprecationHandler) { Objects.requireNonNull(fieldName, "fieldName cannot be null"); // if this parse field has not been completely deprecated then try to // match the preferred name @@ -127,11 +145,11 @@ public boolean match(String fieldName, DeprecationHandler deprecationHandler) { for (String depName : deprecatedNames) { if (fieldName.equals(depName)) { if (fullyDeprecated) { - deprecationHandler.usedDeprecatedField(fieldName); + deprecationHandler.usedDeprecatedField(parserName, location, fieldName); } else if (allReplacedWith == null) { - deprecationHandler.usedDeprecatedName(fieldName, name); + deprecationHandler.usedDeprecatedName(parserName, location, fieldName, name); } else { - deprecationHandler.usedDeprecatedField(fieldName, allReplacedWith); + deprecationHandler.usedDeprecatedField(parserName, location, fieldName, allReplacedWith); } return true; } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java index 2cfda82eb4c48..c524403092943 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java @@ -19,6 +19,8 @@ package org.elasticsearch.common.xcontent; +import java.util.function.Supplier; + /** * Callback for notifying the creator of the {@link XContentParser} that * parsing hit a deprecated field. @@ -32,20 +34,35 @@ public interface DeprecationHandler { */ DeprecationHandler THROW_UNSUPPORTED_OPERATION = new DeprecationHandler() { @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - throw new UnsupportedOperationException("deprecated fields not supported here but got [" - + usedName + "] which is a deprecated name for [" + replacedWith + "]"); + public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { + if (parserName != null) { + throw new UnsupportedOperationException("deprecated fields not supported in [" + parserName + "] but got [" + + usedName + "] at [" + location.get() + "] which is a deprecated name for [" + replacedWith + "]"); + } else { + throw new UnsupportedOperationException("deprecated fields not supported here but got [" + + usedName + "] which is a deprecated name for [" + replacedWith + "]"); + } } @Override - public void usedDeprecatedName(String usedName, String modernName) { - throw new UnsupportedOperationException("deprecated fields not supported here but got [" - + usedName + "] which has been replaced with [" + modernName + "]"); + public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { + if (parserName != null) { + throw new UnsupportedOperationException("deprecated fields not supported in [" + parserName + "] but got [" + + usedName + "] at [" + location.get() + "] which has been replaced with [" + modernName + "]"); + } else { + throw new UnsupportedOperationException("deprecated fields not supported here but got [" + + usedName + "] which has been replaced with [" + modernName + "]"); + } } @Override - public void usedDeprecatedField(String usedName) { - throw new UnsupportedOperationException("deprecated fields not supported here but got [" - + usedName + "] which has been deprecated entirely"); + public void usedDeprecatedField(String parserName, Supplier location, String usedName) { + if (parserName != null) { + throw new UnsupportedOperationException("deprecated fields not supported in [" + parserName + "] but got [" + + usedName + "] at [" + location.get() + "] which has been deprecated entirely"); + } else { + throw new UnsupportedOperationException("deprecated fields not supported here but got [" + + usedName + "] which has been deprecated entirely"); + } } }; @@ -54,17 +71,17 @@ public void usedDeprecatedField(String usedName) { */ DeprecationHandler IGNORE_DEPRECATIONS = new DeprecationHandler() { @Override - public void usedDeprecatedName(String usedName, String modernName) { + public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { } @Override - public void usedDeprecatedField(String usedName, String replacedWith) { + public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { } @Override - public void usedDeprecatedField(String usedName) { + public void usedDeprecatedField(String parserName, Supplier location, String usedName) { } }; @@ -74,7 +91,7 @@ public void usedDeprecatedField(String usedName) { * @param usedName the provided field name * @param modernName the modern name for the field */ - void usedDeprecatedName(String usedName, String modernName); + void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName); /** * Called when the provided field name matches the current field but the entire @@ -82,12 +99,12 @@ public void usedDeprecatedField(String usedName) { * @param usedName the provided field name * @param replacedWith the name of the field that replaced this field */ - void usedDeprecatedField(String usedName, String replacedWith); + void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith); /** * Called when the provided field name matches the current field but the entire * field has been marked as deprecated with no replacement * @param usedName the provided field name */ - void usedDeprecatedField(String usedName); + void usedDeprecatedField(String parserName, Supplier location, String usedName); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java index b11f6afd4bb26..73bbf812f598c 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java @@ -571,7 +571,7 @@ private class FieldParser { } void assertSupports(String parserName, XContentParser parser, String currentFieldName) { - if (parseField.match(currentFieldName, parser.getDeprecationHandler()) == false) { + if (parseField.match(parserName, parser::getTokenLocation, currentFieldName, parser.getDeprecationHandler()) == false) { throw new XContentParseException(parser.getTokenLocation(), "[" + parserName + "] parsefield doesn't accept: " + currentFieldName); } diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java index 43ab7503cd1dd..1d5bfd6c2c429 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java @@ -25,7 +25,10 @@ * position of a parsing error to end users and consequently have line and * column numbers starting from 1. */ -public class XContentLocation { +public final class XContentLocation { + + public static final XContentLocation UNKNOWN = new XContentLocation(-1, -1); + public final int lineNumber; public final int columnNumber; diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java index 381c7c3e959be..04e3691672254 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java @@ -66,7 +66,7 @@ public void testDeprecatedWithNoReplacement() { ParseField field = new ParseField(name).withDeprecation(alternatives).withAllDeprecated(); assertFalse(field.match("not a field name", LoggingDeprecationHandler.INSTANCE)); assertTrue(field.match("dep", LoggingDeprecationHandler.INSTANCE)); - assertWarnings("Deprecated field [dep] used, which has been removed entirely"); + assertWarnings("Deprecated field [dep] used, this field is unused and will be removed entirely"); assertTrue(field.match("old_dep", LoggingDeprecationHandler.INSTANCE)); assertWarnings("Deprecated field [old_dep] used, which has been removed entirely"); assertTrue(field.match("new_dep", LoggingDeprecationHandler.INSTANCE)); diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index c99d1b10d6a4d..9a813c8a24616 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -223,7 +223,7 @@ class TestStruct { objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); objectParser.parse(parser, s, null); assertEquals("foo", s.test); - assertWarnings("Deprecated field [old_test] used, expected [test] instead"); + assertWarnings("[foo][1:15] Deprecated field [old_test] used, expected [test] instead"); } public void testFailOnValueType() throws IOException { diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java index 9a885b55d146c..4ff3502486879 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java @@ -23,6 +23,8 @@ import org.elasticsearch.common.ParseField; import org.elasticsearch.common.logging.DeprecationLogger; +import java.util.function.Supplier; + /** * Logs deprecations to the {@link DeprecationLogger}. *

@@ -49,17 +51,33 @@ private LoggingDeprecationHandler() { } @Override - public void usedDeprecatedName(String usedName, String modernName) { - deprecationLogger.deprecated("Deprecated field [{}] used, expected [{}] instead", usedName, modernName); + public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { + if (parserName != null) { + deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used, expected [{}] instead", + parserName, location.get(), usedName, modernName); + } else { + deprecationLogger.deprecated("Deprecated field [{}] used, expected [{}] instead", usedName, modernName); + } } @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - deprecationLogger.deprecated("Deprecated field [{}] used, replaced by [{}]", usedName, replacedWith); + public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { + if (parserName != null) { + deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used, replaced by [{}]", + parserName, location.get(), usedName, replacedWith); + } + else { + deprecationLogger.deprecated("Deprecated field [{}] used, replaced by [{}]", usedName, replacedWith); + } } @Override - public void usedDeprecatedField(String usedName) { - deprecationLogger.deprecated("Deprecated field [{}] used, which has been removed entirely", usedName); + public void usedDeprecatedField(String parserName, Supplier location, String usedName) { + if (parserName != null) { + deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used, this field is unused and will be removed entirely", + parserName, location.get(), usedName); + } else { + deprecationLogger.deprecated("Deprecated field [{}] used, this field is unused and will be removed entirely", usedName); + } } } diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 93d46ed592570..03916bac58a84 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -302,7 +302,7 @@ public void testDeprecation() throws IOException { QueryBuilder q = parseQuery(query); QueryBuilder expected = new BoolQueryBuilder().mustNot(new MatchAllQueryBuilder()); assertEquals(expected, q); - assertWarnings("Deprecated field [mustNot] used, expected [must_not] instead"); + assertWarnings("[bool][1:24] Deprecated field [mustNot] used, expected [must_not] instead"); } public void testRewrite() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java index 627d67dc833e4..47dc7aaae2a3b 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java @@ -104,7 +104,7 @@ public void testSourceParsing() throws Exception { assertThat(parsed, equalTo(source)); } - assertWarnings("Deprecated field [code] used, expected [source] instead"); + assertWarnings("[stored script source][1:33] Deprecated field [code] used, expected [source] instead"); // complex script with script object and empty options try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java index 5e2a4d7a25302..12a036ee66575 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java @@ -8,10 +8,12 @@ import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.xcontent.DeprecationHandler; import org.elasticsearch.common.xcontent.LoggingDeprecationHandler; +import org.elasticsearch.common.xcontent.XContentLocation; import java.util.ArrayList; import java.util.Collections; import java.util.List; +import java.util.function.Supplier; /** * Very similar to {@link org.elasticsearch.common.xcontent.LoggingDeprecationHandler} main differences are: @@ -26,24 +28,39 @@ public class LoggingDeprecationAccumulationHandler implements DeprecationHandler private final List deprecations = new ArrayList<>(); @Override - public void usedDeprecatedName(String usedName, String modernName) { - LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(usedName, modernName); - deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, expected [{}] instead", - new Object[] {usedName, modernName})); + public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { + LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(parserName, location, usedName, modernName); + if (parserName != null) { + deprecations.add(LoggerMessageFormat.format("[{}][{}] Deprecated field [{}] used, expected [{}] instead", + new Object[]{parserName, location.get(), usedName, modernName})); + } else { + deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, expected [{}] instead", + new Object[]{usedName, modernName})); + } } @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(usedName, replacedWith); - deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, replaced by [{}]", - new Object[] {usedName, replacedWith})); + public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { + LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(parserName, location, usedName, replacedWith); + if (parserName != null) { + deprecations.add(LoggerMessageFormat.format("[{}][{}] Deprecated field [{}] used, replaced by [{}]", + new Object[]{parserName, location.get(), usedName, replacedWith})); + } else { + deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, replaced by [{}]", + new Object[]{usedName, replacedWith})); + } } @Override - public void usedDeprecatedField(String usedName) { - LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(usedName); - deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, which has been deprecated entirely", - new Object[]{ usedName })); + public void usedDeprecatedField(String parserName, Supplier location, String usedName) { + LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(parserName, location, usedName); + if (parserName != null) { + deprecations.add(LoggerMessageFormat.format("[{}][{}] Deprecated field [{}] used, unused and will be removed entirely", + new Object[]{parserName, location.get(), usedName})); + } else { + deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, unused and will be removed entirely", + new Object[]{usedName})); + } } /** diff --git a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java index d6ebc9a2e150b..886602395238f 100644 --- a/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java +++ b/x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java @@ -127,8 +127,8 @@ public void testToMap() throws IOException { public void testDeprecationWarnings() throws IOException { XContentObjectTransformer queryBuilderTransformer = new XContentObjectTransformer<>(NamedXContentRegistry.EMPTY, (p)-> { - p.getDeprecationHandler().usedDeprecatedField("oldField", "newField"); - p.getDeprecationHandler().usedDeprecatedName("oldName", "modernName"); + p.getDeprecationHandler().usedDeprecatedField(null, null, "oldField", "newField"); + p.getDeprecationHandler().usedDeprecatedName(null, null, "oldName", "modernName"); return new BoolQueryBuilder(); }); List deprecations = new ArrayList<>(); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java index 2a4861c5247f7..b52b8b5b64e3f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java @@ -242,7 +242,7 @@ public InferenceProcessor create(Map processorFactori fieldMap = ConfigurationUtils.readOptionalMap(TYPE, tag, config, FIELD_MAPPINGS); //TODO Remove in 8.x if (fieldMap != null) { - LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(FIELD_MAPPINGS, FIELD_MAP); + LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(null, () -> null, FIELD_MAPPINGS, FIELD_MAP); } } InferenceConfig inferenceConfig = inferenceConfigFromMap(ConfigurationUtils.readMap(TYPE, tag, config, INFERENCE_CONFIG)); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index f36ce90660a23..44a6d4edbf9b7 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -49,6 +49,7 @@ import org.elasticsearch.common.xcontent.NamedXContentRegistry; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.elasticsearch.common.xcontent.XContentLocation; import org.elasticsearch.common.xcontent.XContentParser; import org.elasticsearch.common.xcontent.XContentType; import org.elasticsearch.index.query.BoolQueryBuilder; @@ -618,21 +619,38 @@ private ApiKeyLoggingDeprecationHandler(DeprecationLogger logger, String apiKeyI } @Override - public void usedDeprecatedName(String usedName, String modernName) { - deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], expected [{}] instead", - usedName, apiKeyId, modernName); + public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { + if (parserName != null) { + deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used in api key [{}], expected [{}] instead", + parserName, location.get(), usedName, apiKeyId, modernName); + } else { + deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], expected [{}] instead", + usedName, apiKeyId, modernName); + } } @Override - public void usedDeprecatedField(String usedName, String replacedWith) { - deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], replaced by [{}]", - usedName, apiKeyId, replacedWith); + public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { + if (parserName != null) { + deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used in api key [{}], replaced by [{}]", + parserName, location.get(), usedName, apiKeyId, replacedWith); + } else { + deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], replaced by [{}]", + usedName, apiKeyId, replacedWith); + } } @Override - public void usedDeprecatedField(String usedName) { - deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], which has been removed entirely", - usedName); + public void usedDeprecatedField(String parserName, Supplier location, String usedName) { + if (parserName != null) { + deprecationLogger.deprecated( + "[{}][{}] Deprecated field [{}] used in api key [{}], which is unused and will be removed entirely", + parserName, location.get(), usedName); + } else { + deprecationLogger.deprecated( + "[{}][{}] Deprecated field [{}] used in api key [{}], which is unused and will be removed entirely", + usedName); + } } } From 02061e507fc359f04b24d4b72e1b60d6d70368dd Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 17:46:52 +0000 Subject: [PATCH 02/12] test fix --- .../test/java/org/elasticsearch/common/ParseFieldTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java index 04e3691672254..e7420356c8fac 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java @@ -68,9 +68,9 @@ public void testDeprecatedWithNoReplacement() { assertTrue(field.match("dep", LoggingDeprecationHandler.INSTANCE)); assertWarnings("Deprecated field [dep] used, this field is unused and will be removed entirely"); assertTrue(field.match("old_dep", LoggingDeprecationHandler.INSTANCE)); - assertWarnings("Deprecated field [old_dep] used, which has been removed entirely"); + assertWarnings("Deprecated field [old_dep] used, this field is unused and will be removed entirely"); assertTrue(field.match("new_dep", LoggingDeprecationHandler.INSTANCE)); - assertWarnings("Deprecated field [new_dep] used, which has been removed entirely"); + assertWarnings("Deprecated field [new_dep] used, this field is unused and will be removed entirely"); } From 9841bbc139095d57021da1c075f4a2f0efe77420 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 17:56:52 +0000 Subject: [PATCH 03/12] warnings context for ml --- .../rest-api-spec/test/ml/data_frame_analytics_crud.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml index a5a99b30391ea..c5df3d9f6cc51 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml @@ -1869,7 +1869,7 @@ setup: - do: allowed_warnings: - - 'Deprecated field [maximum_number_trees] used, expected [max_trees] instead' + - '[classification][1:139] Deprecated field [maximum_number_trees] used, expected [max_trees] instead' ml.put_data_frame_analytics: id: "classification-with-maximum-number-trees" body: > From 7c093de45f3831d1ac16716f96069b31ebd29fc6 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 09:44:21 +0000 Subject: [PATCH 04/12] feedback, tests --- .../common/xcontent/DeprecationHandler.java | 1 + .../common/ScriptProcessorFactoryTests.java | 2 +- .../xcontent/LoggingDeprecationHandler.java | 26 ++++++--------- ...LoggingDeprecationAccumulationHandler.java | 30 +++++------------ .../xpack/security/authc/ApiKeyService.java | 33 ++++++------------- 5 files changed, 31 insertions(+), 61 deletions(-) diff --git a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java index c524403092943..65f52943f0b86 100644 --- a/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java +++ b/libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java @@ -107,4 +107,5 @@ public void usedDeprecatedField(String parserName, Supplier lo * @param usedName the provided field name */ void usedDeprecatedField(String parserName, Supplier location, String usedName); + } diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java index 9559c150df68a..c7018d40b18ca 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java @@ -98,7 +98,7 @@ public void testInlineBackcompat() throws Exception { configMap.put("inline", "code"); factory.create(null, randomAlphaOfLength(10), configMap); - assertWarnings("Deprecated field [inline] used, expected [source] instead"); + assertWarnings("[script][1:11] Deprecated field [inline] used, expected [source] instead"); } public void testFactoryInvalidateWithInvalidCompiledScript() throws Exception { diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java index 4ff3502486879..74249af951653 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java @@ -52,30 +52,24 @@ private LoggingDeprecationHandler() { @Override public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { - if (parserName != null) { - deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used, expected [{}] instead", - parserName, location.get(), usedName, modernName); - } else { - deprecationLogger.deprecated("Deprecated field [{}] used, expected [{}] instead", usedName, modernName); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecationLogger.deprecated("{}Deprecated field [{}] used, expected [{}] instead", + prefix, usedName, modernName); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { - if (parserName != null) { - deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used, replaced by [{}]", - parserName, location.get(), usedName, replacedWith); - } - else { - deprecationLogger.deprecated("Deprecated field [{}] used, replaced by [{}]", usedName, replacedWith); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecationLogger.deprecated("{} Deprecated field [{}] used, replaced by [{}]", + prefix, usedName, replacedWith); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName) { - if (parserName != null) { - deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used, this field is unused and will be removed entirely", - parserName, location.get(), usedName); + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + if (true) { + deprecationLogger.deprecated("{}Deprecated field [{}] used, this field is unused and will be removed entirely", + prefix, usedName); } else { deprecationLogger.deprecated("Deprecated field [{}] used, this field is unused and will be removed entirely", usedName); } diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java index 12a036ee66575..323e3a84cdf2e 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java @@ -30,37 +30,25 @@ public class LoggingDeprecationAccumulationHandler implements DeprecationHandler @Override public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(parserName, location, usedName, modernName); - if (parserName != null) { - deprecations.add(LoggerMessageFormat.format("[{}][{}] Deprecated field [{}] used, expected [{}] instead", - new Object[]{parserName, location.get(), usedName, modernName})); - } else { - deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, expected [{}] instead", - new Object[]{usedName, modernName})); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecations.add(LoggerMessageFormat.format("{}Deprecated field [{}] used, expected [{}] instead", + new Object[]{prefix, usedName, modernName})); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(parserName, location, usedName, replacedWith); - if (parserName != null) { - deprecations.add(LoggerMessageFormat.format("[{}][{}] Deprecated field [{}] used, replaced by [{}]", - new Object[]{parserName, location.get(), usedName, replacedWith})); - } else { - deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, replaced by [{}]", - new Object[]{usedName, replacedWith})); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecations.add(LoggerMessageFormat.format("{}Deprecated field [{}] used, replaced by [{}]", + new Object[]{prefix, usedName, replacedWith})); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName) { LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(parserName, location, usedName); - if (parserName != null) { - deprecations.add(LoggerMessageFormat.format("[{}][{}] Deprecated field [{}] used, unused and will be removed entirely", - new Object[]{parserName, location.get(), usedName})); - } else { - deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, unused and will be removed entirely", - new Object[]{usedName})); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecations.add(LoggerMessageFormat.format("{}Deprecated field [{}] used, unused and will be removed entirely", + new Object[]{prefix, usedName})); } /** diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java index 44a6d4edbf9b7..c363ad1253509 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java @@ -620,37 +620,24 @@ private ApiKeyLoggingDeprecationHandler(DeprecationLogger logger, String apiKeyI @Override public void usedDeprecatedName(String parserName, Supplier location, String usedName, String modernName) { - if (parserName != null) { - deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used in api key [{}], expected [{}] instead", - parserName, location.get(), usedName, apiKeyId, modernName); - } else { - deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], expected [{}] instead", - usedName, apiKeyId, modernName); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecationLogger.deprecated("{}Deprecated field [{}] used in api key [{}], expected [{}] instead", + prefix, usedName, apiKeyId, modernName); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { - if (parserName != null) { - deprecationLogger.deprecated("[{}][{}] Deprecated field [{}] used in api key [{}], replaced by [{}]", - parserName, location.get(), usedName, apiKeyId, replacedWith); - } else { - deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], replaced by [{}]", - usedName, apiKeyId, replacedWith); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecationLogger.deprecated("{}Deprecated field [{}] used in api key [{}], replaced by [{}]", + prefix, usedName, apiKeyId, replacedWith); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName) { - if (parserName != null) { - deprecationLogger.deprecated( - "[{}][{}] Deprecated field [{}] used in api key [{}], which is unused and will be removed entirely", - parserName, location.get(), usedName); - } else { - deprecationLogger.deprecated( - "[{}][{}] Deprecated field [{}] used in api key [{}], which is unused and will be removed entirely", - usedName); - } + String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; + deprecationLogger.deprecated( + "{}Deprecated field [{}] used in api key [{}], which is unused and will be removed entirely", + prefix, usedName); } } From 8014ecca8e94651837f0a2823aee471a8f5d3d90 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 09:59:41 +0000 Subject: [PATCH 05/12] no really --- .../common/xcontent/LoggingDeprecationHandler.java | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java index 74249af951653..5feca4bfaab5a 100644 --- a/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java +++ b/server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java @@ -60,18 +60,14 @@ public void usedDeprecatedName(String parserName, Supplier loc @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName, String replacedWith) { String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; - deprecationLogger.deprecated("{} Deprecated field [{}] used, replaced by [{}]", + deprecationLogger.deprecated("{}Deprecated field [{}] used, replaced by [{}]", prefix, usedName, replacedWith); } @Override public void usedDeprecatedField(String parserName, Supplier location, String usedName) { String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] "; - if (true) { - deprecationLogger.deprecated("{}Deprecated field [{}] used, this field is unused and will be removed entirely", - prefix, usedName); - } else { - deprecationLogger.deprecated("Deprecated field [{}] used, this field is unused and will be removed entirely", usedName); - } + deprecationLogger.deprecated("{}Deprecated field [{}] used, this field is unused and will be removed entirely", + prefix, usedName); } } From 395c25436db000df12b6943ef2ba268edadc89fd Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 15:00:39 +0000 Subject: [PATCH 06/12] Strip context from warnings in assertions --- .../common/logging/EvilLoggerTests.java | 2 +- .../org/elasticsearch/http/DeprecationHttpIT.java | 2 +- .../common/logging/DeprecationLogger.java | 14 +++++++++++--- .../common/logging/DeprecationLoggerTests.java | 8 ++++++-- .../common/util/concurrent/ThreadContextTests.java | 4 ++-- .../index/query/BoolQueryBuilderTests.java | 2 +- .../elasticsearch/script/StoredScriptTests.java | 2 +- .../java/org/elasticsearch/test/ESTestCase.java | 3 ++- .../test/ml/data_frame_analytics_crud.yml | 2 +- 9 files changed, 26 insertions(+), 13 deletions(-) diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java index e32447c47b092..0da2b9bd2cfde 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java @@ -164,7 +164,7 @@ public void testConcurrentDeprecationLogger() throws IOException, UserException, */ final List warnings = threadContext.getResponseHeaders().get("Warning"); final Set actualWarningValues = - warnings.stream().map(DeprecationLogger::extractWarningValueFromWarningHeader).collect(Collectors.toSet()); + warnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)).collect(Collectors.toSet()); for (int j = 0; j < 128; j++) { assertThat( actualWarningValues, diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java index fc686c5671997..9942f630aab15 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java @@ -187,7 +187,7 @@ private void doTestDeprecationWarningsAppearInHeaders() throws IOException { assertThat(deprecatedWarning, matches(WARNING_HEADER_PATTERN.pattern())); } final List actualWarningValues = - deprecatedWarnings.stream().map(DeprecationLogger::extractWarningValueFromWarningHeader).collect(Collectors.toList()); + deprecatedWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)).collect(Collectors.toList()); for (Matcher headerMatcher : headerMatchers) { assertThat(actualWarningValues, hasItem(headerMatcher)); } diff --git a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java index be157e16de20c..9e6e73acbe839 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java +++ b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java @@ -174,6 +174,8 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje "GMT" + // GMT "\")?"); // closing quote (optional, since an older version can still send a warn-date) + public static final Pattern WARNING_XCONTENT_LOCATION_PATTERN = Pattern.compile("^\\[.*?]\\[\\d+:\\d+] "); + /** * Extracts the warning value from the value of a warning header that is formatted according to RFC 7234. That is, given a string * {@code 299 Elasticsearch-6.0.0 "warning value"}, the return value of this method would be {@code warning value}. @@ -181,7 +183,7 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje * @param s the value of a warning header formatted according to RFC 7234. * @return the extracted warning value */ - public static String extractWarningValueFromWarningHeader(final String s) { + public static String extractWarningValueFromWarningHeader(final String s, boolean stripXContentPosition) { /* * We know the exact format of the warning header, so to extract the warning value we can skip forward from the front to the first * quote and we know the last quote is at the end of the string @@ -196,8 +198,14 @@ public static String extractWarningValueFromWarningHeader(final String s) { */ final int firstQuote = s.indexOf('\"'); final int lastQuote = s.length() - 1; - final String warningValue = s.substring(firstQuote + 1, lastQuote); + String warningValue = s.substring(firstQuote + 1, lastQuote); assert assertWarningValue(s, warningValue); + if (stripXContentPosition) { + Matcher matcher = WARNING_XCONTENT_LOCATION_PATTERN.matcher(warningValue); + if (matcher.find()) { + warningValue = warningValue.substring(matcher.end()); + } + } return warningValue; } @@ -232,7 +240,7 @@ void deprecated(final Set threadContexts, final String message, f final String formattedMessage = LoggerMessageFormat.format(message, params); final String warningHeaderValue = formatWarning(formattedMessage); assert WARNING_HEADER_PATTERN.matcher(warningHeaderValue).matches(); - assert extractWarningValueFromWarningHeader(warningHeaderValue).equals(escapeAndEncode(formattedMessage)); + assert extractWarningValueFromWarningHeader(warningHeaderValue, false).equals(escapeAndEncode(formattedMessage)); while (iterator.hasNext()) { try { final ThreadContext next = iterator.next(); diff --git a/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java b/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java index e7a78454e57ff..668f9e1616f1c 100644 --- a/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java +++ b/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java @@ -216,10 +216,14 @@ public void testFailsWhenRemovingUnknownThreadContext() throws IOException { expectThrows(IllegalStateException.class, () -> DeprecationLogger.removeThreadContext(threadContext)); } - public void testWarningValueFromWarningHeader() throws InterruptedException { + public void testWarningValueFromWarningHeader() { final String s = randomAlphaOfLength(16); final String first = DeprecationLogger.formatWarning(s); - assertThat(DeprecationLogger.extractWarningValueFromWarningHeader(first), equalTo(s)); + assertThat(DeprecationLogger.extractWarningValueFromWarningHeader(first, false), equalTo(s)); + + final String withPos = "[context][1:11] Blah blah blah"; + final String formatted = DeprecationLogger.formatWarning(withPos); + assertThat(DeprecationLogger.extractWarningValueFromWarningHeader(formatted, true), equalTo("Blah blah blah")); } public void testEscapeBackslashesAndQuotes() { diff --git a/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java b/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java index 577b451d8f19e..5305b80c9828b 100644 --- a/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java +++ b/server/src/test/java/org/elasticsearch/common/util/concurrent/ThreadContextTests.java @@ -208,11 +208,11 @@ public void testResponseHeaders() { } final String value = DeprecationLogger.formatWarning("qux"); - threadContext.addResponseHeader("baz", value, DeprecationLogger::extractWarningValueFromWarningHeader); + threadContext.addResponseHeader("baz", value, s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, false)); // pretend that another thread created the same response at a different time if (randomBoolean()) { final String duplicateValue = DeprecationLogger.formatWarning("qux"); - threadContext.addResponseHeader("baz", duplicateValue, DeprecationLogger::extractWarningValueFromWarningHeader); + threadContext.addResponseHeader("baz", duplicateValue, s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, false)); } threadContext.addResponseHeader("Warning", "One is the loneliest number"); diff --git a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java index 03916bac58a84..93d46ed592570 100644 --- a/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java +++ b/server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java @@ -302,7 +302,7 @@ public void testDeprecation() throws IOException { QueryBuilder q = parseQuery(query); QueryBuilder expected = new BoolQueryBuilder().mustNot(new MatchAllQueryBuilder()); assertEquals(expected, q); - assertWarnings("[bool][1:24] Deprecated field [mustNot] used, expected [must_not] instead"); + assertWarnings("Deprecated field [mustNot] used, expected [must_not] instead"); } public void testRewrite() throws IOException { diff --git a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java index 47dc7aaae2a3b..627d67dc833e4 100644 --- a/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java +++ b/server/src/test/java/org/elasticsearch/script/StoredScriptTests.java @@ -104,7 +104,7 @@ public void testSourceParsing() throws Exception { assertThat(parsed, equalTo(source)); } - assertWarnings("[stored script source][1:33] Deprecated field [code] used, expected [source] instead"); + assertWarnings("Deprecated field [code] used, expected [source] instead"); // complex script with script object and empty options try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) { 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 75e577680274a..2bacd445b7924 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -429,7 +429,8 @@ protected final void assertWarnings(String... expectedWarnings) { final List actualWarnings = threadContext.getResponseHeaders().get("Warning"); assertNotNull("no warnings, expected: " + Arrays.asList(expectedWarnings), actualWarnings); final Set actualWarningValues = - actualWarnings.stream().map(DeprecationLogger::extractWarningValueFromWarningHeader).collect(Collectors.toSet()); + actualWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)) + .collect(Collectors.toSet()); for (String msg : expectedWarnings) { assertThat(actualWarningValues, hasItem(DeprecationLogger.escapeAndEncode(msg))); } diff --git a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml index c5df3d9f6cc51..a5a99b30391ea 100644 --- a/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml +++ b/x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml @@ -1869,7 +1869,7 @@ setup: - do: allowed_warnings: - - '[classification][1:139] Deprecated field [maximum_number_trees] used, expected [max_trees] instead' + - 'Deprecated field [maximum_number_trees] used, expected [max_trees] instead' ml.put_data_frame_analytics: id: "classification-with-maximum-number-trees" body: > From 8e80f3aedf57d92c9cc11fa6d343868e91facc09 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 15:11:21 +0000 Subject: [PATCH 07/12] line length --- .../java/org/elasticsearch/common/logging/EvilLoggerTests.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java index 0da2b9bd2cfde..2e35ac9581f9a 100644 --- a/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java +++ b/qa/evil-tests/src/test/java/org/elasticsearch/common/logging/EvilLoggerTests.java @@ -164,7 +164,8 @@ public void testConcurrentDeprecationLogger() throws IOException, UserException, */ final List warnings = threadContext.getResponseHeaders().get("Warning"); final Set actualWarningValues = - warnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)).collect(Collectors.toSet()); + warnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)) + .collect(Collectors.toSet()); for (int j = 0; j < 128; j++) { assertThat( actualWarningValues, From 7b374e3cc2673a4825e07bdfd4a3b67dfa824b9c Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 15:24:18 +0000 Subject: [PATCH 08/12] precommit already, fine, fine --- .../test/java/org/elasticsearch/http/DeprecationHttpIT.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java index 9942f630aab15..c9bed57fb13e7 100644 --- a/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java +++ b/qa/smoke-test-http/src/test/java/org/elasticsearch/http/DeprecationHttpIT.java @@ -187,7 +187,8 @@ private void doTestDeprecationWarningsAppearInHeaders() throws IOException { assertThat(deprecatedWarning, matches(WARNING_HEADER_PATTERN.pattern())); } final List actualWarningValues = - deprecatedWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)).collect(Collectors.toList()); + deprecatedWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)) + .collect(Collectors.toList()); for (Matcher headerMatcher : headerMatchers) { assertThat(actualWarningValues, hasItem(headerMatcher)); } From b1393a1b10c44606e52d175824ab07f3eca3c200 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 15:54:12 +0000 Subject: [PATCH 09/12] test fix --- .../ingest/common/ScriptProcessorFactoryTests.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java index c7018d40b18ca..9559c150df68a 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java @@ -98,7 +98,7 @@ public void testInlineBackcompat() throws Exception { configMap.put("inline", "code"); factory.create(null, randomAlphaOfLength(10), configMap); - assertWarnings("[script][1:11] Deprecated field [inline] used, expected [source] instead"); + assertWarnings("Deprecated field [inline] used, expected [source] instead"); } public void testFactoryInvalidateWithInvalidCompiledScript() throws Exception { From 171b91c818d74eabd84653bce23d47cab0c0d333 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 15:57:56 +0000 Subject: [PATCH 10/12] Add option to not strip context from warnings in assertions --- .../elasticsearch/common/xcontent/ObjectParserTests.java | 2 +- .../src/main/java/org/elasticsearch/test/ESTestCase.java | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java index 9a813c8a24616..5eca7d527730b 100644 --- a/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java +++ b/libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java @@ -223,7 +223,7 @@ class TestStruct { objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING); objectParser.parse(parser, s, null); assertEquals("foo", s.test); - assertWarnings("[foo][1:15] Deprecated field [old_test] used, expected [test] instead"); + assertWarnings(false, "[foo][1:15] Deprecated field [old_test] used, expected [test] instead"); } public void testFailOnValueType() throws IOException { 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 2bacd445b7924..0a27993b40d7e 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESTestCase.java @@ -422,6 +422,10 @@ protected final void assertSettingDeprecationsAndWarnings(final String[] setting } protected final void assertWarnings(String... expectedWarnings) { + assertWarnings(true, expectedWarnings); + } + + protected final void assertWarnings(boolean stripXContentPosition, String... expectedWarnings) { if (enableWarningsCheck() == false) { throw new IllegalStateException("unable to check warning headers if the test is not set to do so"); } @@ -429,7 +433,7 @@ protected final void assertWarnings(String... expectedWarnings) { final List actualWarnings = threadContext.getResponseHeaders().get("Warning"); assertNotNull("no warnings, expected: " + Arrays.asList(expectedWarnings), actualWarnings); final Set actualWarningValues = - actualWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, true)) + actualWarnings.stream().map(s -> DeprecationLogger.extractWarningValueFromWarningHeader(s, stripXContentPosition)) .collect(Collectors.toSet()); for (String msg : expectedWarnings) { assertThat(actualWarningValues, hasItem(DeprecationLogger.escapeAndEncode(msg))); From 8a1fb9011482008e997f9c64965e6abd3f939d14 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 20 Mar 2020 09:10:50 +0000 Subject: [PATCH 11/12] Strip xcontentloc in yaml tests --- .../org/elasticsearch/test/rest/yaml/section/DoSection.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java index fec634b526756..d7884dd79d510 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java +++ b/test/framework/src/main/java/org/elasticsearch/test/rest/yaml/section/DoSection.java @@ -331,7 +331,7 @@ void checkWarningHeaders(final List warningHeaders, final Version master final Matcher matcher = WARNING_HEADER_PATTERN.matcher(header); final boolean matches = matcher.matches(); if (matches) { - final String message = matcher.group(1); + final String message = DeprecationLogger.extractWarningValueFromWarningHeader(header, true); if (allowed.contains(message)) { continue; } From b44af1798d9b32ddf2b222b0d5bd44c22f6d9453 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Fri, 20 Mar 2020 10:12:18 +0000 Subject: [PATCH 12/12] Handle negative pos values --- .../org/elasticsearch/common/logging/DeprecationLogger.java | 2 +- .../elasticsearch/common/logging/DeprecationLoggerTests.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java index 9e6e73acbe839..52ee3b7a5d288 100644 --- a/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java +++ b/server/src/main/java/org/elasticsearch/common/logging/DeprecationLogger.java @@ -174,7 +174,7 @@ public void deprecatedAndMaybeLog(final String key, final String msg, final Obje "GMT" + // GMT "\")?"); // closing quote (optional, since an older version can still send a warn-date) - public static final Pattern WARNING_XCONTENT_LOCATION_PATTERN = Pattern.compile("^\\[.*?]\\[\\d+:\\d+] "); + public static final Pattern WARNING_XCONTENT_LOCATION_PATTERN = Pattern.compile("^\\[.*?]\\[-?\\d+:-?\\d+] "); /** * Extracts the warning value from the value of a warning header that is formatted according to RFC 7234. That is, given a string diff --git a/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java b/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java index 668f9e1616f1c..d904d2f0c8526 100644 --- a/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java +++ b/server/src/test/java/org/elasticsearch/common/logging/DeprecationLoggerTests.java @@ -224,6 +224,10 @@ public void testWarningValueFromWarningHeader() { final String withPos = "[context][1:11] Blah blah blah"; final String formatted = DeprecationLogger.formatWarning(withPos); assertThat(DeprecationLogger.extractWarningValueFromWarningHeader(formatted, true), equalTo("Blah blah blah")); + + final String withNegativePos = "[context][-1:-1] Blah blah blah"; + assertThat(DeprecationLogger.extractWarningValueFromWarningHeader(DeprecationLogger.formatWarning(withNegativePos), true), + equalTo("Blah blah blah")); } public void testEscapeBackslashesAndQuotes() {