Skip to content

Commit 7636930

Browse files
authored
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.
1 parent 5c8cd16 commit 7636930

File tree

15 files changed

+115
-58
lines changed

15 files changed

+115
-58
lines changed

libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,13 @@
1919
package org.elasticsearch.common;
2020

2121
import org.elasticsearch.common.xcontent.DeprecationHandler;
22+
import org.elasticsearch.common.xcontent.XContentLocation;
2223

2324
import java.util.Collections;
2425
import java.util.HashSet;
2526
import java.util.Objects;
2627
import java.util.Set;
28+
import java.util.function.Supplier;
2729

2830
/**
2931
* Holds a field that can be found in a request while parsing and its different
@@ -115,6 +117,22 @@ public ParseField withAllDeprecated() {
115117
* names for this {@link ParseField}.
116118
*/
117119
public boolean match(String fieldName, DeprecationHandler deprecationHandler) {
120+
return match(null, () -> XContentLocation.UNKNOWN, fieldName, deprecationHandler);
121+
}
122+
123+
/**
124+
* Does {@code fieldName} match this field?
125+
* @param parserName
126+
* the name of the parent object holding this field
127+
* @param location
128+
* the XContentLocation of the field
129+
* @param fieldName
130+
* the field name to match against this {@link ParseField}
131+
* @param deprecationHandler called if {@code fieldName} is deprecated
132+
* @return true if <code>fieldName</code> matches any of the acceptable
133+
* names for this {@link ParseField}.
134+
*/
135+
public boolean match(String parserName, Supplier<XContentLocation> location, String fieldName, DeprecationHandler deprecationHandler) {
118136
Objects.requireNonNull(fieldName, "fieldName cannot be null");
119137
// if this parse field has not been completely deprecated then try to
120138
// match the preferred name
@@ -127,11 +145,11 @@ public boolean match(String fieldName, DeprecationHandler deprecationHandler) {
127145
for (String depName : deprecatedNames) {
128146
if (fieldName.equals(depName)) {
129147
if (fullyDeprecated) {
130-
deprecationHandler.usedDeprecatedField(fieldName);
148+
deprecationHandler.usedDeprecatedField(parserName, location, fieldName);
131149
} else if (allReplacedWith == null) {
132-
deprecationHandler.usedDeprecatedName(fieldName, name);
150+
deprecationHandler.usedDeprecatedName(parserName, location, fieldName, name);
133151
} else {
134-
deprecationHandler.usedDeprecatedField(fieldName, allReplacedWith);
152+
deprecationHandler.usedDeprecatedField(parserName, location, fieldName, allReplacedWith);
135153
}
136154
return true;
137155
}

libs/x-content/src/main/java/org/elasticsearch/common/xcontent/DeprecationHandler.java

Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,8 @@
1919

2020
package org.elasticsearch.common.xcontent;
2121

22+
import java.util.function.Supplier;
23+
2224
/**
2325
* Callback for notifying the creator of the {@link XContentParser} that
2426
* parsing hit a deprecated field.
@@ -32,20 +34,35 @@ public interface DeprecationHandler {
3234
*/
3335
DeprecationHandler THROW_UNSUPPORTED_OPERATION = new DeprecationHandler() {
3436
@Override
35-
public void usedDeprecatedField(String usedName, String replacedWith) {
36-
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
37-
+ usedName + "] which is a deprecated name for [" + replacedWith + "]");
37+
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith) {
38+
if (parserName != null) {
39+
throw new UnsupportedOperationException("deprecated fields not supported in [" + parserName + "] but got ["
40+
+ usedName + "] at [" + location.get() + "] which is a deprecated name for [" + replacedWith + "]");
41+
} else {
42+
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
43+
+ usedName + "] which is a deprecated name for [" + replacedWith + "]");
44+
}
3845
}
3946
@Override
40-
public void usedDeprecatedName(String usedName, String modernName) {
41-
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
42-
+ usedName + "] which has been replaced with [" + modernName + "]");
47+
public void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName) {
48+
if (parserName != null) {
49+
throw new UnsupportedOperationException("deprecated fields not supported in [" + parserName + "] but got ["
50+
+ usedName + "] at [" + location.get() + "] which has been replaced with [" + modernName + "]");
51+
} else {
52+
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
53+
+ usedName + "] which has been replaced with [" + modernName + "]");
54+
}
4355
}
4456

