Skip to content

Commit

Permalink
Revert "Report parser name and location in XContent deprecation warni…
Browse files Browse the repository at this point in the history
…ngs (#53752)"

This reverts commit 7636930.
  • Loading branch information
romseygeek committed Mar 19, 2020
1 parent 8d5478f commit 10d72b0
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 115 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,11 @@
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
Expand Down Expand Up @@ -117,22 +115,6 @@ 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 <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
Expand All @@ -145,11 +127,11 @@ public boolean match(String parserName, Supplier<XContentLocation> location, Str
for (String depName : deprecatedNames) {
if (fieldName.equals(depName)) {
if (fullyDeprecated) {
deprecationHandler.usedDeprecatedField(parserName, location, fieldName);
deprecationHandler.usedDeprecatedField(fieldName);
} else if (allReplacedWith == null) {
deprecationHandler.usedDeprecatedName(parserName, location, fieldName, name);
deprecationHandler.usedDeprecatedName(fieldName, name);
} else {
deprecationHandler.usedDeprecatedField(parserName, location, fieldName, allReplacedWith);
deprecationHandler.usedDeprecatedField(fieldName, allReplacedWith);
}
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,6 @@

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.
Expand All @@ -34,35 +32,20 @@ public interface DeprecationHandler {
*/
DeprecationHandler THROW_UNSUPPORTED_OPERATION = new DeprecationHandler() {
@Override
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 + "]");
}
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 + "]");
}
@Override
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 + "]");
}
public void usedDeprecatedName(String usedName, String modernName) {
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
+ usedName + "] which has been replaced with [" + modernName + "]");
}

@Override
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");
}
public void usedDeprecatedField(String usedName) {
throw new UnsupportedOperationException("deprecated fields not supported here but got ["
+ usedName + "] which has been deprecated entirely");
}
};

Expand All @@ -71,17 +54,17 @@ public void usedDeprecatedField(String parserName, Supplier<XContentLocation> lo
*/
DeprecationHandler IGNORE_DEPRECATIONS = new DeprecationHandler() {
@Override
public void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName) {
public void usedDeprecatedName(String usedName, String modernName) {

}

@Override
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith) {
public void usedDeprecatedField(String usedName, String replacedWith) {

}

@Override
public void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName) {
public void usedDeprecatedField(String usedName) {

}
};
Expand All @@ -91,21 +74,20 @@ public void usedDeprecatedField(String parserName, Supplier<XContentLocation> lo
* @param usedName the provided field name
* @param modernName the modern name for the field
*/
void usedDeprecatedName(String parserName, Supplier<XContentLocation> location, String usedName, String modernName);
void usedDeprecatedName(String usedName, String modernName);

/**
* Called when the provided field name matches the current field but the entire
* field has been marked as deprecated and another field should be used
* @param usedName the provided field name
* @param replacedWith the name of the field that replaced this field
*/
void usedDeprecatedField(String parserName, Supplier<XContentLocation> location, String usedName, String replacedWith);
void usedDeprecatedField(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 parserName, Supplier<XContentLocation> location, String usedName);

void usedDeprecatedField(String usedName);
}
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ private class FieldParser {
}

void assertSupports(String parserName, XContentParser parser, String currentFieldName) {
if (parseField.match(parserName, parser::getTokenLocation, currentFieldName, parser.getDeprecationHandler()) == false) {
if (parseField.match(currentFieldName, parser.getDeprecationHandler()) == false) {
throw new XContentParseException(parser.getTokenLocation(),
"[" + parserName + "] parsefield doesn't accept: " + currentFieldName);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,7 @@
* position of a parsing error to end users and consequently have line and
* column numbers starting from 1.
*/
public final class XContentLocation {

public static final XContentLocation UNKNOWN = new XContentLocation(-1, -1);

public class XContentLocation {
public final int lineNumber;
public final int columnNumber;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,11 +66,11 @@ 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, this field is unused and will be removed entirely");
assertWarnings("Deprecated field [dep] used, which has been removed entirely");
assertTrue(field.match("old_dep", LoggingDeprecationHandler.INSTANCE));
assertWarnings("Deprecated field [old_dep] used, this field is unused and will be removed entirely");
assertWarnings("Deprecated field [old_dep] used, which has been removed entirely");
assertTrue(field.match("new_dep", LoggingDeprecationHandler.INSTANCE));
assertWarnings("Deprecated field [new_dep] used, this field is unused and will be removed entirely");
assertWarnings("Deprecated field [new_dep] used, which has been removed entirely");


}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("Deprecated field [old_test] used, expected [test] instead");
}

public void testFailOnValueType() throws IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@
import org.elasticsearch.common.ParseField;
import org.elasticsearch.common.logging.DeprecationLogger;

import java.util.function.Supplier;

/**
* Logs deprecations to the {@link DeprecationLogger}.
* <p>
Expand All @@ -51,23 +49,17 @@ private LoggingDeprecationHandler() {
}

@Override
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);
public void usedDeprecatedName(String usedName, String modernName) {
deprecationLogger.deprecated("Deprecated field [{}] used, expected [{}] instead", usedName, modernName);
}

@Override
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);
public void usedDeprecatedField(String usedName, String replacedWith) {
deprecationLogger.deprecated("Deprecated field [{}] used, replaced by [{}]", usedName, replacedWith);
}

@Override
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);
public void usedDeprecatedField(String usedName) {
deprecationLogger.deprecated("Deprecated field [{}] used, which has been removed entirely", usedName);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,10 @@
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:
Expand All @@ -28,27 +26,24 @@ public class LoggingDeprecationAccumulationHandler implements DeprecationHandler
private final List<String> deprecations = new ArrayList<>();

@Override
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}));
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}));
}

@Override
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}));
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}));
}

@Override
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}));
public void usedDeprecatedField(String usedName) {
LoggingDeprecationHandler.INSTANCE.usedDeprecatedField(usedName);
deprecations.add(LoggerMessageFormat.format("Deprecated field [{}] used, which has been deprecated entirely",
new Object[]{ usedName }));
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,8 @@ public void testToMap() throws IOException {
public void testDeprecationWarnings() throws IOException {
XContentObjectTransformer<QueryBuilder> queryBuilderTransformer = new XContentObjectTransformer<>(NamedXContentRegistry.EMPTY,
(p)-> {
p.getDeprecationHandler().usedDeprecatedField(null, null, "oldField", "newField");
p.getDeprecationHandler().usedDeprecatedName(null, null, "oldName", "modernName");
p.getDeprecationHandler().usedDeprecatedField("oldField", "newField");
p.getDeprecationHandler().usedDeprecatedName("oldName", "modernName");
return new BoolQueryBuilder();
});
List<String> deprecations = new ArrayList<>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ public InferenceProcessor create(Map<String, Processor.Factory> processorFactori
fieldMap = ConfigurationUtils.readOptionalMap(TYPE, tag, config, FIELD_MAPPINGS);
//TODO Remove in 8.x
if (fieldMap != null) {
LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(null, () -> null, FIELD_MAPPINGS, FIELD_MAP);
LoggingDeprecationHandler.INSTANCE.usedDeprecatedName(FIELD_MAPPINGS, FIELD_MAP);
}
}
InferenceConfig inferenceConfig = inferenceConfigFromMap(ConfigurationUtils.readMap(TYPE, tag, config, INFERENCE_CONFIG));
Expand Down
Loading

0 comments on commit 10d72b0

Please sign in to comment.