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.

There is some randomization in the YAML test suite which means we can't check
for exact xcontentlocation in the deprecation warning headers.
  • Loading branch information
romseygeek committed Mar 19, 2020
1 parent 8d5478f commit c6cdd3a
Show file tree
Hide file tree
Showing 15 changed files with 58 additions and 115 deletions.
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
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);
}
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
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
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
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
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
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);
}
}
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
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
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
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
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

0 comments on commit c6cdd3a

Please sign in to comment.