4557
@Override
46-
public void usedDeprecatedField(String usedName) {
47-
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
48-
+ usedName + "] which has been deprecated entirely");
58+
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName) {
59+
if (parserName != null) {
60+
throw new UnsupportedOperationException("deprecated fields not supported in [" + parserName + "] but got ["
61+
+ usedName + "] at [" + location.get() + "] which has been deprecated entirely");
62+
} else {
63+
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
64+
+ usedName + "] which has been deprecated entirely");
65+
}
4966
}
5067
};
5168

@@ -54,17 +71,17 @@ public void usedDeprecatedField(String usedName) {
5471
*/
5572
DeprecationHandler IGNORE_DEPRECATIONS = new DeprecationHandler() {
5673
@Override
57-
public void usedDeprecatedName(String usedName, String modernName) {
74+
public void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName) {
5875

5976
}
6077

6178
@Override
62-
public void usedDeprecatedField(String usedName, String replacedWith) {
79+
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith) {
6380

6481
}
6582

6683
@Override
67-
public void usedDeprecatedField(String usedName) {
84+
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName) {
6885

6986
}
7087
};
@@ -74,20 +91,21 @@ public void usedDeprecatedField(String usedName) {
7491
* @param usedName the provided field name
7592
* @param modernName the modern name for the field
7693
*/
77-
void usedDeprecatedName(String usedName, String modernName);
94+
void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName);
7895

7996
/**
8097
* Called when the provided field name matches the current field but the entire
8198
* field has been marked as deprecated and another field should be used
8299
* @param usedName the provided field name
83100
* @param replacedWith the name of the field that replaced this field
84101
*/
85-
void usedDeprecatedField(String usedName, String replacedWith);
102+
void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith);
86103

87104
/**
88105
* Called when the provided field name matches the current field but the entire
89106
* field has been marked as deprecated with no replacement
90107
* @param usedName the provided field name
91108
*/
92-
void usedDeprecatedField(String usedName);
109+
void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName);
110+
93111
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -571,7 +571,7 @@ private class FieldParser {
571571
}
572572

