Browse Source

Report parser name and location in XContent deprecation warnings (#53752)

It's simple to deprecate a field used in an ObjectParser just by adding deprecation
markers to the relevant ParseField objects. However, the warnings themselves don't 
currently have any context; they simply say that a deprecated field has been used, 
but not where in the input it appears. This commit adds the parent object parser
name and XContentLocation to these deprecation messages.
Alan Woodward 5 years ago
parent
commit
7636930ceb
15 changed files with 115 additions and 58 deletions
  1. 21 3
      libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java
  2. 33 15
      libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java
  3. 1 1
      libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java
  4. 4 1
      libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java
  5. 3 3
      libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java
  6. 1 1
      libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java
  7. 1 1
      modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java
  8. 14 6
      server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java
  9. 1 1
      server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java
  10. 1 1
      server/src/test/java/org/elasticsearch/script/StoredScriptTests.java
  11. 17 12
      x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java
  12. 2 2
      x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java
  13. 1 1
      x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java
  14. 14 9
      x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java
  15. 1 1
      x-pack/plugin/src/test/resources/rest-api-spec/test/ml/data_frame_analytics_crud.yml

+ 21 - 3
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 class ParseField {
      *         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 <code>fieldName</code> matches any of the acceptable
+     *         names for this {@link ParseField}.
+     */
+    public boolean match(String parserName, Supplier<XContentLocation> 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 class ParseField {
         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;
             }

+ 33 - 15
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<XContentLocation> 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<XContentLocation> 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<XContentLocation> 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 interface DeprecationHandler {
      */
     DeprecationHandler IGNORE_DEPRECATIONS = new DeprecationHandler() {
         @Override
-        public void usedDeprecatedName(String usedName, String modernName) {
+        public void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName) {
 
         }
 
         @Override
-        public void usedDeprecatedField(String usedName, String replacedWith) {
+        public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith) {
 
         }
 
         @Override
-        public void usedDeprecatedField(String usedName) {
+        public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName) {
 
         }
     };
@@ -74,7 +91,7 @@ public interface DeprecationHandler {
      * @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<XContentLocation> location, String usedName, String modernName);
 
     /**
      * Called when the provided field name matches the current field but the entire
@@ -82,12 +99,13 @@ public interface DeprecationHandler {
      * @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<XContentLocation> 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<XContentLocation> location, String usedName);
+
 }

+ 1 - 1
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java

@@ -571,7 +571,7 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
         }
 
         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);
             }

+ 4 - 1
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/XContentLocation.java

@@ -25,7 +25,10 @@ package org.elasticsearch.common.xcontent;
  * 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;
 

+ 3 - 3
libs/x-content/src/test/java/org/elasticsearch/common/ParseFieldTests.java

@@ -66,11 +66,11 @@ public class ParseFieldTests extends ESTestCase {
         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");
+        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");
 
 
     }

+ 1 - 1
libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java

@@ -223,7 +223,7 @@ public class ObjectParserTests extends ESTestCase {
         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 {

+ 1 - 1
modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/ScriptProcessorFactoryTests.java

@@ -98,7 +98,7 @@ public class ScriptProcessorFactoryTests extends ESTestCase {
         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 {

+ 14 - 6
server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java

@@ -23,6 +23,8 @@ import org.apache.logging.log4j.LogManager;
 import org.elasticsearch.common.ParseField;
 import org.elasticsearch.common.logging.DeprecationLogger;
 
+import java.util.function.Supplier;
+
 /**
  * Logs deprecations to the {@link DeprecationLogger}.
  * <p>
@@ -49,17 +51,23 @@ public class LoggingDeprecationHandler implements DeprecationHandler {
     }
 
     @Override
-    public void usedDeprecatedName(String usedName, String modernName) {
-        deprecationLogger.deprecated("Deprecated field [{}] used, expected [{}] instead", usedName, modernName);
+    public void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName) {
+        String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] ";
+        deprecationLogger.deprecated("{}Deprecated field [{}] used, expected [{}] instead",
+            prefix, 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<XContentLocation> location, String usedName, String replacedWith) {
+        String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] ";
+        deprecationLogger.deprecated("{}Deprecated field [{}] used, replaced by [{}]",
+            prefix, 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<XContentLocation> location, String usedName) {
+        String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] ";
+        deprecationLogger.deprecated("{}Deprecated field [{}] used, this field is unused and will be removed entirely",
+            prefix, usedName);
     }
 }

+ 1 - 1
server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java

@@ -302,7 +302,7 @@ public class BoolQueryBuilderTests extends AbstractQueryTestCase<BoolQueryBuilde
         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 {

+ 1 - 1
server/src/test/java/org/elasticsearch/script/StoredScriptTests.java

@@ -104,7 +104,7 @@ public class StoredScriptTests extends AbstractSerializingTestCase<StoredScriptS
 
             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)) {

+ 17 - 12
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ml/utils/LoggingDeprecationAccumulationHandler.java

@@ -8,10 +8,12 @@ package org.elasticsearch.xpack.core.ml.utils;
 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,27 @@ public class LoggingDeprecationAccumulationHandler implements DeprecationHandler
     private final List<String> 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<XContentLocation> location, String usedName, String modernName) {
+        LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(parserName, location, 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 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<XContentLocation> location, String usedName, String replacedWith) {
+        LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(parserName, location, 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 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<XContentLocation> location, String usedName) {
+        LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(parserName, location, 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}));
     }
 
     /**

+ 2 - 2
x-pack/plugin/core/src/test/java/org/elasticsearch/xpack/core/ml/utils/XContentObjectTransformerTests.java

@@ -127,8 +127,8 @@ public class XContentObjectTransformerTests extends ESTestCase {
     public void testDeprecationWarnings() throws IOException {
         XContentObjectTransformer<QueryBuilder> 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<String> deprecations = new ArrayList<>();

+ 1 - 1
x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessor.java

@@ -242,7 +242,7 @@ public class InferenceProcessor extends AbstractProcessor {
                 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));

+ 14 - 9
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/ApiKeyService.java

@@ -49,6 +49,7 @@ import org.elasticsearch.common.xcontent.DeprecationHandler;
 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,25 @@ public class ApiKeyService {
         }
 
         @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<XContentLocation> location, String usedName, String 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 usedName, String replacedWith) {
-            deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], replaced by [{}]",
-                usedName, apiKeyId, replacedWith);
+        public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String 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 usedName) {
-            deprecationLogger.deprecated("Deprecated field [{}] used in api key [{}], which has been removed entirely",
-                usedName);
+        public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String 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);
         }
     }
 

+ 1 - 1
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: >