Fix URL endpoint for injected `NessieApi` from `OldNessieServer` #7958

XN137 · 2024-01-16T13:52:42Z

in #7854 CurrentNessieServer was adjusted to build NessieApi instances with an URL that includes app/.
However this adjustment was missing in OldNessieServer which caused the injected NessieApi instance to use a wrong endpoint and run into HTTP 404 errors since 0.75.0.

XN137 · 2024-01-16T14:17:29Z

...common/src/main/java/org/projectnessie/tools/compatibility/internal/CurrentNessieServer.java

@@ -61,7 +61,7 @@ static CurrentNessieServer currentNessieServer(

  @Override
  public URI getUri(Class<? extends NessieApi> apiType) {
-    return Util.resolveNessieUri(jersey.getUri().resolve("api/"), apiType);
+    return Util.resolveNessieUri(jersey.getUri(), serverKey.getVersion(), apiType);


#7854 added api/ here but nothing version-aware in OldNessieServer

any ideas how to stay "ahead" of such problems in the future?
i.e. do we need server/client compatibility tests that use a Version.LAST_RELEASE value? then at least after a new release we might get notified.

or a test that uses Version.CURRENT with OldNessieServer somehow?

OldNessieServer relies on published jars, so it is currently at least one release behind (it cannot test main due to lack of artifacts). One option may be to do publishToMavenLocal and use those artifacts in CI.

Add the version to nessie-compatibility.properties?

If we go with publishToMavenLocal I believe some extra changes will be required to support "snapshot" versions.

Add the version to nessie-compatibility.properties?

doesnt this require awareness of the developer that made the change that compatibility was broken?
CI tests should find this for the developer "automatically"

What I'm proposing is to run publishToMavenLocal in CI and use that version for tests automatically, indeed... but let's do that in another PR (I believe it will have some ripple effect in the code that handles versions).

snazy · 2024-01-16T14:20:49Z

Please change the PR title / commit-title to what the change does where.

...t/java/org/projectnessie/tools/compatibility/internal/TestNessieCompatibilityExtensions.java

dimas-b

LGTM 👍 Thanks for the fix @XN137 !

compatibility/common/src/main/java/org/projectnessie/tools/compatibility/internal/Util.java

XN137 · 2024-01-16T14:33:03Z

...t/java/org/projectnessie/tools/compatibility/internal/TestNessieCompatibilityExtensions.java

+
+      // use api
+      List<Reference> references = api.getAllReferences().get().getReferences();
+      soft.assertThat(references).isNotEmpty();


note that without assertNoFailedTestEvents the outer assertion for allVersions would pass just fine, even though the API usage causes an error (since it comes last)

snazy · 2024-01-16T14:39:55Z

Please change the PR title / commit-title to what the change does where.

Sorry, but can you be a bit more specific in the PR description / commit message - what's exactly broken and what's changed.

dimas-b · 2024-01-16T14:45:04Z

[...] PR description / commit message

Suggestion: Support /api URL suffix in OldNessieServer

XN137 · 2024-01-16T14:52:43Z

...t/java/org/projectnessie/tools/compatibility/internal/TestNessieCompatibilityExtensions.java

@@ -131,15 +155,34 @@ void olderServers() {
    soft.assertThat(OldServersSample.uris).allMatch(uri -> uri.getPath().equals("/"));
  }

+  @Test
+  void injectedNessieApiUrlChanged() {


test fails like this:

Multiple Failures (1 failure) -- failure 1 --The following test events have failed: :Event [type = FINISHED, testDescriptor = TestMethodTestDescriptor: [engine:nessie-multi-env]/[nessie-version:0.75.0]/[class:org.projectnessie.tools.compatibility.internal.TestNessieCompatibilityExtensions$ApiEndpointServerSample]/[method:testSome()], timestamp = 2024-01-16T13:46:43.453995109Z, payload = TestExecutionResult [status = FAILED, throwable = org.projectnessie.client.rest.NessieServiceException: Not Found (HTTP/404): Additionally, the client-side exception below was caught while decoding the HTTP response: com.fasterxml.jackson.databind.exc.MismatchedInputException: Cannot deserialize value of type `org.projectnessie.error.ImmutableNessieError` from [Unavailable value] (token `JsonToken.NOT_AVAILABLE`) at [Source: UNKNOWN; byte offset: #UNKNOWN] at com.fasterxml.jackson.databind.exc.MismatchedInputException.from(MismatchedInputException.java:59) at com.fasterxml.jackson.databind.DeserializationContext.reportInputMismatch(DeserializationContext.java:1767) at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1541) at com.fasterxml.jackson.databind.DeserializationContext.handleUnexpectedToken(DeserializationContext.java:1488) at com.fasterxml.jackson.databind.deser.BeanDeserializer._deserializeOther(BeanDeserializer.java:224) at com.fasterxml.jackson.databind.deser.BeanDeserializer.deserialize(BeanDeserializer.java:187) at com.fasterxml.jackson.databind.deser.DefaultDeserializationContext.readRootValue(DefaultDeserializationContext.java:342) at com.fasterxml.jackson.databind.ObjectReader._bind(ObjectReader.java:2099) at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1249) at com.fasterxml.jackson.databind.ObjectReader.readValue(ObjectReader.java:1267) at com.fasterxml.jackson.databind.ObjectReader.treeToValue(ObjectReader.java:2044) at org.projectnessie.client.rest.ResponseCheckFilter.decodeErrorObject(ResponseCheckFilter.java:111) at org.projectnessie.client.rest.ResponseCheckFilter.checkResponse(ResponseCheckFilter.java:55) at org.projectnessie.client.rest.NessieHttpResponseFilter.filter(NessieHttpResponseFilter.java:29) at org.projectnessie.client.http.impl.jdk11.JavaRequest.lambda$executeRequest$1(JavaRequest.java:143) at java.base/java.util.ArrayList.forEach(ArrayList.java:1511) at java.base/java.util.Collections$UnmodifiableCollection.forEach(Collections.java:1092) at org.projectnessie.client.http.impl.jdk11.JavaRequest.executeRequest(JavaRequest.java:143) at org.projectnessie.client.http.HttpRequest.get(HttpRequest.java:106) at org.projectnessie.client.rest.v1.RestV1TreeClient.getAllReferences(RestV1TreeClient.java:58) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77) at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) at java.base/java.lang.reflect.Method.invoke(Method.java:568) at org.projectnessie.client.rest.v1.RestV1Client$ExceptionRewriter.invoke(RestV1Client.java:84) at jdk.proxy3/jdk.proxy3.$Proxy293.getAllReferences(Unknown Source) at org.projectnessie.client.rest.v1.HttpGetAllReferences.get(HttpGetAllReferences.java:42) at org.projectnessie.client.rest.v1.HttpGetAllReferences.get(HttpGetAllReferences.java:22) at org.projectnessie.client.builder.BaseGetAllReferencesBuilder.get(BaseGetAllReferencesBuilder.java:70) at org.projectnessie.tools.compatibility.internal.TestNessieCompatibilityExtensions$ApiEndpointServerSample.testSome

XN137 · 2024-01-16T14:53:14Z

Sorry, but can you be a bit more specific in the PR description / commit message - what's exactly broken and what's changed.

updated, please make a concrete suggestion if you are still not happy.

XN137 added 2 commits January 16, 2024 14:49

Add failing test injectedNessieApiUrlChanged

a6fe710

fix test by making resolveNessieUri version aware

eee45f1

XN137 commented Jan 16, 2024

View reviewed changes

...t/java/org/projectnessie/tools/compatibility/internal/TestNessieCompatibilityExtensions.java Show resolved Hide resolved

adjustment comments

ed94a2c

dimas-b reviewed Jan 16, 2024

View reviewed changes

compatibility/common/src/main/java/org/projectnessie/tools/compatibility/internal/Util.java Outdated Show resolved Hide resolved

rename new version constant

ba05a2a

XN137 changed the title ~~NessieApi injection is broken with OldNessieServer from 0.75.0~~ Fix broken NessieApi injection with OldNessieServer since 0.75.0 Jan 16, 2024

XN137 marked this pull request as ready for review January 16, 2024 14:31

XN137 commented Jan 16, 2024

View reviewed changes

dimas-b previously approved these changes Jan 16, 2024

View reviewed changes

use assertNoFailedTestEvents where applicable

4f67113

XN137 dismissed dimas-b’s stale review via 4f67113 January 16, 2024 14:45

XN137 changed the title ~~Fix broken NessieApi injection with OldNessieServer since 0.75.0~~ Fix URL endpoint for injected NessieApi from OldNessieServer Jan 16, 2024

XN137 commented Jan 16, 2024

View reviewed changes

snazy approved these changes Jan 16, 2024

View reviewed changes

dimas-b approved these changes Jan 16, 2024

View reviewed changes

dimas-b merged commit c4366af into projectnessie:main Jan 16, 2024
17 checks passed

XN137 deleted the injected-nessieapi-problem branch January 16, 2024 18:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix URL endpoint for injected `NessieApi` from `OldNessieServer` #7958

Fix URL endpoint for injected `NessieApi` from `OldNessieServer` #7958

XN137 commented Jan 16, 2024 •

edited

XN137 Jan 16, 2024

dimas-b Jan 16, 2024

snazy Jan 16, 2024

dimas-b Jan 16, 2024 •

edited

XN137 Jan 16, 2024

dimas-b Jan 16, 2024 •

edited

snazy commented Jan 16, 2024

dimas-b left a comment

XN137 Jan 16, 2024 •

edited

snazy commented Jan 16, 2024

dimas-b commented Jan 16, 2024

XN137 Jan 16, 2024

XN137 commented Jan 16, 2024

Fix URL endpoint for injected NessieApi from OldNessieServer #7958

Fix URL endpoint for injected NessieApi from OldNessieServer #7958

Conversation

XN137 commented Jan 16, 2024 • edited

XN137 Jan 16, 2024

Choose a reason for hiding this comment

dimas-b Jan 16, 2024

Choose a reason for hiding this comment

snazy Jan 16, 2024

Choose a reason for hiding this comment

dimas-b Jan 16, 2024 • edited

Choose a reason for hiding this comment

XN137 Jan 16, 2024

Choose a reason for hiding this comment

dimas-b Jan 16, 2024 • edited

Choose a reason for hiding this comment

snazy commented Jan 16, 2024

dimas-b left a comment

Choose a reason for hiding this comment

XN137 Jan 16, 2024 • edited

Choose a reason for hiding this comment

snazy commented Jan 16, 2024

dimas-b commented Jan 16, 2024

XN137 Jan 16, 2024

Choose a reason for hiding this comment

XN137 commented Jan 16, 2024

Fix URL endpoint for injected `NessieApi` from `OldNessieServer` #7958

Fix URL endpoint for injected `NessieApi` from `OldNessieServer` #7958

XN137 commented Jan 16, 2024 •

edited

dimas-b Jan 16, 2024 •

edited

dimas-b Jan 16, 2024 •

edited

XN137 Jan 16, 2024 •

edited