Skip to content

Commit 026519e

Browse files
jpountzjpountz
committed
ParseFieldMatcher should log when using deprecated settings. #16988
I always thought ParseFieldMatcher would log when using a deprecated setting, but it does not.
1 parent 3d13c27 commit 026519e

File tree

3 files changed

+59
-72
lines changed

3 files changed

+59
-72
lines changed

‎core/src/main/java/org/elasticsearch/common/ParseField.java‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,26 +18,23 @@
1818
*/
1919
package org.elasticsearch.common;
2020

21+
import org.elasticsearch.common.logging.DeprecationLogger;
22+
import org.elasticsearch.common.logging.Loggers;
2123

22-
import java.util.EnumSet;
2324
import java.util.HashSet;
2425

2526
/**
2627
* Holds a field that can be found in a request while parsing and its different variants, which may be deprecated.
2728
*/
2829
public class ParseField {
30+
31+
private static final DeprecationLogger DEPRECATION_LOGGER = new DeprecationLogger(Loggers.getLogger(ParseField.class));
32+
2933
private final String camelCaseName;
3034
private final String underscoreName;
3135
private final String[] deprecatedNames;
3236
private String allReplacedWith = null;
3337

34-
static final EnumSet<Flag> EMPTY_FLAGS = EnumSet.noneOf(Flag.class);
35-
static final EnumSet<Flag> STRICT_FLAGS = EnumSet.of(Flag.STRICT);
36-
37-
enum Flag {
38-
STRICT
39-
}
40-
4138
public ParseField(String value, String... deprecatedNames) {
4239
camelCaseName = Strings.toCamelCase(value);
4340
underscoreName = Strings.toUnderscoreCase(value);
@@ -80,19 +77,21 @@ public ParseField withAllDeprecated(String allReplacedWith) {
8077
return parseField;
8178
}
8279

83-
boolean match(String currentFieldName, EnumSet<Flag> flags) {
80+
boolean match(String currentFieldName, boolean strict) {
8481
if (allReplacedWith == null && (currentFieldName.equals(camelCaseName) || currentFieldName.equals(underscoreName))) {
8582
return true;
8683
}
8784
String msg;
8885
for (String depName : deprecatedNames) {
8986
if (currentFieldName.equals(depName)) {
90-
if (flags.contains(Flag.STRICT)) {
91-
msg = "Deprecated field [" + currentFieldName + "] used, expected [" + underscoreName + "] instead";
92-
if (allReplacedWith != null) {
93-
msg = "Deprecated field [" + currentFieldName + "] used, replaced by [" + allReplacedWith + "]";
94-
}
87+
msg = "Deprecated field [" + currentFieldName + "] used, expected [" + underscoreName + "] instead";
88+
if (allReplacedWith != null) {
89+
msg = "Deprecated field [" + currentFieldName + "] used, replaced by [" + allReplacedWith + "]";
90+
}
91+
if (strict) {
9592
throw new IllegalArgumentException(msg);
93+
} else {
94+
DEPRECATION_LOGGER.deprecated(msg);
9695
}
9796
return true;
9897
}

‎core/src/main/java/org/elasticsearch/common/ParseFieldMatcher.java‎

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,29 +21,28 @@
2121

2222
import org.elasticsearch.common.settings.Settings;
2323

24-
import java.util.EnumSet;
25-
2624
/**
2725
* Matcher to use in combination with {@link ParseField} while parsing requests. Matches a {@link ParseField}
2826
* against a field name and throw deprecation exception depending on the current value of the {@link #PARSE_STRICT} setting.
2927
*/
3028
public class ParseFieldMatcher {
3129
public static final String PARSE_STRICT = "index.query.parse.strict";
32-
public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(ParseField.EMPTY_FLAGS);
33-
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(ParseField.STRICT_FLAGS);
30+
public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(false);
31+
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(true);
3432

35-
private final EnumSet<ParseField.Flag> parseFlags;
33+
private final boolean strict;
3634

3735
public ParseFieldMatcher(Settings settings) {
38-
if (settings.getAsBoolean(PARSE_STRICT, false)) {
39-
this.parseFlags = EnumSet.of(ParseField.Flag.STRICT);
40-
} else {
41-
this.parseFlags = ParseField.EMPTY_FLAGS;
42-
}
36+
this(settings.getAsBoolean(PARSE_STRICT, false));
37+
}
38+
39+
public ParseFieldMatcher(boolean strict) {
40+
this.strict = strict;
4341
}
4442

45-
public ParseFieldMatcher(EnumSet<ParseField.Flag> parseFlags) {
46-
this.parseFlags = parseFlags;
43+
/** Should deprecated settings be rejected? */
44+
public boolean isStrict() {
45+
return strict;
4746
}
4847

4948
/**
@@ -55,6 +54,6 @@ public ParseFieldMatcher(EnumSet<ParseField.Flag> parseFlags) {
5554
* @return true whenever the parse field that we are looking for was found, false otherwise
5655
*/
5756
public boolean match(String fieldName, ParseField parseField) {
58-
return parseField.match(fieldName, parseFlags);
57+
return parseField.match(fieldName, strict);
5958
}
6059
}

‎core/src/test/java/org/elasticsearch/common/ParseFieldTests.java‎

Lines changed: 34 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@
2020

2121
import org.elasticsearch.test.ESTestCase;
2222

23-
import java.util.EnumSet;
24-
23+
import static org.hamcrest.CoreMatchers.containsString;
2524
import static org.hamcrest.CoreMatchers.is;
2625
import static org.hamcrest.CoreMatchers.not;
2726
import static org.hamcrest.CoreMatchers.sameInstance;
@@ -33,69 +32,59 @@ public void testParse() {
3332
String[] deprecated = new String[]{"barFoo", "bar_foo"};
3433
ParseField withDeprecations = field.withDeprecation("Foobar", randomFrom(deprecated));
3534
assertThat(field, not(sameInstance(withDeprecations)));
36-
assertThat(field.match(randomFrom(values), ParseField.EMPTY_FLAGS), is(true));
37-
assertThat(field.match("foo bar", ParseField.EMPTY_FLAGS), is(false));
38-
assertThat(field.match(randomFrom(deprecated), ParseField.EMPTY_FLAGS), is(false));
39-
assertThat(field.match("barFoo", ParseField.EMPTY_FLAGS), is(false));
35+
assertThat(field.match(randomFrom(values), false), is(true));
36+
assertThat(field.match("foo bar", false), is(false));
37+
assertThat(field.match(randomFrom(deprecated), false), is(false));
38+
assertThat(field.match("barFoo", false), is(false));
4039

41-
assertThat(withDeprecations.match(randomFrom(values), ParseField.EMPTY_FLAGS), is(true));
42-
assertThat(withDeprecations.match("foo bar", ParseField.EMPTY_FLAGS), is(false));
43-
assertThat(withDeprecations.match(randomFrom(deprecated), ParseField.EMPTY_FLAGS), is(true));
44-
assertThat(withDeprecations.match("barFoo", ParseField.EMPTY_FLAGS), is(true));
40+
assertThat(withDeprecations.match(randomFrom(values), false), is(true));
41+
assertThat(withDeprecations.match("foo bar", false), is(false));
42+
assertThat(withDeprecations.match(randomFrom(deprecated), false), is(true));
43+
assertThat(withDeprecations.match("barFoo", false), is(true));
4544

4645
// now with strict mode
47-
EnumSet<ParseField.Flag> flags = EnumSet.of(ParseField.Flag.STRICT);
48-
assertThat(field.match(randomFrom(values), flags), is(true));
49-
assertThat(field.match("foo bar", flags), is(false));
50-
assertThat(field.match(randomFrom(deprecated), flags), is(false));
51-
assertThat(field.match("barFoo", flags), is(false));
52-
53-
assertThat(withDeprecations.match(randomFrom(values), flags), is(true));
54-
assertThat(withDeprecations.match("foo bar", flags), is(false));
55-
try {
56-
withDeprecations.match(randomFrom(deprecated), flags);
57-
fail();
58-
} catch (IllegalArgumentException ex) {
59-
60-
}
46+
assertThat(field.match(randomFrom(values), true), is(true));
47+
assertThat(field.match("foo bar", true), is(false));
48+
assertThat(field.match(randomFrom(deprecated), true), is(false));
49+
assertThat(field.match("barFoo", true), is(false));
6150

62-
try {
63-
withDeprecations.match("barFoo", flags);
64-
fail();
65-
} catch (IllegalArgumentException ex) {
66-
67-
}
51+
assertThat(withDeprecations.match(randomFrom(values), true), is(true));
52+
assertThat(withDeprecations.match("foo bar", true), is(false));
53+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class,
54+
() -> withDeprecations.match(randomFrom(deprecated), true));
55+
assertThat(e.getMessage(), containsString("used, expected [foo_bar] instead"));
56+
e = expectThrows(IllegalArgumentException.class, () -> withDeprecations.match("barFoo", true));
57+
assertThat(e.getMessage(), containsString("Deprecated field [barFoo] used, expected [foo_bar] instead"));
6858
}
6959

7060
public void testAllDeprecated() {
7161
String[] values = new String[]{"like_text", "likeText"};
7262

7363
boolean withDeprecatedNames = randomBoolean();
7464
String[] deprecated = new String[]{"text", "same_as_text"};
75-
String[] allValues = values;
65+
String[] allValues;
7666
if (withDeprecatedNames) {
77-
String[] newArray = new String[allValues.length + deprecated.length];
78-
System.arraycopy(allValues, 0, newArray, 0, allValues.length);
79-
System.arraycopy(deprecated, 0, newArray, allValues.length, deprecated.length);
67+
String[] newArray = new String[values.length + deprecated.length];
68+
System.arraycopy(values, 0, newArray, 0, values.length);
69+
System.arraycopy(deprecated, 0, newArray, values.length, deprecated.length);
8070
allValues = newArray;
71+
} else {
72+
allValues = values;
8173
}
8274

83-
ParseField field = new ParseField(randomFrom(values));
75+
ParseField field;
8476
if (withDeprecatedNames) {
85-
field = field.withDeprecation(deprecated);
77+
field = new ParseField(randomFrom(values)).withDeprecation(deprecated).withAllDeprecated("like");
78+
} else {
79+
field = new ParseField(randomFrom(values)).withAllDeprecated("like");
8680
}
87-
field = field.withAllDeprecated("like");
8881

8982
// strict mode off
90-
assertThat(field.match(randomFrom(allValues), ParseField.EMPTY_FLAGS), is(true));
91-
assertThat(field.match("not a field name", ParseField.EMPTY_FLAGS), is(false));
83+
assertThat(field.match(randomFrom(allValues), false), is(true));
84+
assertThat(field.match("not a field name", false), is(false));
9285

9386
// now with strict mode
94-
EnumSet<ParseField.Flag> flags = EnumSet.of(ParseField.Flag.STRICT);
95-
try {
96-
field.match(randomFrom(allValues), flags);
97-
fail();
98-
} catch (IllegalArgumentException ex) {
99-
}
87+
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> field.match(randomFrom(allValues), true));
88+
assertThat(e.getMessage(), containsString(" used, replaced by [like]"));
10089
}
10190
}

0 commit comments

Comments
 (0)