ParseFieldMatcher should log when using deprecated settings. #16988
I always thought ParseFieldMatcher would log when using a deprecated setting, but it does not.
This commit is contained in:
parent
3d13c27fa0
commit
026519e81b
|
@ -18,26 +18,23 @@
|
|||
*/
|
||||
package org.elasticsearch.common;
|
||||
|
||||
import org.elasticsearch.common.logging.DeprecationLogger;
|
||||
import org.elasticsearch.common.logging.Loggers;
|
||||
|
||||
import java.util.EnumSet;
|
||||
import java.util.HashSet;
|
||||
|
||||
/**
|
||||
* Holds a field that can be found in a request while parsing and its different variants, which may be deprecated.
|
||||
*/
|
||||
public class ParseField {
|
||||
|
||||
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ParseField.class));
|
||||
|
||||
private final String camelCaseName;
|
||||
private final String underscoreName;
|
||||
private final String[] deprecatedNames;
|
||||
private String allReplacedWith = null;
|
||||
|
||||
static final EnumSet<Flag> EMPTY_FLAGS = EnumSet.noneOf(Flag.class);
|
||||
static final EnumSet<Flag> STRICT_FLAGS = EnumSet.of(Flag.STRICT);
|
||||
|
||||
enum Flag {
|
||||
STRICT
|
||||
}
|
||||
|
||||
public ParseField(String value, String... deprecatedNames) {
|
||||
camelCaseName = Strings.toCamelCase(value);
|
||||
underscoreName = Strings.toUnderscoreCase(value);
|
||||
|
@ -80,19 +77,21 @@ public class ParseField {
|
|||
return parseField;
|
||||
}
|
||||
|
||||
boolean match(String currentFieldName, EnumSet<Flag> flags) {
|
||||
boolean match(String currentFieldName, boolean strict) {
|
||||
if (allReplacedWith == null && (currentFieldName.equals(camelCaseName) || currentFieldName.equals(underscoreName))) {
|
||||
return true;
|
||||
}
|
||||
String msg;
|
||||
for (String depName : deprecatedNames) {
|
||||
if (currentFieldName.equals(depName)) {
|
||||
if (flags.contains(Flag.STRICT)) {
|
||||
msg = "Deprecated field [" + currentFieldName + "] used, expected [" + underscoreName + "] instead";
|
||||
if (allReplacedWith != null) {
|
||||
msg = "Deprecated field [" + currentFieldName + "] used, replaced by [" + allReplacedWith + "]";
|
||||
}
|
||||
if (strict) {
|
||||
throw new IllegalArgumentException(msg);
|
||||
} else {
|
||||
DEPRECATION_LOGGER.deprecated(msg);
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
|
|
@ -21,29 +21,28 @@ package org.elasticsearch.common;
|
|||
|
||||
import org.elasticsearch.common.settings.Settings;
|
||||
|
||||
import java.util.EnumSet;
|
||||
|
||||
/**
|
||||
* Matcher to use in combination with {@link ParseField} while parsing requests. Matches a {@link ParseField}
|
||||
* against a field name and throw deprecation exception depending on the current value of the {@link #PARSE_STRICT} setting.
|
||||
*/
|
||||
public class ParseFieldMatcher {
|
||||
public static final String PARSE_STRICT = "index.query.parse.strict";
|
||||
public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(ParseField.EMPTY_FLAGS);
|
||||
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(ParseField.STRICT_FLAGS);
|
||||
public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(false);
|
||||
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(true);
|
||||
|
||||
private final EnumSet<ParseField.Flag> parseFlags;
|
||||
private final boolean strict;
|
||||
|
||||
public ParseFieldMatcher(Settings settings) {
|
||||
if (settings.getAsBoolean(PARSE_STRICT, false)) {
|
||||
this.parseFlags = EnumSet.of(ParseField.Flag.STRICT);
|
||||
} else {
|
||||
this.parseFlags = ParseField.EMPTY_FLAGS;
|
||||
}
|
||||
this(settings.getAsBoolean(PARSE_STRICT, false));
|
||||
}
|
||||
|
||||
public ParseFieldMatcher(EnumSet<ParseField.Flag> parseFlags) {
|
||||
this.parseFlags = parseFlags;
|
||||
public ParseFieldMatcher(boolean strict) {
|
||||
this.strict = strict;
|
||||
}
|
||||
|
||||
/** Should deprecated settings be rejected? */
|
||||
public boolean isStrict() {
|
||||
return strict;
|
||||
}
|
||||
|
||||
/**
|
||||
|
@ -55,6 +54,6 @@ public class ParseFieldMatcher {
|
|||
* @return true whenever the parse field that we are looking for was found, false otherwise
|
||||
*/
|
||||
public boolean match(String fieldName, ParseField parseField) {
|
||||
return parseField.match(fieldName, parseFlags);
|
||||
return parseField.match(fieldName, strict);
|
||||
}
|
||||
}
|
||||
|
|
|
@ -20,8 +20,7 @@ package org.elasticsearch.common;
|
|||
|
||||
import org.elasticsearch.test.ESTestCase;
|
||||
|
||||
import java.util.EnumSet;
|
||||
|
||||
import static org.hamcrest.CoreMatchers.containsString;
|
||||
import static org.hamcrest.CoreMatchers.is;
|
||||
import static org.hamcrest.CoreMatchers.not;
|
||||
import static org.hamcrest.CoreMatchers.sameInstance;
|
||||
|
@ -33,38 +32,29 @@ public class ParseFieldTests extends ESTestCase {
|
|||
String[] deprecated = new String[]{"barFoo", "bar_foo"};
|
||||
ParseField withDeprecations = field.withDeprecation("Foobar", randomFrom(deprecated));
|
||||
assertThat(field, not(sameInstance(withDeprecations)));
|
||||
assertThat(field.match(randomFrom(values), ParseField.EMPTY_FLAGS), is(true));
|
||||
assertThat(field.match("foo bar", ParseField.EMPTY_FLAGS), is(false));
|
||||
assertThat(field.match(randomFrom(deprecated), ParseField.EMPTY_FLAGS), is(false));
|
||||
assertThat(field.match("barFoo", ParseField.EMPTY_FLAGS), is(false));
|
||||
assertThat(field.match(randomFrom(values), false), is(true));
|
||||
assertThat(field.match("foo bar", false), is(false));
|
||||
assertThat(field.match(randomFrom(deprecated), false), is(false));
|
||||
assertThat(field.match("barFoo", false), is(false));
|
||||
|
||||
assertThat(withDeprecations.match(randomFrom(values), ParseField.EMPTY_FLAGS), is(true));
|
||||
assertThat(withDeprecations.match("foo bar", ParseField.EMPTY_FLAGS), is(false));
|
||||
assertThat(withDeprecations.match(randomFrom(deprecated), ParseField.EMPTY_FLAGS), is(true));
|
||||
assertThat(withDeprecations.match("barFoo", ParseField.EMPTY_FLAGS), is(true));
|
||||
assertThat(withDeprecations.match(randomFrom(values), false), is(true));
|
||||
assertThat(withDeprecations.match("foo bar", false), is(false));
|
||||
assertThat(withDeprecations.match(randomFrom(deprecated), false), is(true));
|
||||
assertThat(withDeprecations.match("barFoo", false), is(true));
|
||||
|
||||
// now with strict mode
|
||||
EnumSet<ParseField.Flag> flags = EnumSet.of(ParseField.Flag.STRICT);
|
||||
assertThat(field.match(randomFrom(values), flags), is(true));
|
||||
assertThat(field.match("foo bar", flags), is(false));
|
||||
assertThat(field.match(randomFrom(deprecated), flags), is(false));
|
||||
assertThat(field.match("barFoo", flags), is(false));
|
||||
assertThat(field.match(randomFrom(values), true), is(true));
|
||||
assertThat(field.match("foo bar", true), is(false));
|
||||
assertThat(field.match(randomFrom(deprecated), true), is(false));
|
||||
assertThat(field.match("barFoo", true), is(false));
|
||||
|
||||
assertThat(withDeprecations.match(randomFrom(values), flags), is(true));
|
||||
assertThat(withDeprecations.match("foo bar", flags), is(false));
|
||||
try {
|
||||
withDeprecations.match(randomFrom(deprecated), flags);
|
||||
fail();
|
||||
} catch (IllegalArgumentException ex) {
|
||||
|
||||
}
|
||||
|
||||
try {
|
||||
withDeprecations.match("barFoo", flags);
|
||||
fail();
|
||||
} catch (IllegalArgumentException ex) {
|
||||
|
||||
}
|
||||
assertThat(withDeprecations.match(randomFrom(values), true), is(true));
|
||||
assertThat(withDeprecations.match("foo bar", true), is(false));
|
||||
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
|
||||
() -> withDeprecations.match(randomFrom(deprecated), true));
|
||||
assertThat(e.getMessage(), containsString("used, expected [foo_bar] instead"));
|
||||
e = expectThrows(IllegalArgumentException.class, () -> withDeprecations.match("barFoo", true));
|
||||
assertThat(e.getMessage(), containsString("Deprecated field [barFoo] used, expected [foo_bar] instead"));
|
||||
}
|
||||
|
||||
public void testAllDeprecated() {
|
||||
|
@ -72,30 +62,29 @@ public class ParseFieldTests extends ESTestCase {
|
|||
|
||||
boolean withDeprecatedNames = randomBoolean();
|
||||
String[] deprecated = new String[]{"text", "same_as_text"};
|
||||
String[] allValues = values;
|
||||
String[] allValues;
|
||||
if (withDeprecatedNames) {
|
||||
String[] newArray = new String[allValues.length + deprecated.length];
|
||||
System.arraycopy(allValues, 0, newArray, 0, allValues.length);
|
||||
System.arraycopy(deprecated, 0, newArray, allValues.length, deprecated.length);
|
||||
String[] newArray = new String[values.length + deprecated.length];
|
||||
System.arraycopy(values, 0, newArray, 0, values.length);
|
||||
System.arraycopy(deprecated, 0, newArray, values.length, deprecated.length);
|
||||
allValues = newArray;
|
||||
} else {
|
||||
allValues = values;
|
||||
}
|
||||
|
||||
ParseField field = new ParseField(randomFrom(values));
|
||||
ParseField field;
|
||||
if (withDeprecatedNames) {
|
||||
field = field.withDeprecation(deprecated);
|
||||
field = new ParseField(randomFrom(values)).withDeprecation(deprecated).withAllDeprecated("like");
|
||||
} else {
|
||||
field = new ParseField(randomFrom(values)).withAllDeprecated("like");
|
||||
}
|
||||
field = field.withAllDeprecated("like");
|
||||
|
||||
// strict mode off
|
||||
assertThat(field.match(randomFrom(allValues), ParseField.EMPTY_FLAGS), is(true));
|
||||
assertThat(field.match("not a field name", ParseField.EMPTY_FLAGS), is(false));
|
||||
assertThat(field.match(randomFrom(allValues), false), is(true));
|
||||
assertThat(field.match("not a field name", false), is(false));
|
||||
|
||||
// now with strict mode
|
||||
EnumSet<ParseField.Flag> flags = EnumSet.of(ParseField.Flag.STRICT);
|
||||
try {
|
||||
field.match(randomFrom(allValues), flags);
|
||||
fail();
|
||||
} catch (IllegalArgumentException ex) {
|
||||
}
|
||||
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> field.match(randomFrom(allValues), true));
|
||||
assertThat(e.getMessage(), containsString(" used, replaced by [like]"));
|
||||
}
|
||||
}
|
||||
|
|
Loading…
Reference in New Issue