1
0
Эх сурвалжийг харах

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.
Przemyslaw Gomulka 4 жил өмнө
parent
commit
8d09fbf82b

+ 14 - 0
libs/core/src/main/java/org/elasticsearch/common/RestApiVersion.java

@@ -8,6 +8,8 @@
 
 package org.elasticsearch.common;
 
+import java.util.function.Function;
+
 /**
  * A enum representing versions of the REST API (particularly with regard to backwards compatibility).
  *
@@ -30,6 +32,10 @@ public enum RestApiVersion {
         return fromMajorVersion(major - 1);
     }
 
+    public boolean matches(Function<RestApiVersion, Boolean> restApiVersionFunctions){
+        return restApiVersionFunctions.apply(this);
+    }
+
     private static RestApiVersion fromMajorVersion(int majorVersion) {
         return valueOf("V_" + majorVersion);
     }
@@ -42,4 +48,12 @@ public enum RestApiVersion {
         return CURRENT;
     }
 
+    public static Function<RestApiVersion, Boolean> equalTo(RestApiVersion restApiVersion) {
+        return r -> r.major == restApiVersion.major;
+    }
+
+    public static Function<RestApiVersion, Boolean> onOrAfter(RestApiVersion restApiVersion) {
+        return r -> r.major >= restApiVersion.major;
+    }
+
 }

+ 12 - 14
libs/x-content/src/main/java/org/elasticsearch/common/ParseField.java

@@ -10,13 +10,11 @@ package org.elasticsearch.common;
 import org.elasticsearch.common.xcontent.DeprecationHandler;
 import org.elasticsearch.common.xcontent.XContentLocation;
 
-import java.util.Arrays;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
-import java.util.List;
 import java.util.Objects;
 import java.util.Set;
+import java.util.function.Function;
 import java.util.function.Supplier;
 
 /**
@@ -26,7 +24,7 @@ import java.util.function.Supplier;
 public class ParseField {
     private final String name;
     private final String[] deprecatedNames;
-    private final Set<RestApiVersion> restApiVersions = new HashSet<>(2);
+    private final Function<RestApiVersion, Boolean> forRestApiVersion;
     private String allReplacedWith = null;
     private final String[] allNames;
     private boolean fullyDeprecated = false;
@@ -34,7 +32,7 @@ public class ParseField {
     private static final String[] EMPTY = new String[0];
 
 
-    private ParseField(String name, Collection<RestApiVersion> restApiVersions, String[] deprecatedNames) {
+    private ParseField(String name, Function<RestApiVersion, Boolean> forRestApiVersion, String[] deprecatedNames) {
         this.name = name;
         if (deprecatedNames == null || deprecatedNames.length == 0) {
             this.deprecatedNames = EMPTY;
@@ -43,7 +41,7 @@ public class ParseField {
             Collections.addAll(set, deprecatedNames);
             this.deprecatedNames = set.toArray(new String[set.size()]);
         }
-        this.restApiVersions.addAll(restApiVersions);
+        this.forRestApiVersion = forRestApiVersion;
 
         Set<String> allNames = new HashSet<>();
         allNames.add(name);
@@ -59,7 +57,7 @@ public class ParseField {
      *                        accepted when strict matching is used.
      */
     public ParseField(String name, String... deprecatedNames) {
-        this(name, List.of(RestApiVersion.current(), RestApiVersion.minimumSupported()) ,deprecatedNames);
+        this(name, RestApiVersion.onOrAfter(RestApiVersion.minimumSupported()) ,deprecatedNames);
     }
 
     /**
@@ -90,18 +88,18 @@ public class ParseField {
 
 
     /**
-     * Creates a new field with current name and deprecatedNames, but overrides restApiVersions
-     * @param restApiVersions rest api versions which specifies when a lookup will be allowed
+     * Creates a new field with current name and deprecatedNames, but overrides forRestApiVersion
+     * @param forRestApiVersion - a boolean function indicating if a field is for the given RestApiVersion
      */
