Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add an API compatibility check to Nessie clients #6818

Merged
merged 18 commits into from
May 17, 2023
2 changes: 2 additions & 0 deletions api/client/build.gradle.kts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ dependencies {
testFixturesApi(libs.undertow.servlet)
testFixturesImplementation(libs.logback.classic)

testImplementation(libs.wiremock)

intTestImplementation(libs.testcontainers.testcontainers)
intTestImplementation(libs.testcontainers.junit)
intTestImplementation(libs.testcontainers.keycloak) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,12 @@ public final class NessieConfigConstants {
public static final String CONF_FORCE_URL_CONNECTION_CLIENT =
"nessie.force-url-connection-client";

/**
* Enables API compatibility check when creating the Nessie client. The default is {@code true}.
*/
public static final String CONF_ENABLE_API_COMPATIBILITY_CHECK =
"nessie.enable-api-compatibility-check";

public static final int DEFAULT_READ_TIMEOUT_MILLIS = 25000;
public static final int DEFAULT_CONNECT_TIMEOUT_MILLIS = 5000;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
package org.projectnessie.client.http;

import static org.projectnessie.client.NessieConfigConstants.CONF_CONNECT_TIMEOUT;
import static org.projectnessie.client.NessieConfigConstants.CONF_ENABLE_API_COMPATIBILITY_CHECK;
import static org.projectnessie.client.NessieConfigConstants.CONF_FORCE_URL_CONNECTION_CLIENT;
import static org.projectnessie.client.NessieConfigConstants.CONF_NESSIE_DISABLE_COMPRESSION;
import static org.projectnessie.client.NessieConfigConstants.CONF_NESSIE_HTTP_2;
Expand Down Expand Up @@ -67,6 +68,8 @@ public class HttpClientBuilder implements NessieClientBuilder<HttpClientBuilder>

private boolean tracing;

private boolean enableApiCompatibilityCheck = true;

protected HttpClientBuilder() {}

public static HttpClientBuilder builder() {
Expand Down Expand Up @@ -174,6 +177,11 @@ public HttpClientBuilder fromConfig(Function<String, String> configuration) {
withForceUrlConnectionClient(Boolean.parseBoolean(s.trim()));
}

s = configuration.apply(CONF_ENABLE_API_COMPATIBILITY_CHECK);
if (s != null) {
withEnableApiCompatibilityCheck(Boolean.parseBoolean(s));
}

return this;
}

Expand Down Expand Up @@ -304,6 +312,12 @@ public HttpClientBuilder withForceUrlConnectionClient(boolean forceUrlConnection
return this;
}

@CanIgnoreReturnValue
public HttpClientBuilder withEnableApiCompatibilityCheck(boolean enable) {
enableApiCompatibilityCheck = enable;
return this;
}

@CanIgnoreReturnValue
public HttpClientBuilder withResponseFactory(HttpResponseFactory responseFactory) {
builder.setResponseFactory(responseFactory);
Expand All @@ -323,12 +337,18 @@ public <API extends NessieApi> API build(Class<API> apiVersion) {
if (apiVersion.isAssignableFrom(HttpApiV1.class)) {
builder.setJsonView(Views.V1.class);
HttpClient httpClient = builder.build();
if (enableApiCompatibilityCheck) {
NessieApiCompatibility.check(1, httpClient);
}
return (API) new HttpApiV1(new NessieHttpClient(httpClient));
}

if (apiVersion.isAssignableFrom(HttpApiV2.class)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you move this if up (minor nit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually this would break things: HttpApiV2 is a subclass of NessieApiV1.

builder.setJsonView(Views.V2.class);
HttpClient httpClient = builder.build();
if (enableApiCompatibilityCheck) {
NessieApiCompatibility.check(2, httpClient);
}
return (API) new HttpApiV2(httpClient);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/*
* Copyright (C) 2023 Dremio
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.projectnessie.client.http;

import com.fasterxml.jackson.databind.JsonNode;

public class NessieApiCompatibility {

private static final String MIN_API_VERSION = "minSupportedApiVersion";
private static final String MAX_API_VERSION = "maxSupportedApiVersion";
private static final String ACTUAL_API_VERSION = "actualApiVersion";

/**
* Checks if the API version of the client is compatible with the server's.
*
* @param clientApiVersion the API version of the client
* @param httpClient the underlying HTTP client.
* @throws NessieApiCompatibilityException if the API version is not compatible.
*/
public static void check(int clientApiVersion, HttpClient httpClient)
throws NessieApiCompatibilityException {
JsonNode config = httpClient.newRequest().path("config").get().readEntity(JsonNode.class);
int minServerApiVersion =
config.hasNonNull(MIN_API_VERSION) ? config.get(MIN_API_VERSION).asInt() : 1;
int maxServerApiVersion = config.get(MAX_API_VERSION).asInt();
int actualServerApiVersion =
config.hasNonNull(ACTUAL_API_VERSION) ? config.get(ACTUAL_API_VERSION).asInt() : 0;
if (clientApiVersion < minServerApiVersion
|| clientApiVersion > maxServerApiVersion
|| (actualServerApiVersion > 0 && clientApiVersion != actualServerApiVersion)) {
throw new NessieApiCompatibilityException(
clientApiVersion, minServerApiVersion, maxServerApiVersion, actualServerApiVersion);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/*
* Copyright (C) 2023 Dremio
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.projectnessie.client.http;

public class NessieApiCompatibilityException extends RuntimeException {

private final int clientApiVersion;
private final int minServerApiVersion;
private final int maxServerApiVersion;
private final int actualServerApiVersion;

public NessieApiCompatibilityException(
int clientApiVersion,
int minServerApiVersion,
int maxServerApiVersion,
int actualServerApiVersion) {
super(
formatMessage(
clientApiVersion, minServerApiVersion, maxServerApiVersion, actualServerApiVersion));
this.clientApiVersion = clientApiVersion;
this.minServerApiVersion = minServerApiVersion;
this.maxServerApiVersion = maxServerApiVersion;
this.actualServerApiVersion = actualServerApiVersion;
}

private static String formatMessage(
int clientApiVersion,
int minServerApiVersion,
int maxServerApiVersion,
int actualServerApiVersion) {
if (clientApiVersion < minServerApiVersion) {
return String.format(
"API version %d is too old for server (minimum supported version is %d)",
clientApiVersion, minServerApiVersion);
}
if (clientApiVersion > maxServerApiVersion) {
return String.format(
"API version %d is too new for server (maximum supported version is %d)",
clientApiVersion, maxServerApiVersion);
}
return String.format(
"API version mismatch, check URI prefix (expected: %d, actual: %d)",
clientApiVersion, actualServerApiVersion);
}

/** The client's API version. */
public int getClientApiVersion() {
return clientApiVersion;
}

/** The minimum API version supported by the server. */
public int getMinServerApiVersion() {
return minServerApiVersion;
}

/** The maximum API version supported by the server. */
public int getMaxServerApiVersion() {
return maxServerApiVersion;
}

/**
* The actual API version used by the server, or zero if the server does not report its actual API
* version.
*/
public int getActualServerApiVersion() {
return actualServerApiVersion;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
/*
* Copyright (C) 2023 Dremio
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.projectnessie.client.http;

import static com.github.tomakehurst.wiremock.client.WireMock.get;
import static com.github.tomakehurst.wiremock.client.WireMock.stubFor;
import static java.net.HttpURLConnection.HTTP_OK;
import static org.assertj.core.api.Assertions.assertThatCode;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import static org.assertj.core.api.InstanceOfAssertFactories.type;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.node.JsonNodeFactory;
import com.fasterxml.jackson.databind.node.ObjectNode;
import com.github.tomakehurst.wiremock.client.ResponseDefinitionBuilder;
import com.github.tomakehurst.wiremock.junit5.WireMockRuntimeInfo;
import com.github.tomakehurst.wiremock.junit5.WireMockTest;
import java.net.URI;
import org.junit.jupiter.api.extension.ExtendWith;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.CsvSource;
import org.mockito.junit.jupiter.MockitoExtension;
import org.projectnessie.client.rest.NessieHttpResponseFilter;

@ExtendWith(MockitoExtension.class)
@WireMockTest
class TestNessieApiCompatibility {

enum Expectation {
OK,
TOO_OLD,
TOO_NEW,
MISMATCH
}

@ParameterizedTest
@CsvSource(
value = {
"1, 1, 1, 0, OK",
"1, 1, 2, 1, OK",
"1, 1, 2, 2, MISMATCH", // v2 endpoint mistakenly called with v1 client
"1, 2, 2, 2, TOO_OLD",
"2, 1, 1, 1, TOO_NEW",
"2, 1, 2, 1, MISMATCH", // v1 endpoint mistakenly called with v2 client
"2, 1, 2, 2, OK",
"2, 2, 2, 2, OK",
})
void checkApiCompatibility(
int client,
int serverMin,
int serverMax,
int serverActual,
Expectation expectation,
WireMockRuntimeInfo wireMock) {

ObjectNode config = JsonNodeFactory.instance.objectNode();
config.set("minSupportedApiVersion", JsonNodeFactory.instance.numberNode(serverMin));
config.set("maxSupportedApiVersion", JsonNodeFactory.instance.numberNode(serverMax));
if (serverActual > 0) {
config.set("actualApiVersion", JsonNodeFactory.instance.numberNode(serverActual));
}

stubFor(
get("/config")
.willReturn(
ResponseDefinitionBuilder.responseDefinition()
.withStatus(HTTP_OK)
.withBody(config.toString())
.withHeader("Content-Type", "application/json")));

try (HttpClient httpClient =
HttpClient.builder()
.setBaseUri(URI.create(wireMock.getHttpBaseUrl()))
.setObjectMapper(new ObjectMapper())
.addResponseFilter(new NessieHttpResponseFilter())
.build()) {

if (expectation == Expectation.OK) {

assertThatCode(() -> NessieApiCompatibility.check(client, httpClient))
.doesNotThrowAnyException();

} else {

assertThatThrownBy(() -> NessieApiCompatibility.check(client, httpClient))
.hasMessageContaining(
expectation == Expectation.MISMATCH
? "mismatch"
: expectation == Expectation.TOO_OLD ? "too old" : "too new")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: put too old, etc. into enum object fields in Expectation and use smth like expectation.getExpectedText()

.asInstanceOf(type(NessieApiCompatibilityException.class))
.extracting(
NessieApiCompatibilityException::getClientApiVersion,
NessieApiCompatibilityException::getMinServerApiVersion,
NessieApiCompatibilityException::getMaxServerApiVersion,
NessieApiCompatibilityException::getActualServerApiVersion)
.containsExactly(client, serverMin, serverMax, serverActual);
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
/*
* Copyright (C) 2023 Dremio
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.projectnessie.client.http;

import static org.assertj.core.api.Assertions.assertThat;

import org.junit.jupiter.api.Test;

class TestNessieApiCompatibilityException {

@Test
void testMessages() {
NessieApiCompatibilityException e = new NessieApiCompatibilityException(1, 2, 3, 3);
assertThat(e.getMessage())
.isEqualTo("API version 1 is too old for server (minimum supported version is 2)");
e = new NessieApiCompatibilityException(5, 3, 4, 4);
assertThat(e.getMessage())
.isEqualTo("API version 5 is too new for server (maximum supported version is 4)");
e = new NessieApiCompatibilityException(3, 2, 4, 2);
assertThat(e.getMessage())
.isEqualTo("API version mismatch, check URI prefix (expected: 3, actual: 2)");
}

@Test
void testGetters() {
NessieApiCompatibilityException e = new NessieApiCompatibilityException(1, 2, 4, 3);
assertThat(e.getClientApiVersion()).isEqualTo(1);
assertThat(e.getMinServerApiVersion()).isEqualTo(2);
assertThat(e.getMaxServerApiVersion()).isEqualTo(4);
assertThat(e.getActualServerApiVersion()).isEqualTo(3);
}
}
Loading