Browse Source

Distinguish Negotiated vs Default Rest API Version (#98121)

This commit changes the RestApiVersion parsing code to return an
Optional<RestApiVersion> so that the RestRequest can distinguish between
reqeust that included an explicit API version, and requests that should
use the server's default version.

This is needed because "compatible-with=8" will have a specific meaning
on serverless that is not the same as the default API behaviour.
Tim Vernum 2 years ago
parent
commit
6269744e35

+ 13 - 4
server/src/main/java/org/elasticsearch/rest/RestCompatibleVersionHelper.java

@@ -13,6 +13,8 @@ import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.xcontent.MediaType;
 import org.elasticsearch.xcontent.ParsedMediaType;
 
+import java.util.Optional;
+
 /**
  * A helper that is responsible for parsing a Compatible REST API version from RestRequest.
  * It also performs a validation of allowed combination of versions provided on those headers.
@@ -20,7 +22,10 @@ import org.elasticsearch.xcontent.ParsedMediaType;
  */
 class RestCompatibleVersionHelper {
 
-    static RestApiVersion getCompatibleVersion(
+    /**
+     * @return The requested API version, or {@link Optional#empty()} if there was no explicit version in the request.
+     */
+    static Optional<RestApiVersion> getCompatibleVersion(
         @Nullable ParsedMediaType acceptHeader,
         @Nullable ParsedMediaType contentTypeHeader,
         boolean hasContent
@@ -76,15 +81,19 @@ class RestCompatibleVersionHelper {
                 );
             }
             if (contentTypeVersion < RestApiVersion.current().major) {
-                return RestApiVersion.minimumSupported();
+                return Optional.of(RestApiVersion.minimumSupported());
             }
         }
 
         if (acceptVersion < RestApiVersion.current().major) {
-            return RestApiVersion.minimumSupported();
+            return Optional.of(RestApiVersion.minimumSupported());
         }
 
-        return RestApiVersion.current();
+        if (cVersion == null && aVersion == null) {
+            return Optional.empty();
+        } else {
+            return Optional.of(RestApiVersion.current());
+        }
     }
 
     static Byte parseVersion(ParsedMediaType parsedMediaType) {

+ 12 - 5
server/src/main/java/org/elasticsearch/rest/RestRequest.java

@@ -38,6 +38,7 @@ import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.regex.Pattern;
@@ -62,7 +63,7 @@ public class RestRequest implements ToXContent.Params {
     private final HttpChannel httpChannel;
     private final ParsedMediaType parsedAccept;
     private final ParsedMediaType parsedContentType;
-    private final RestApiVersion restApiVersion;
+    private final Optional<RestApiVersion> restApiVersion;
     private HttpRequest httpRequest;
 
     private boolean contentConsumed = false;
@@ -112,9 +113,11 @@ public class RestRequest implements ToXContent.Params {
         } catch (ElasticsearchStatusException e) {
             throw new MediaTypeHeaderException(e, "Accept", "Content-Type");
         }
-        this.parserConfig = parserConfig.restApiVersion().equals(restApiVersion)
+
+        var effectiveApiVersion = this.getRestApiVersion();
+        this.parserConfig = parserConfig.restApiVersion().equals(effectiveApiVersion)
             ? parserConfig
-            : parserConfig.withRestApiVersion(restApiVersion);
+            : parserConfig.withRestApiVersion(effectiveApiVersion);
         this.httpChannel = httpChannel;
         this.params = params;
         this.rawPath = path;
@@ -123,7 +126,7 @@ public class RestRequest implements ToXContent.Params {
     }
 
     protected RestRequest(RestRequest other) {
-        assert other.parserConfig.restApiVersion().equals(other.restApiVersion);
+        assert other.parserConfig.restApiVersion().equals(other.getRestApiVersion());
         this.parsedAccept = other.parsedAccept;
         this.parsedContentType = other.parsedContentType;
         if (other.xContentType.get() != null) {
@@ -610,7 +613,11 @@ public class RestRequest implements ToXContent.Params {
      * The requested version of the REST API.
      */
     public RestApiVersion getRestApiVersion() {
-        return restApiVersion;
+        return restApiVersion.orElse(RestApiVersion.current());
+    }
+
+    public boolean hasExplicitRestApiVersion() {
+        return restApiVersion.isPresent();
     }
 
     public void markResponseRestricted(String restriction) {

+ 28 - 5
server/src/test/java/org/elasticsearch/rest/RestCompatibleVersionHelperTests.java

@@ -11,9 +11,14 @@ import org.elasticsearch.ElasticsearchStatusException;
 import org.elasticsearch.core.RestApiVersion;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xcontent.ParsedMediaType;
+import org.hamcrest.CustomTypeSafeMatcher;
+import org.hamcrest.Description;
 import org.hamcrest.Matcher;
 
+import java.util.Optional;
+
 import static org.elasticsearch.test.LambdaMatchers.transformedMatch;
+import static org.hamcrest.CoreMatchers.both;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.not;
@@ -145,7 +150,7 @@ public class RestCompatibleVersionHelperTests extends ESTestCase {
 
         assertThat(requestWith(acceptHeader(null), contentTypeHeader(CURRENT_VERSION), bodyNotPresent()), not(isCompatible()));
 
-        assertThat(requestWith(acceptHeader(null), contentTypeHeader(null), bodyNotPresent()), not(isCompatible()));
+        assertThat(requestWith(acceptHeader(null), contentTypeHeader(null), bodyNotPresent()), not(hasVersion()));
 
         // Accept header = application/json means current version. If body is provided then accept and content-Type should be the same
         assertThat(requestWith(acceptHeader("application/json"), contentTypeHeader(null), bodyNotPresent()), not(isCompatible()));
@@ -322,12 +327,30 @@ public class RestCompatibleVersionHelperTests extends ESTestCase {
 
     }
 
-    private Matcher<RestApiVersion> isCompatible() {
+    private Matcher<Optional<RestApiVersion>> isCompatible() {
         return requestHasVersion(PREVIOUS_VERSION);
     }
 
-    private Matcher<RestApiVersion> requestHasVersion(int version) {
-        return transformedMatch(v -> (int) v.major, equalTo(version));
+    private Matcher<Optional<RestApiVersion>> hasVersion() {
+        return new CustomTypeSafeMatcher<>("has-version") {
+            @Override
+            protected boolean matchesSafely(Optional<RestApiVersion> item) {
+                return item.isPresent();
+            }
+
+            @Override
+            protected void describeMismatchSafely(Optional<RestApiVersion> item, Description mismatchDescription) {
+                if (item.isEmpty()) {
+                    mismatchDescription.appendText("optional is empty");
+                } else {
+                    mismatchDescription.appendText("optional is present");
+                }
+            }
+        };
+    }
+
+    private Matcher<Optional<RestApiVersion>> requestHasVersion(int version) {
+        return both(hasVersion()).and(transformedMatch(opt -> (int) opt.get().major, equalTo(version)));
     }
 
     private String bodyNotPresent() {
@@ -361,7 +384,7 @@ public class RestCompatibleVersionHelperTests extends ESTestCase {
         return null;
     }
 
-    private RestApiVersion requestWith(String accept, String contentType, String body) {
+    private Optional<RestApiVersion> requestWith(String accept, String contentType, String body) {
         ParsedMediaType parsedAccept = ParsedMediaType.parseMediaType(accept);
         ParsedMediaType parsedContentType = ParsedMediaType.parseMediaType(contentType);
         return RestCompatibleVersionHelper.getCompatibleVersion(parsedAccept, parsedContentType, body.isEmpty() == false);