Skip to content

Commit 8d09fbf

Browse files
pgomulkapgomulka
authored
Allow for field declaration for future rest versions (#69774)
When renaming/removing a field, a new field might be declared which should be parseable starting with the current version. This commit changes the way ParseField is declared for compatible Version. Instead of concrete version a boolean function has to be used to indicate for what version a field is parseable. The onOrAfter and equalTo functions are declared on RestApiVersion to allow for this.
1 parent f93242a commit 8d09fbf

File tree

7 files changed

+98
-36
lines changed

7 files changed

+98
-36
lines changed

‎libs/core/src/main/java/org/elasticsearch/common/RestApiVersion.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@
88

99
package org.elasticsearch.common;
1010

11+
import java.util.function.Function;
12+
1113
/**
1214
* A enum representing versions of the REST API (particularly with regard to backwards compatibility).
1315
*
@@ -30,6 +32,10 @@ public RestApiVersion previous() {
3032
return fromMajorVersion(major - 1);
3133
}
3234

35+
public boolean matches(Function<RestApiVersion, Boolean> restApiVersionFunctions){
36+
return restApiVersionFunctions.apply(this);
37+
}
38+
3339
private static RestApiVersion fromMajorVersion(int majorVersion) {
3440
return valueOf("V_" + majorVersion);
3541
}
@@ -42,4 +48,12 @@ public static RestApiVersion current() {
4248
return CURRENT;
4349
}
4450

51+
public static Function<RestApiVersion, Boolean> equalTo(RestApiVersion restApiVersion) {
52+
return r -> r.major == restApiVersion.major;
53+
}
54+
55+
public static Function<RestApiVersion, Boolean> onOrAfter(RestApiVersion restApiVersion) {
56+
return r -> r.major >= restApiVersion.major;
57+
}
58+
4559
}

‎libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java‎

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,11 @@
1010
import org.elasticsearch.common.xcontent.DeprecationHandler;
1111
import org.elasticsearch.common.xcontent.XContentLocation;
1212

13-
import java.util.Arrays;
14-
import java.util.Collection;
1513
import java.util.Collections;
1614
import java.util.HashSet;
17-
import java.util.List;
1815
import java.util.Objects;
1916
import java.util.Set;
17+
import java.util.function.Function;
2018
import java.util.function.Supplier;
2119

2220
/**
@@ -26,15 +24,15 @@
2624
public class ParseField {
2725
private final String name;
2826
private final String[] deprecatedNames;
29-
private final Set<RestApiVersion> restApiVersions = new HashSet<>(2);
27+
private final Function<RestApiVersion, Boolean> forRestApiVersion;
3028
private String allReplacedWith = null;
3129
private final String[] allNames;
3230
private boolean fullyDeprecated = false;
3331

3432
private static final String[] EMPTY = new String[0];
3533

3634

37-
private ParseField(String name, Collection<RestApiVersion> restApiVersions, String[] deprecatedNames) {
35+
private ParseField(String name, Function<RestApiVersion, Boolean> forRestApiVersion, String[] deprecatedNames) {
3836
this.name = name;
3937
if (deprecatedNames == null || deprecatedNames.length == 0) {
4038
this.deprecatedNames = EMPTY;
@@ -43,7 +41,7 @@ private ParseField(String name, Collection<RestApiVersion> restApiVersions, Stri
4341
Collections.addAll(set, deprecatedNames);
4442
this.deprecatedNames = set.toArray(new String[set.size()]);
4543
}
46-
this.restApiVersions.addAll(restApiVersions);
44+
this.forRestApiVersion = forRestApiVersion;
4745

4846
Set<String> allNames = new HashSet<>();
4947
allNames.add(name);
@@ -59,7 +57,7 @@ private ParseField(String name, Collection<RestApiVersion> restApiVersions, Stri
5957
* accepted when strict matching is used.
6058
*/
6159
public ParseField(String name, String... deprecatedNames) {
62-
this(name, List.of(RestApiVersion.current(), RestApiVersion.minimumSupported()) ,deprecatedNames);
60+
this(name, RestApiVersion.onOrAfter(RestApiVersion.minimumSupported()) ,deprecatedNames);
6361
}
6462

6563
/**
@@ -90,18 +88,18 @@ public ParseField withDeprecation(String... deprecatedNames) {
9088

9189

9290
/**
93-
* Creates a new field with current name and deprecatedNames, but overrides restApiVersions
94-
* @param restApiVersions rest api versions which specifies when a lookup will be allowed
91+
* Creates a new field with current name and deprecatedNames, but overrides forRestApiVersion
92+
* @param forRestApiVersion - a boolean function indicating if a field is for the given RestApiVersion
9593
*/
96-
public ParseField withRestApiVersions(RestApiVersion... restApiVersions) {
97-
return new ParseField(this.name, Arrays.asList(restApiVersions), this.deprecatedNames);
94+
public ParseField forRestApiVersion(Function<RestApiVersion, Boolean> forRestApiVersion) {
95+
return new ParseField(this.name, forRestApiVersion, this.deprecatedNames);
9896
}
9997

10098
/**
101-
* @return rest api versions under which a lookup will be allowed
99+
* @return a function indicating for which RestApiVersion a field is declared for
102100
*/
103-
public Set<RestApiVersion> getRestApiVersions() {
104-
return restApiVersions;
101+
public Function<RestApiVersion, Boolean> getForRestApiVersion() {
102+
return forRestApiVersion;
105103
}
106104

107105
/**

‎libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -336,10 +336,14 @@ private boolean isConstructorArg(BiConsumer<?, ?> consumer) {
336336
private Map<RestApiVersion, Integer> addConstructorArg(BiConsumer<?, ?> consumer, ParseField parseField) {
337337

338338
boolean required = consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER;
339-
for (RestApiVersion restApiVersion : parseField.getRestApiVersions()) {
340339

341-
constructorArgInfos.computeIfAbsent(restApiVersion, (v)-> new ArrayList<>())
342-
.add(new ConstructorArgInfo(parseField, required));
340+
if (RestApiVersion.minimumSupported().matches(parseField.getForRestApiVersion())) {
341+
constructorArgInfos.computeIfAbsent(RestApiVersion.minimumSupported(), (v)-> new ArrayList<>())
342+
.add(new ConstructorArgInfo(parseField, required));
343+
}
344+
if (RestApiVersion.current().matches(parseField.getForRestApiVersion())) {
345+
constructorArgInfos.computeIfAbsent(RestApiVersion.current(), (v)-> new ArrayList<>())
346+
.add(new ConstructorArgInfo(parseField, required));
343347
}
344348

345349
//calculate the positions for the arguments

‎libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -366,13 +366,15 @@ public void declareField(Parser<Value, Context> p, ParseField parseField, ValueT
366366
}
367367
FieldParser fieldParser = new FieldParser(p, type.supportedTokens(), parseField, type);
368368
for (String fieldValue : parseField.getAllNamesIncludedDeprecated()) {
369-
if (parseField.getRestApiVersions().contains(RestApiVersion.minimumSupported())) {
370-
fieldParserMap.putIfAbsent(RestApiVersion.minimumSupported(), new HashMap<>());
371-
fieldParserMap.get(RestApiVersion.minimumSupported()).putIfAbsent(fieldValue, fieldParser);
369+
370+
if (RestApiVersion.minimumSupported().matches(parseField.getForRestApiVersion())) {
371+
fieldParserMap.computeIfAbsent(RestApiVersion.minimumSupported(), (v)-> new HashMap<>())
372+
.putIfAbsent(fieldValue, fieldParser);
372373
}
373-
if (parseField.getRestApiVersions().contains(RestApiVersion.current())) {
374-
fieldParserMap.putIfAbsent(RestApiVersion.current(), new HashMap<>());
375-
fieldParserMap.get(RestApiVersion.current()).putIfAbsent(fieldValue, fieldParser);
374+
if (RestApiVersion.current().matches(parseField.getForRestApiVersion())) {
375+
fieldParserMap.computeIfAbsent(RestApiVersion.current(), (v)-> new HashMap<>())
376+
.putIfAbsent(fieldValue, fieldParser);
377+
376378
}
377379
}
378380

‎libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -573,15 +573,17 @@ public static class StructWithCompatibleFields {
573573
// The declaration is only available for lookup when parser has compatibility set
574574
PARSER.declareInt(constructorArg(),
575575
new ParseField("new_name", "old_name")
576-
.withRestApiVersions(RestApiVersion.minimumSupported()));
576+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())));
577577

578578
// declare `new_name` to be parsed when compatibility is NOT used
579579
PARSER.declareInt(constructorArg(),
580-
new ParseField("new_name").withRestApiVersions(RestApiVersion.current()));
580+
new ParseField("new_name")
581+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.current())));
581582

582583
// declare `old_name` to throw exception when compatibility is NOT used
583584
PARSER.declareInt((r,s) -> failWithException(),
584-
new ParseField("old_name").withRestApiVersions(RestApiVersion.current()));
585+
new ParseField("old_name")
586+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.current())));
585587
}
586588
private int intField;
587589

@@ -649,11 +651,12 @@ public static class StructRemovalField {
649651
// The deprecation shoudl be done manually
650652
PARSER.declareInt(logWarningDoNothing("old_name"),
651653
new ParseField("old_name")
652-
.withRestApiVersions(RestApiVersion.minimumSupported()));
654+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())));
653655

654656
// declare `old_name` to throw exception when compatibility is NOT used
655657
PARSER.declareInt((r,s) -> failWithException(),
656-
new ParseField("old_name").withRestApiVersions(RestApiVersion.current()));
658+
new ParseField("old_name")
659+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.current())));
657660
}
658661

659662
private final String secondField;

‎libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java‎

Lines changed: 43 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99

1010
import org.elasticsearch.common.CheckedFunction;
1111
import org.elasticsearch.common.ParseField;
12-
import org.elasticsearch.common.Strings;
1312
import org.elasticsearch.common.RestApiVersion;
13+
import org.elasticsearch.common.Strings;
1414
import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser;
1515
import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
1616
import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -1018,15 +1018,17 @@ public static class StructWithCompatibleFields {
10181018
// The declaration is only available for lookup when parser has compatibility set
10191019
PARSER.declareInt(StructWithCompatibleFields::setIntField,
10201020
new ParseField("new_name", "old_name")
1021-
.withRestApiVersions(RestApiVersion.minimumSupported()));
1021+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())));
10221022

10231023
// declare `new_name` to be parsed when compatibility is NOT used
10241024
PARSER.declareInt(StructWithCompatibleFields::setIntField,
1025-
new ParseField("new_name").withRestApiVersions(RestApiVersion.current()));
1025+
new ParseField("new_name")
1026+
.forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.current())));
10261027

10271028
// declare `old_name` to throw exception when compatibility is NOT used
10281029
PARSER.declareInt((r,s) -> failWithException(),
1029-
new ParseField("old_name").withRestApiVersions(RestApiVersion.current()));
1030+
new ParseField("old_name")
1031+
.forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.current())));
10301032
}
10311033

10321034
private static void failWithException() {
@@ -1063,7 +1065,6 @@ public void testCompatibleFieldDeclarations() throws IOException {
10631065
assertEquals(1, o.intField);
10641066
}
10651067
{
1066-
10671068
// old_name is allowed to be parsed with compatibility, but results in deprecation
10681069
XContentParser parser = createParserWithCompatibilityFor(JsonXContent.jsonXContent, "{\"old_name\": 1}",
10691070
RestApiVersion.minimumSupported());
@@ -1074,4 +1075,41 @@ public void testCompatibleFieldDeclarations() throws IOException {
10741075

10751076
}
10761077
}
1078+
public static class StructWithOnOrAfterField {
1079+
// real usage would have exact version like RestApiVersion.V_7 (equal to current version) instead of minimumSupported
1080+
1081+
static final ObjectParser<StructWithOnOrAfterField, Void> PARSER =
1082+
new ObjectParser<>("struct_with_on_or_after_field", StructWithOnOrAfterField::new);
1083+
static {
1084+
1085+
// in real usage you would use a real version like RestApiVersion.V_8 and expect it to parse for version V_9, V_10 etc
1086+
PARSER.declareInt(StructWithOnOrAfterField::setIntField,
1087+
new ParseField("new_name")
1088+
.forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())));
1089+
1090+
}
1091+
1092+
private int intField;
1093+
1094+
private void setIntField(int intField) {
1095+
this.intField = intField;
1096+
}
1097+
}
1098+
1099+
public void testFieldsForVersionsOnOrAfter() throws IOException {
1100+
// this test needs to verify that a field declared in version N will be available in version N+1
1101+
// to do this, we assume a version N is minimum (so that the test passes for future releases) and the N+1 is current()
1102+
1103+
// new name is accessed in "current" version - lets assume the current is minimumSupported
1104+
XContentParser parser = createParserWithCompatibilityFor(JsonXContent.jsonXContent, "{\"new_name\": 1}",
1105+
RestApiVersion.minimumSupported());
1106+
StructWithOnOrAfterField o1 = StructWithOnOrAfterField.PARSER.parse(parser, null);
1107+
assertEquals(1, o1.intField);
1108+
1109+
// new name is accessed in "future" version - lets assume the future is currentVersion (minimumSupported+1)
1110+
XContentParser futureParser = createParserWithCompatibilityFor(JsonXContent.jsonXContent, "{\"new_name\": 1}",
1111+
RestApiVersion.current());
1112+
StructWithOnOrAfterField o2 = StructWithOnOrAfterField.PARSER.parse(futureParser, null);
1113+
assertEquals(1, o2.intField);
1114+
}
10771115
}

‎server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,13 +349,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws
349349
PARSER.declareField((p, v, c) -> destParser.parse(p, v.getDestination(), c), new ParseField("dest"), ObjectParser.ValueType.OBJECT);
350350

351351
PARSER.declareInt(ReindexRequest::setMaxDocsValidateIdentical,
352-
new ParseField("max_docs", "size").withRestApiVersions(RestApiVersion.V_7));
352+
new ParseField("max_docs", "size")
353+
.forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7)));
353354

354355
PARSER.declareInt(ReindexRequest::setMaxDocsValidateIdentical,
355-
new ParseField("max_docs").withRestApiVersions(RestApiVersion.V_8));
356+
new ParseField("max_docs")
357+
.forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.V_8)));
356358
// avoid silently accepting an ignored size.
357359
PARSER.declareInt((r,s) -> failOnSizeSpecified(),
358-
new ParseField("size").withRestApiVersions(RestApiVersion.V_8));
360+
new ParseField("size")
361+
.forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.V_8)));
359362

360363
PARSER.declareField((p, v, c) -> v.setScript(Script.parse(p)), new ParseField("script"),
361364
ObjectParser.ValueType.OBJECT);

0 commit comments

Comments
 (0)