From 88f6ad100ac887dbea91e2803e0303087a98db2e Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 17:12:39 +0000 Subject: [PATCH 1/5] 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 b716af4292661b6bf0dffc0b3720f849d2fc0ca7 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 17:46:52 +0000 Subject: [PATCH 2/5] 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 d070a61ea17af1f86948efab2fb5e8c4e424ad95 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Wed, 18 Mar 2020 17:56:52 +0000 Subject: [PATCH 3/5] 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 e0a74921aaa3390ffc8039b786f90e8d3dc88400 Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 09:44:21 +0000 Subject: [PATCH 4/5] 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 95b20db90e896ab637a57a3aec3f6e6ef958edbc Mon Sep 17 00:00:00 2001 From: Alan Woodward Date: Thu, 19 Mar 2020 09:59:41 +0000 Subject: [PATCH 5/5] 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); } }