Skip to content

Commit

Permalink
Distinguish Negotiated vs Default Rest API Version (#98121)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tvernum authored Aug 10, 2023
1 parent 6a1bdb0 commit 6269744
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 14 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,19 @@
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.
* Package scope as it is only aimed to be used by RestRequest
*/
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
Expand Down Expand Up @@ -76,15 +81,19 @@ static RestApiVersion getCompatibleVersion(
);
}
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) {
Expand Down
17 changes: 12 additions & 5 deletions server/src/main/java/org/elasticsearch/rest/RestRequest.java
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
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;
Expand All @@ -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;
Expand Down Expand Up @@ -112,9 +113,11 @@ private RestRequest(
} 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;
Expand All @@ -123,7 +126,7 @@ private RestRequest(
}

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) {
Expand Down Expand Up @@ -610,7 +613,11 @@ public static XContentType parseContentType(List<String> header) {
* 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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,14 @@
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;
Expand Down Expand Up @@ -145,7 +150,7 @@ public void testAcceptAndContentTypeCombinations() {

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()));
Expand Down Expand Up @@ -322,12 +327,30 @@ public void testVersionParsing() {

}

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() {
Expand Down Expand Up @@ -361,7 +384,7 @@ private String mediaType(String version) {
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);
Expand Down

0 comments on commit 6269744

Please sign in to comment.