diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java index 1cde2b8ad43c..32fb4008040b 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java @@ -135,17 +135,17 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog .addAll(TOKEN_PREFERENCE_ORDER) .build(); + // these default endpoints must not be updated in order to maintain backwards compatibility with + // legacy servers private static final Set DEFAULT_ENDPOINTS = ImmutableSet.builder() .add(Endpoint.V1_LIST_NAMESPACES) .add(Endpoint.V1_LOAD_NAMESPACE) - .add(Endpoint.V1_NAMESPACE_EXISTS) .add(Endpoint.V1_CREATE_NAMESPACE) .add(Endpoint.V1_UPDATE_NAMESPACE) .add(Endpoint.V1_DELETE_NAMESPACE) .add(Endpoint.V1_LIST_TABLES) .add(Endpoint.V1_LOAD_TABLE) - .add(Endpoint.V1_TABLE_EXISTS) .add(Endpoint.V1_CREATE_TABLE) .add(Endpoint.V1_UPDATE_TABLE) .add(Endpoint.V1_DELETE_TABLE) @@ -155,11 +155,12 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog .add(Endpoint.V1_COMMIT_TRANSACTION) .build(); + // these view endpoints must not be updated in order to maintain backwards compatibility with + // legacy servers private static final Set VIEW_ENDPOINTS = ImmutableSet.builder() .add(Endpoint.V1_LIST_VIEWS) .add(Endpoint.V1_LOAD_VIEW) - .add(Endpoint.V1_VIEW_EXISTS) .add(Endpoint.V1_CREATE_VIEW) .add(Endpoint.V1_UPDATE_VIEW) .add(Endpoint.V1_DELETE_VIEW) diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java index 696240bb6da2..0455e2cf0d41 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTCatalog.java @@ -2508,6 +2508,15 @@ public void testNamespaceExistsViaHEADRequest() { @Test public void testNamespaceExistsFallbackToGETRequest() { + // server indicates support of loading a namespace only via GET, which is + // what older REST servers would send back too + verifyNamespaceExistsFallbackToGETRequest( + ConfigResponse.builder() + .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_NAMESPACE)) + .build()); + } + + private void verifyNamespaceExistsFallbackToGETRequest(ConfigResponse configResponse) { RESTCatalogAdapter adapter = Mockito.spy( new RESTCatalogAdapter(backendCatalog) { @@ -2518,13 +2527,7 @@ public T execute( Consumer errorHandler, Consumer> responseHeaders) { if ("v1/config".equals(request.path())) { - return castResponse( - responseType, - ConfigResponse.builder() - // server indicates support of loading a namespace only via GET, which is - // what older REST servers would send back too - .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_NAMESPACE)) - .build()); + return castResponse(responseType, configResponse); } return super.execute(request, responseType, errorHandler, responseHeaders); @@ -2553,6 +2556,13 @@ public T execute( any()); } + @Test + public void testNamespaceExistsFallbackToGETRequestWithLegacyServer() { + // simulate a legacy server that doesn't send back supported endpoints, thus the + // client relies on the default endpoints + verifyNamespaceExistsFallbackToGETRequest(ConfigResponse.builder().build()); + } + @Test public void testTableExistsViaHEADRequest() { RESTCatalogAdapter adapter = Mockito.spy(new RESTCatalogAdapter(backendCatalog)); @@ -2578,6 +2588,13 @@ public void testTableExistsViaHEADRequest() { @Test public void testTableExistsFallbackToGETRequest() { + // server indicates support of loading a table only via GET, which is + // what older REST servers would send back too + verifyTableExistsFallbackToGETRequest( + ConfigResponse.builder().withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_TABLE)).build()); + } + + private void verifyTableExistsFallbackToGETRequest(ConfigResponse configResponse) { RESTCatalogAdapter adapter = Mockito.spy( new RESTCatalogAdapter(backendCatalog) { @@ -2588,13 +2605,7 @@ public T execute( Consumer errorHandler, Consumer> responseHeaders) { if ("v1/config".equals(request.path())) { - return castResponse( - responseType, - ConfigResponse.builder() - // server indicates support of loading a table only via GET, which is - // what older REST servers would send back too - .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_TABLE)) - .build()); + return castResponse(responseType, configResponse); } return super.execute(request, responseType, errorHandler, responseHeaders); @@ -2627,6 +2638,13 @@ public T execute( any()); } + @Test + public void testTableExistsFallbackToGETRequestWithLegacyServer() { + // simulate a legacy server that doesn't send back supported endpoints, thus the + // client relies on the default endpoints + verifyTableExistsFallbackToGETRequest(ConfigResponse.builder().build()); + } + private RESTCatalog catalog(RESTCatalogAdapter adapter) { RESTCatalog catalog = new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter); diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java index 73ff5f052a11..0214e8762a0f 100644 --- a/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java +++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java @@ -246,6 +246,15 @@ public void viewExistsViaHEADRequest() { @Test public void viewExistsFallbackToGETRequest() { + // server indicates support of loading a view only via GET, which is + // what older REST servers would send back too + verifyViewExistsFallbackToGETRequest( + ConfigResponse.builder().withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_VIEW)).build(), + ImmutableMap.of()); + } + + private void verifyViewExistsFallbackToGETRequest( + ConfigResponse configResponse, Map catalogProperties) { RESTCatalogAdapter adapter = Mockito.spy( new RESTCatalogAdapter(backendCatalog) { @@ -256,13 +265,7 @@ public T execute( Consumer errorHandler, Consumer> responseHeaders) { if ("v1/config".equals(request.path())) { - return castResponse( - responseType, - ConfigResponse.builder() - // server indicates support of loading a view only via GET, which is - // what older REST servers would send back too - .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_VIEW)) - .build()); + return castResponse(responseType, configResponse); } return super.execute(request, responseType, errorHandler, responseHeaders); @@ -271,8 +274,7 @@ public T execute( RESTCatalog catalog = new RESTCatalog(SessionCatalog.SessionContext.createEmpty(), (config) -> adapter); - catalog.initialize("test", ImmutableMap.of()); - + catalog.initialize("test", catalogProperties); assertThat(catalog.viewExists(TableIdentifier.of("ns", "view"))).isFalse(); Mockito.verify(adapter) @@ -289,6 +291,15 @@ public T execute( any()); } + @Test + public void viewExistsFallbackToGETRequestWithLegacyServer() { + // simulate a legacy server that doesn't send back supported endpoints, thus the + // client relies on the default endpoints + verifyViewExistsFallbackToGETRequest( + ConfigResponse.builder().build(), + ImmutableMap.of(RESTSessionCatalog.VIEW_ENDPOINTS_SUPPORTED, "true")); + } + @Override protected RESTCatalog catalog() { return restCatalog; diff --git a/open-api/rest-catalog-open-api.yaml b/open-api/rest-catalog-open-api.yaml index 4b82fb3a3b0e..446adfe4f8d7 100644 --- a/open-api/rest-catalog-open-api.yaml +++ b/open-api/rest-catalog-open-api.yaml @@ -111,8 +111,6 @@ paths: - GET /v1/{prefix}/namespaces/{namespace} - - HEAD /v1/{prefix}/namespaces/{namespace} - - DELETE /v1/{prefix}/namespaces/{namespace} - POST /v1/{prefix}/namespaces/{namespace}/properties @@ -123,8 +121,6 @@ paths: - GET /v1/{prefix}/namespaces/{namespace}/tables/{table} - - HEAD /v1/{prefix}/namespaces/{namespace}/tables/{table} - - POST /v1/{prefix}/namespaces/{namespace}/tables/{table} - DELETE /v1/{prefix}/namespaces/{namespace}/tables/{table}