Skip to content

Commit 6a27628

Browse files
javannajavanna
committed
Remove support for strict parsing mode
We return deprecation warnings as response headers, besides logging them. Strict parsing mode stayed around, but was only used in query tests, though we also introduced checks for deprecation warnings there that don't need strict parsing anymore (see #20993). We can then safely remove support for strict parsing mode. The final goal is to remove the ParseFieldMatcher class, but there are many many users of it. This commit prepares the field for the removal, by deprecating ParseFieldMatcher and making it effectively not needed. Strict parsing is removed from ParseFieldMatcher, and strict parsing is replaced in tests where needed with deprecation warnings checks. Note that the setting to enable strict parsing was never ported to the new settings infra hance it cannot be set in production. It is really only used in our own tests. Relates to #19552
1 parent 38914f1 commit 6a27628

21 files changed

+172
-230
lines changed

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

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -101,14 +101,10 @@ public ParseField withAllDeprecated(String allReplacedWith) {
101101
/**
102102
* @param fieldName
103103
* the field name to match against this {@link ParseField}
104-
* @param strict
105-
* if true an exception will be thrown if a deprecated field name
106-
* is given. If false the deprecated name will be matched but a
107-
* message will also be logged to the {@link DeprecationLogger}
108104
* @return true if <code>fieldName</code> matches any of the acceptable
109105
* names for this {@link ParseField}.
110106
*/
111-
boolean match(String fieldName, boolean strict) {
107+
public boolean match(String fieldName) {
112108
Objects.requireNonNull(fieldName, "fieldName cannot be null");
113109
// if this parse field has not been completely deprecated then try to
114110
// match the preferred name
@@ -128,11 +124,7 @@ boolean match(String fieldName, boolean strict) {
128124
// message to indicate what should be used instead
129125
msg = "Deprecated field [" + fieldName + "] used, replaced by [" + allReplacedWith + "]";
130126
}
131-
if (strict) {
132-
throw new IllegalArgumentException(msg);
133-
} else {
134-
DEPRECATION_LOGGER.deprecated(msg);
135-
}
127+
DEPRECATION_LOGGER.deprecated(msg);
136128
return true;
137129
}
138130
}

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

Lines changed: 11 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,38 +22,29 @@
2222
import org.elasticsearch.common.settings.Settings;
2323

2424
/**
25-
* Matcher to use in combination with {@link ParseField} while parsing requests. Matches a {@link ParseField}
26-
* against a field name and throw deprecation exception depending on the current value of the {@link #PARSE_STRICT} setting.
25+
* Matcher to use in combination with {@link ParseField} while parsing requests.
26+
*
27+
* @deprecated This class used to be useful to parse in strict mode and emit errors rather than deprecation warnings. Now that we return
28+
* warnings as response headers all the time, it is no longer useful and will soon be removed. The removal is in progress and there is
29+
* already no strict mode in fact. Use {@link ParseField} directly.
2730
*/
31+
@Deprecated
2832
public class ParseFieldMatcher {
29-
public static final String PARSE_STRICT = "index.query.parse.strict";
30-
public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(false);
31-
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(true);
32-
33-
private final boolean strict;
33+
public static final ParseFieldMatcher EMPTY = new ParseFieldMatcher(Settings.EMPTY);
34+
public static final ParseFieldMatcher STRICT = new ParseFieldMatcher(Settings.EMPTY);
3435

3536
public ParseFieldMatcher(Settings settings) {
36-
this(settings.getAsBoolean(PARSE_STRICT, false));
37-
}
38-
39-
public ParseFieldMatcher(boolean strict) {
40-
this.strict = strict;
41-
}
42-
43-
/** Should deprecated settings be rejected? */
44-
public boolean isStrict() {
45-
return strict;
37+
//we don't do anything with the settings argument, this whole class will be soon removed
4638
}
4739

4840
/**
49-
* Matches a {@link ParseField} against a field name, and throws deprecation exception depending on the current
50-
* value of the {@link #PARSE_STRICT} setting.
41+
* Matches a {@link ParseField} against a field name,
5142
* @param fieldName the field name found in the request while parsing
5243
* @param parseField the parse field that we are looking for
5344
* @throws IllegalArgumentException whenever we are in strict mode and the request contained a deprecated field
5445
* @return true whenever the parse field that we are looking for was found, false otherwise
5546
*/
5647
public boolean match(String fieldName, ParseField parseField) {
57-
return parseField.match(fieldName, strict);
48+
return parseField.match(fieldName);
5849
}
5950
}

‎core/src/main/java/org/elasticsearch/index/mapper/TypeParsers.java

Lines changed: 18 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@
2525
import org.elasticsearch.common.joda.FormatDateTimeFormatter;
2626
import org.elasticsearch.common.joda.Joda;
2727
import org.elasticsearch.common.logging.DeprecationLogger;
28-
import org.elasticsearch.common.logging.ESLoggerFactory;
2928
import org.elasticsearch.common.logging.Loggers;
3029
import org.elasticsearch.common.xcontent.support.XContentMapValues;
3130
import org.elasticsearch.index.analysis.NamedAnalyzer;
@@ -43,7 +42,6 @@
4342
import static org.elasticsearch.common.xcontent.support.XContentMapValues.isArray;
4443
import static org.elasticsearch.common.xcontent.support.XContentMapValues.lenientNodeBooleanValue;
4544
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeFloatValue;
46-
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeIntegerValue;
4745
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeMapValue;
4846
import static org.elasticsearch.common.xcontent.support.XContentMapValues.nodeStringValue;
4947

@@ -59,16 +57,11 @@ public class TypeParsers {
5957
private static final Set<String> BOOLEAN_STRINGS = new HashSet<>(Arrays.asList("true", "false"));
6058

6159
public static boolean nodeBooleanValue(String name, Object node, Mapper.TypeParser.ParserContext parserContext) {
62-
// Hook onto ParseFieldMatcher so that parsing becomes strict when setting index.query.parse.strict
63-
if (parserContext.parseFieldMatcher().isStrict()) {
64-
return XContentMapValues.nodeBooleanValue(node);
65-
} else {
66-
// TODO: remove this leniency in 6.0
67-
if (BOOLEAN_STRINGS.contains(node.toString()) == false) {
68-
DEPRECATION_LOGGER.deprecated("Expected a boolean for property [{}] but got [{}]", name, node);
69-
}
70-
return XContentMapValues.lenientNodeBooleanValue(node);
60+
// TODO: remove this leniency in 6.0
61+
if (BOOLEAN_STRINGS.contains(node.toString()) == false) {
62+
DEPRECATION_LOGGER.deprecated("Expected a boolean for property [{}] but got [{}]", name, node);
7163
}
64+
return XContentMapValues.lenientNodeBooleanValue(node);
7265
}
7366

7467
private static void parseAnalyzersAndTermVectors(FieldMapper.Builder builder, String name, Map<String, Object> fieldNode, Mapper.TypeParser.ParserContext parserContext) {
@@ -211,10 +204,10 @@ public static void parseField(FieldMapper.Builder builder, String name, Map<Stri
211204
throw new MapperParsingException("[" + propName + "] must not have a [null] value");
212205
}
213206
if (propName.equals("store")) {
214-
builder.store(parseStore(name, propNode.toString(), parserContext));
207+
builder.store(parseStore(propNode.toString()));
215208
iterator.remove();
216209
} else if (propName.equals("index")) {
217-
builder.index(parseIndex(name, propNode.toString(), parserContext));
210+
builder.index(parseIndex(name, propNode.toString()));
218211
iterator.remove();
219212
} else if (propName.equals(DOC_VALUES)) {
220213
builder.docValues(nodeBooleanValue(DOC_VALUES, propNode, parserContext));
@@ -346,7 +339,7 @@ public static void parseTermVector(String fieldName, String termVector, FieldMap
346339
}
347340
}
348341

349-
public static boolean parseIndex(String fieldName, String index, Mapper.TypeParser.ParserContext parserContext) throws MapperParsingException {
342+
private static boolean parseIndex(String fieldName, String index) throws MapperParsingException {
350343
switch (index) {
351344
case "true":
352345
return true;
@@ -355,31 +348,23 @@ public static boolean parseIndex(String fieldName, String index, Mapper.TypePars
355348
case "not_analyzed":
356349
case "analyzed":
357350
case "no":
358-
if (parserContext.parseFieldMatcher().isStrict() == false) {
359-
DEPRECATION_LOGGER.deprecated("Expected a boolean for property [index] but got [{}]", index);
360-
return "no".equals(index) == false;
361-
} else {
362-
throw new IllegalArgumentException("Can't parse [index] value [" + index + "] for field [" + fieldName + "], expected [true] or [false]");
363-
}
351+
DEPRECATION_LOGGER.deprecated("Expected a boolean for property [index] but got [{}]", index);
352+
return "no".equals(index) == false;
364353
default:
365354
throw new IllegalArgumentException("Can't parse [index] value [" + index + "] for field [" + fieldName + "], expected [true] or [false]");
366355
}
367356
}
368357

369-
public static boolean parseStore(String fieldName, String store, Mapper.TypeParser.ParserContext parserContext) throws MapperParsingException {
370-
if (parserContext.parseFieldMatcher().isStrict()) {
371-
return XContentMapValues.nodeBooleanValue(store);
358+
private static boolean parseStore(String store) throws MapperParsingException {
359+
if (BOOLEAN_STRINGS.contains(store) == false) {
360+
DEPRECATION_LOGGER.deprecated("Expected a boolean for property [store] but got [{}]", store);
361+
}
362+
if ("no".equals(store)) {
363+
return false;
364+
} else if ("yes".equals(store)) {
365+
return true;
372366
} else {
373-
if (BOOLEAN_STRINGS.contains(store) == false) {
374-
DEPRECATION_LOGGER.deprecated("Expected a boolean for property [store] but got [{}]", store);
375-
}
376-
if ("no".equals(store)) {
377-
return false;
378-
} else if ("yes".equals(store)) {
379-
return true;
380-
} else {
381-
return lenientNodeBooleanValue(store);
382-
}
367+
return lenientNodeBooleanValue(store);
383368
}
384369
}
385370

‎core/src/main/java/org/elasticsearch/index/query/GeoBoundingBoxQueryBuilder.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ public class GeoBoundingBoxQueryBuilder extends AbstractQueryBuilder<GeoBounding
6262
private static final ParseField TYPE_FIELD = new ParseField("type");
6363
private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
6464
private static final ParseField COERCE_FIELD =new ParseField("coerce", "normalize")
65-
.withAllDeprecated("use field validation_method instead");
65+
.withAllDeprecated("validation_method");
6666
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
67-
.withAllDeprecated("use field validation_method instead");
67+
.withAllDeprecated("validation_method");
6868
private static final ParseField FIELD_FIELD = new ParseField("field");
6969
private static final ParseField TOP_FIELD = new ParseField("top");
7070
private static final ParseField BOTTOM_FIELD = new ParseField("bottom");

‎core/src/main/java/org/elasticsearch/index/query/GeoDistanceQueryBuilder.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,10 +66,8 @@ public class GeoDistanceQueryBuilder extends AbstractQueryBuilder<GeoDistanceQue
6666
public static final boolean DEFAULT_IGNORE_UNMAPPED = false;
6767

6868
private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
69-
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
70-
.withAllDeprecated("use validation_method instead");
71-
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
72-
.withAllDeprecated("use validation_method instead");
69+
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed").withAllDeprecated("validation_method");
70+
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize").withAllDeprecated("validation_method");
7371
@Deprecated
7472
private static final ParseField OPTIMIZE_BBOX_FIELD = new ParseField("optimize_bbox")
7573
.withAllDeprecated("no replacement: `optimize_bbox` is no longer supported due to recent improvements");

‎core/src/main/java/org/elasticsearch/index/query/GeoPolygonQueryBuilder.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,10 +49,8 @@ public class GeoPolygonQueryBuilder extends AbstractQueryBuilder<GeoPolygonQuery
4949
*/
5050
public static final boolean DEFAULT_IGNORE_UNMAPPED = false;
5151

52-
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
53-
.withAllDeprecated("use validation_method instead");
54-
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
55-
.withAllDeprecated("use validation_method instead");
52+
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize").withAllDeprecated("validation_method");
53+
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed").withAllDeprecated("validation_method");
5654
private static final ParseField VALIDATION_METHOD = new ParseField("validation_method");
5755
private static final ParseField POINTS_FIELD = new ParseField("points");
5856
private static final ParseField IGNORE_UNMAPPED_FIELD = new ParseField("ignore_unmapped");

‎core/src/main/java/org/elasticsearch/search/sort/GeoDistanceSortBuilder.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,10 +74,8 @@ public class GeoDistanceSortBuilder extends SortBuilder<GeoDistanceSortBuilder>
7474
private static final ParseField UNIT_FIELD = new ParseField("unit");
7575
private static final ParseField DISTANCE_TYPE_FIELD = new ParseField("distance_type");
7676
private static final ParseField VALIDATION_METHOD_FIELD = new ParseField("validation_method");
77-
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed")
78-
.withAllDeprecated("use validation_method instead");
79-
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize")
80-
.withAllDeprecated("use validation_method instead");
77+
private static final ParseField IGNORE_MALFORMED_FIELD = new ParseField("ignore_malformed").withAllDeprecated("validation_method");
78+
private static final ParseField COERCE_FIELD = new ParseField("coerce", "normalize").withAllDeprecated("validation_method");
8179
private static final ParseField SORTMODE_FIELD = new ParseField("mode", "sort_mode");
8280

8381
private final String fieldName;

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

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

2121
import org.elasticsearch.test.ESTestCase;
2222

23-
import static org.hamcrest.CoreMatchers.containsString;
2423
import static org.hamcrest.CoreMatchers.is;
2524
import static org.hamcrest.CoreMatchers.not;
2625
import static org.hamcrest.CoreMatchers.sameInstance;
@@ -33,32 +32,16 @@ public void testParse() {
3332
String[] deprecated = new String[]{"barFoo", "bar_foo", "Foobar"};
3433
ParseField withDeprecations = field.withDeprecation(deprecated);
3534
assertThat(field, not(sameInstance(withDeprecations)));
36-
assertThat(field.match(name, false), is(true));
37-
assertThat(field.match("foo bar", false), is(false));
35+
assertThat(field.match(name), is(true));
36+
assertThat(field.match("foo bar"), is(false));
3837
for (String deprecatedName : deprecated) {
39-
assertThat(field.match(deprecatedName, false), is(false));
38+
assertThat(field.match(deprecatedName), is(false));
4039
}
4140

42-
assertThat(withDeprecations.match(name, false), is(true));
43-
assertThat(withDeprecations.match("foo bar", false), is(false));
41+
assertThat(withDeprecations.match(name), is(true));
42+
assertThat(withDeprecations.match("foo bar"), is(false));
4443
for (String deprecatedName : deprecated) {
45-
assertThat(withDeprecations.match(deprecatedName, false), is(true));
46-
}
47-
48-
// now with strict mode
49-
assertThat(field.match(name, true), is(true));
50-
assertThat(field.match("foo bar", true), is(false));
51-
for (String deprecatedName : deprecated) {
52-
assertThat(field.match(deprecatedName, true), is(false));
53-
}
54-
55-
assertThat(withDeprecations.match(name, true), is(true));
56-
assertThat(withDeprecations.match("foo bar", true), is(false));
57-
for (String deprecatedName : deprecated) {
58-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> {
59-
withDeprecations.match(deprecatedName, true);
60-
});
61-
assertThat(e.getMessage(), containsString("used, expected [foo_bar] instead"));
44+
assertThat(withDeprecations.match(deprecatedName), is(true));
6245
}
6346
}
6447

@@ -84,13 +67,8 @@ public void testAllDeprecated() {
8467
field = new ParseField(name).withAllDeprecated("like");
8568
}
8669

87-
// strict mode off
88-
assertThat(field.match(randomFrom(allValues), false), is(true));
89-
assertThat(field.match("not a field name", false), is(false));
90-
91-
// now with strict mode
92-
IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> field.match(randomFrom(allValues), true));
93-
assertThat(e.getMessage(), containsString(" used, replaced by [like]"));
70+
assertThat(field.match(randomFrom(allValues)), is(true));
71+
assertThat(field.match("not a field name"), is(false));
9472
}
9573

9674
public void testGetAllNamesIncludedDeprecated() {

‎core/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java

Lines changed: 22 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
import org.elasticsearch.common.ParseFieldMatcher;
2323
import org.elasticsearch.common.ParseFieldMatcherSupplier;
2424
import org.elasticsearch.common.ParsingException;
25+
import org.elasticsearch.common.logging.DeprecationLogger;
26+
import org.elasticsearch.common.settings.Settings;
27+
import org.elasticsearch.common.util.concurrent.ThreadContext;
2528
import org.elasticsearch.common.xcontent.AbstractObjectParser.NoContextParser;
2629
import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser;
2730
import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
@@ -35,6 +38,8 @@
3538
import java.util.Arrays;
3639
import java.util.List;
3740

41+
import static org.hamcrest.Matchers.equalTo;
42+
import static org.hamcrest.Matchers.hasItem;
3843
import static org.hamcrest.Matchers.hasSize;
3944

4045
public class ObjectParserTests extends ESTestCase {
@@ -218,27 +223,27 @@ public void setTest(int test) {
218223
}
219224
}
220225

221-
public void testDeprecationFail() throws IOException {
222-
XContentParser parser = createParser(JsonXContent.jsonXContent, "{\"old_test\" : \"foo\"}");
223-
class TestStruct {
224-
public String test;
225-
}
226-
ObjectParser<TestStruct, ParseFieldMatcherSupplier> objectParser = new ObjectParser<>("foo");
227-
TestStruct s = new TestStruct();
226+
public void testDeprecationWarnings() throws IOException {
227+
try (ThreadContext threadContext = new ThreadContext(Settings.EMPTY)) {
228+
DeprecationLogger.setThreadContext(threadContext);
229+
class TestStruct {
230+
public String test;
231+
}
232+
ObjectParser<TestStruct, ParseFieldMatcherSupplier> objectParser = new ObjectParser<>("foo");
233+
TestStruct s = new TestStruct();
228234

229-
objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING);
235+
objectParser.declareField((i, v, c) -> v.test = i.text(), new ParseField("test", "old_test"), ObjectParser.ValueType.STRING);
230236

231-
try {
232-
objectParser.parse(parser, s, STRICT_PARSING);
233-
fail("deprecated value");
234-
} catch (IllegalArgumentException ex) {
235-
assertEquals(ex.getMessage(), "Deprecated field [old_test] used, expected [test] instead");
237+
XContentParser parser = createParser(XContentType.JSON.xContent(), "{\"old_test\" : \"foo\"}");
238+
objectParser.parse(parser, s, () -> ParseFieldMatcher.EMPTY);
239+
240+
assertEquals("foo", s.test);
236241

242+
final List<String> warnings = threadContext.getResponseHeaders().get(DeprecationLogger.DEPRECATION_HEADER);
243+
assertThat(warnings, hasSize(1));
244+
assertThat(warnings, hasItem(equalTo("Deprecated field [old_test] used, expected [test] instead")));
245+
DeprecationLogger.removeThreadContext(threadContext);
237246
}
238-
assertNull(s.test);
239-
parser = createParser(JsonXContent.jsonXContent, "{\"old_test\" : \"foo\"}");
240-
objectParser.parse(parser, s, () -> ParseFieldMatcher.EMPTY);
241-
assertEquals("foo", s.test);
242247
}
243248

244249
public void testFailOnValueType() throws IOException {

0 commit comments

Comments
 (0)