573573
void assertSupports(String parserName, XContentParser parser, String currentFieldName) {
574-
if (parseField.match(currentFieldName, parser.getDeprecationHandler()) == false) {
574+
if (parseField.match(parserName, parser::getTokenLocation, currentFieldName, parser.getDeprecationHandler()) == false) {
575575
throw new XContentParseException(parser.getTokenLocation(),
576576
"[" + parserName + "] parsefield doesn't accept: " + currentFieldName);
577577
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,10 @@
2525
* position of a parsing error to end users and consequently have line and
2626
* column numbers starting from 1.
2727
*/
28-
public class XContentLocation {
28+
public final class XContentLocation {
29+
30+
public static final XContentLocation UNKNOWN = new XContentLocation(-1, -1);
31+
2932
public final int lineNumber;
3033
public final int columnNumber;
3134

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,11 @@ public void testDeprecatedWithNoReplacement() {
6666
ParseField field = new ParseField(name).withDeprecation(alternatives).withAllDeprecated();
6767
assertFalse(field.match("not a field name", LoggingDeprecationHandler.INSTANCE));
6868
assertTrue(field.match("dep", LoggingDeprecationHandler.INSTANCE));
69-
assertWarnings("Deprecated field [dep] used, which has been removed entirely");
69+
assertWarnings("Deprecated field [dep] used, this field is unused and will be removed entirely");
7070
assertTrue(field.match("old_dep", LoggingDeprecationHandler.INSTANCE));
71-
assertWarnings("Deprecated field [old_dep] used, which has been removed entirely");
71+
assertWarnings("Deprecated field [old_dep] used, this field is unused and will be removed entirely");
7272
assertTrue(field.match("new_dep", LoggingDeprecationHandler.INSTANCE));
73-
assertWarnings("Deprecated field [new_dep] used, which has been removed entirely");
73+
assertWarnings("Deprecated field [new_dep] used, this field is unused and will be removed entirely");
7474

7575

7676
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -223,7 +223,7 @@ class TestStruct {
223223
objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING);
224224
objectParser.parse(parser, s, null);
225225
assertEquals("foo", s.test);
226-
assertWarnings("Deprecated field [old_test] used, expected [test] instead");
226+
assertWarnings("[foo][1:15] Deprecated field [old_test] used, expected [test] instead");
227227
}
228228

229229
public void testFailOnValueType() throws IOException {

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ public void testInlineBackcompat() throws Exception {
9898
configMap.put("inline", "code");
9999

100100
factory.create(null, randomAlphaOfLength(10), configMap);
101-
assertWarnings("Deprecated field [inline] used, expected [source] instead");
101+
assertWarnings("[script][1:11] Deprecated field [inline] used, expected [source] instead");
102102
}
103103

104104
public void testFactoryInvalidateWithInvalidCompiledScript() throws Exception {

server/src/main/java/org/elasticsearch/common/xcontent/LoggingDeprecationHandler.java

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@
2323
import org.elasticsearch.common.ParseField;
2424
import org.elasticsearch.common.logging.DeprecationLogger;
2525

26+
import java.util.function.Supplier;
27+
2628
/**
2729
* Logs deprecations to the {@link DeprecationLogger}.
2830
* <p>
@@ -49,17 +51,23 @@ private LoggingDeprecationHandler() {
4951
}
5052

5153
@Override
52-
public void usedDeprecatedName(String usedName, String modernName) {
53-
deprecationLogger.deprecated("Deprecated field [{}] used, expected [{}] instead", usedName, modernName);
54+
public void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName) {
55+
String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] ";
56+
deprecationLogger.deprecated("{}Deprecated field [{}] used, expected [{}] instead",
57+
prefix, usedName, modernName);
5458
}
5559

5660
@Override
57-
public void usedDeprecatedField(String usedName, String replacedWith) {
58-
deprecationLogger.deprecated("Deprecated field [{}] used, replaced by [{}]", usedName, replacedWith);
61+
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith) {
62+
String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] ";
63+
deprecationLogger.deprecated("{}Deprecated field [{}] used, replaced by [{}]",
64+
prefix, usedName, replacedWith);
5965
}
6066

6167
@Override
62-
public void usedDeprecatedField(String usedName) {
63-
deprecationLogger.deprecated("Deprecated field [{}] used, which has been removed entirely", usedName);
68+
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName) {
69+
String prefix = parserName == null ? "" : "[" + parserName + "][" + location.get() + "] ";
70+
deprecationLogger.deprecated("{}Deprecated field [{}] used, this field is unused and will be removed entirely",
71+
prefix, usedName);
6472
}
6573
}

server/src/test/java/org/elasticsearch/index/query/BoolQueryBuilderTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -302,7 +302,7 @@ public void testDeprecation() throws IOException {
302302
QueryBuilder q = parseQuery(query);
303303
QueryBuilder expected = new BoolQueryBuilder().mustNot(new MatchAllQueryBuilder());
304304
assertEquals(expected, q);
305-
assertWarnings("Deprecated field [mustNot] used, expected [must_not] instead");
305+
assertWarnings("[bool][1:24] Deprecated field [mustNot] used, expected [must_not] instead");
306306
}
307307

308308
public void testRewrite() throws IOException {

server/src/test/java/org/elasticsearch/script/StoredScriptTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ public void testSourceParsing() throws Exception {
104104

105105
assertThat(parsed, equalTo(source));
106106
}
107-
assertWarnings("Deprecated field [code] used, expected [source] instead");
107+
assertWarnings("[stored script source][1:33] Deprecated field [code] used, expected [source] instead");
108108

109109
// complex script with script object and empty options
110110
try (XContentBuilder builder = XContentFactory.contentBuilder(XContentType.JSON)) {

0 commit comments

Comments
 (0)