-    public ParseField withRestApiVersions(RestApiVersion... restApiVersions) {
-        return new ParseField(this.name, Arrays.asList(restApiVersions), this.deprecatedNames);
+    public ParseField forRestApiVersion(Function<RestApiVersion, Boolean> forRestApiVersion) {
+        return new ParseField(this.name, forRestApiVersion, this.deprecatedNames);
     }
 
     /**
-     * @return rest api versions under which a lookup will be allowed
+     * @return a function indicating for which RestApiVersion a field is declared for
      */
-    public Set<RestApiVersion> getRestApiVersions() {
-        return restApiVersions;
+    public Function<RestApiVersion, Boolean> getForRestApiVersion() {
+        return forRestApiVersion;
     }
 
     /**

+ 7 - 3
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ConstructingObjectParser.java

@@ -336,10 +336,14 @@ public final class ConstructingObjectParser<Value, Context> extends AbstractObje
     private Map<RestApiVersion, Integer> addConstructorArg(BiConsumer<?, ?> consumer, ParseField parseField) {
 
         boolean required = consumer == REQUIRED_CONSTRUCTOR_ARG_MARKER;
-        for (RestApiVersion restApiVersion : parseField.getRestApiVersions()) {
 
-            constructorArgInfos.computeIfAbsent(restApiVersion, (v)-> new ArrayList<>())
-                    .add(new ConstructorArgInfo(parseField, required));
+        if (RestApiVersion.minimumSupported().matches(parseField.getForRestApiVersion())) {
+            constructorArgInfos.computeIfAbsent(RestApiVersion.minimumSupported(), (v)-> new ArrayList<>())
+                .add(new ConstructorArgInfo(parseField, required));
+        }
+        if (RestApiVersion.current().matches(parseField.getForRestApiVersion())) {
+            constructorArgInfos.computeIfAbsent(RestApiVersion.current(), (v)-> new ArrayList<>())
+                .add(new ConstructorArgInfo(parseField, required));
         }
 
         //calculate the positions for the arguments

+ 8 - 6
libs/x-content/src/main/java/org/elasticsearch/common/xcontent/ObjectParser.java

@@ -366,13 +366,15 @@ public final class ObjectParser<Value, Context> extends AbstractObjectParser<Val
         }
         FieldParser fieldParser = new FieldParser(p, type.supportedTokens(), parseField, type);
         for (String fieldValue : parseField.getAllNamesIncludedDeprecated()) {
-            if (parseField.getRestApiVersions().contains(RestApiVersion.minimumSupported())) {
-                fieldParserMap.putIfAbsent(RestApiVersion.minimumSupported(), new HashMap<>());
-                fieldParserMap.get(RestApiVersion.minimumSupported()).putIfAbsent(fieldValue, fieldParser);
+
+            if (RestApiVersion.minimumSupported().matches(parseField.getForRestApiVersion())) {
+                fieldParserMap.computeIfAbsent(RestApiVersion.minimumSupported(), (v)-> new HashMap<>())
+                    .putIfAbsent(fieldValue, fieldParser);
             }
-            if (parseField.getRestApiVersions().contains(RestApiVersion.current())) {
-                fieldParserMap.putIfAbsent(RestApiVersion.current(), new HashMap<>());
-                fieldParserMap.get(RestApiVersion.current()).putIfAbsent(fieldValue, fieldParser);
+            if (RestApiVersion.current().matches(parseField.getForRestApiVersion())) {
+                fieldParserMap.computeIfAbsent(RestApiVersion.current(), (v)-> new HashMap<>())
+                    .putIfAbsent(fieldValue, fieldParser);
+
             }
         }
 

+ 8 - 5
libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ConstructingObjectParserTests.java

@@ -573,15 +573,17 @@ public class ConstructingObjectParserTests extends ESTestCase {
             // The declaration is only available for lookup when parser has compatibility set
             PARSER.declareInt(constructorArg(),
                 new ParseField("new_name", "old_name")
-                    .withRestApiVersions(RestApiVersion.minimumSupported()));
+                    .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())));
 
             // declare `new_name` to be parsed when compatibility is NOT used
             PARSER.declareInt(constructorArg(),
-                new ParseField("new_name").withRestApiVersions(RestApiVersion.current()));
+                new ParseField("new_name")
+                    .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.current())));
 
             // declare `old_name` to throw exception when compatibility is NOT used
             PARSER.declareInt((r,s) -> failWithException(),
-                new ParseField("old_name").withRestApiVersions(RestApiVersion.current()));
+                new ParseField("old_name")
+                    .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.current())));
         }
         private int intField;
 
@@ -649,11 +651,12 @@ public class ConstructingObjectParserTests extends ESTestCase {
             // The deprecation shoudl be done manually
             PARSER.declareInt(logWarningDoNothing("old_name"),
                 new ParseField("old_name")
-                    .withRestApiVersions(RestApiVersion.minimumSupported()));
+                    .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())));
 
             // declare `old_name` to throw exception when compatibility is NOT used
             PARSER.declareInt((r,s) -> failWithException(),
-                new ParseField("old_name").withRestApiVersions(RestApiVersion.current()));
+                new ParseField("old_name")
+                    .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.current())));
         }
 
         private final String secondField;

+ 43 - 5
libs/x-content/src/test/java/org/elasticsearch/common/xcontent/ObjectParserTests.java

@@ -9,8 +9,8 @@ package org.elasticsearch.common.xcontent;
 
 import org.elasticsearch.common.CheckedFunction;
 import org.elasticsearch.common.ParseField;
-import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.RestApiVersion;
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.xcontent.ObjectParser.NamedObjectParser;
 import org.elasticsearch.common.xcontent.ObjectParser.ValueType;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
@@ -1018,15 +1018,17 @@ public class ObjectParserTests extends ESTestCase {
             // The declaration is only available for lookup when parser has compatibility set
             PARSER.declareInt(StructWithCompatibleFields::setIntField,
                 new ParseField("new_name", "old_name")
-                    .withRestApiVersions(RestApiVersion.minimumSupported()));
+                    .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.minimumSupported())));
 
             // declare `new_name` to be parsed when compatibility is NOT used
             PARSER.declareInt(StructWithCompatibleFields::setIntField,
-                new ParseField("new_name").withRestApiVersions(RestApiVersion.current()));
+                new ParseField("new_name")
+                    .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.current())));
 
             // declare `old_name` to throw exception when compatibility is NOT used
             PARSER.declareInt((r,s) -> failWithException(),
-                new ParseField("old_name").withRestApiVersions(RestApiVersion.current()));
+                new ParseField("old_name")
+                    .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.current())));
         }
 
         private static void failWithException() {
@@ -1063,7 +1065,6 @@ public class ObjectParserTests extends ESTestCase {
             assertEquals(1, o.intField);
         }
         {
-
             // old_name is allowed to be parsed with compatibility, but results in deprecation
             XContentParser parser = createParserWithCompatibilityFor(JsonXContent.jsonXContent, "{\"old_name\": 1}",
                 RestApiVersion.minimumSupported());
@@ -1074,4 +1075,41 @@ public class ObjectParserTests extends ESTestCase {
 
         }
     }
+    public static class StructWithOnOrAfterField {
+        // real usage would have exact version like RestApiVersion.V_7 (equal to current version) instead of minimumSupported
+
+        static final ObjectParser<StructWithOnOrAfterField, Void> PARSER =
+            new ObjectParser<>("struct_with_on_or_after_field", StructWithOnOrAfterField::new);
+        static {
+
+            // 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
+            PARSER.declareInt(StructWithOnOrAfterField::setIntField,
+                new ParseField("new_name")
+                    .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.minimumSupported())));
+
+        }
+
+        private int intField;
+
+        private  void setIntField(int intField) {
+            this.intField = intField;
+        }
+    }
+
+    public void testFieldsForVersionsOnOrAfter() throws IOException {
+        // this test needs to verify that a field declared in version N will be available in version N+1
+        // to do this, we assume a version N is minimum (so that the test passes for future releases) and the N+1 is current()
+
+        // new name is accessed in "current" version - lets assume the current is minimumSupported
+        XContentParser parser = createParserWithCompatibilityFor(JsonXContent.jsonXContent, "{\"new_name\": 1}",
+            RestApiVersion.minimumSupported());
+        StructWithOnOrAfterField o1 = StructWithOnOrAfterField.PARSER.parse(parser, null);
+        assertEquals(1, o1.intField);
+
+        // new name is accessed in "future" version - lets assume the future is currentVersion (minimumSupported+1)
+        XContentParser futureParser = createParserWithCompatibilityFor(JsonXContent.jsonXContent, "{\"new_name\": 1}",
+            RestApiVersion.current());
+        StructWithOnOrAfterField o2 = StructWithOnOrAfterField.PARSER.parse(futureParser, null);
+        assertEquals(1, o2.intField);
+    }
 }

+ 6 - 3
server/src/main/java/org/elasticsearch/index/reindex/ReindexRequest.java

@@ -349,13 +349,16 @@ public class ReindexRequest extends AbstractBulkIndexByScrollRequest<ReindexRequ
         PARSER.declareField((p, v, c) -> destParser.parse(p, v.getDestination(), c), new ParseField("dest"), ObjectParser.ValueType.OBJECT);
 
         PARSER.declareInt(ReindexRequest::setMaxDocsValidateIdentical,
-            new ParseField("max_docs", "size").withRestApiVersions(RestApiVersion.V_7));
+            new ParseField("max_docs", "size")
+                .forRestApiVersion(RestApiVersion.equalTo(RestApiVersion.V_7)));
 
         PARSER.declareInt(ReindexRequest::setMaxDocsValidateIdentical,
-            new ParseField("max_docs").withRestApiVersions(RestApiVersion.V_8));
+            new ParseField("max_docs")
+                .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.V_8)));
         // avoid silently accepting an ignored size.
         PARSER.declareInt((r,s) -> failOnSizeSpecified(),
-            new ParseField("size").withRestApiVersions(RestApiVersion.V_8));
+            new ParseField("size")
+                .forRestApiVersion(RestApiVersion.onOrAfter(RestApiVersion.V_8)));
 
         PARSER.declareField((p, v, c) -> v.setScript(Script.parse(p)), new ParseField("script"),
             ObjectParser.ValueType.OBJECT);