From e261fa80691513954a9e3829876ae9e825c854e2 Mon Sep 17 00:00:00 2001 From: Eduard Tudenhoefner Date: Wed, 21 Aug 2024 14:20:46 +0200 Subject: [PATCH] add flag to assume view endpoint support + some tests --- .../iceberg/rest/RESTSessionCatalog.java | 26 ++++++- .../org/apache/iceberg/rest/TestEndpoint.java | 74 +++++++++++++++++++ 2 files changed, 98 insertions(+), 2 deletions(-) create mode 100644 core/src/test/java/org/apache/iceberg/rest/TestEndpoint.java 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 f291f1b13736..a9fa31f7d7f2 100644 --- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java +++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java @@ -116,6 +116,9 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog private static final String DEFAULT_FILE_IO_IMPL = "org.apache.iceberg.io.ResolvingFileIO"; private static final String REST_METRICS_REPORTING_ENABLED = "rest-metrics-reporting-enabled"; private static final String REST_SNAPSHOT_LOADING_MODE = "snapshot-loading-mode"; + // for backwards compatibility with older REST servers where it can be assumed that a particular + // server supports view endpoints but doesn't send the "endpoints" field in the ConfigResponse + private static final String VIEW_ENDPOINTS_SUPPORTED = "view-endpoints-supported"; public static final String REST_PAGE_SIZE = "rest-page-size"; private static final List TOKEN_PREFERENCE_ORDER = ImmutableList.of( @@ -149,6 +152,16 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog .add(ResourcePaths.V1_REPORT_METRICS) .build(); + private static final Set VIEW_ENDPOINTS = + ImmutableSet.builder() + .add(ResourcePaths.V1_LIST_VIEWS) + .add(ResourcePaths.V1_LOAD_VIEW) + .add(ResourcePaths.V1_CREATE_VIEW) + .add(ResourcePaths.V1_UPDATE_VIEW) + .add(ResourcePaths.V1_DELETE_VIEW) + .add(ResourcePaths.V1_RENAME_VIEW) + .build(); + private final Function, RESTClient> clientBuilder; private final BiFunction, FileIO> ioBuilder; private Cache sessions = null; @@ -247,8 +260,17 @@ public void initialize(String name, Map unresolved) { // build the final configuration and set up the catalog's auth Map mergedProps = config.merge(props); Map baseHeaders = configHeaders(mergedProps); - this.endpoints = - config.endpoints().isEmpty() ? DEFAULT_ENDPOINTS : ImmutableSet.copyOf(config.endpoints()); + if (config.endpoints().isEmpty()) { + this.endpoints = + PropertyUtil.propertyAsBoolean(mergedProps, VIEW_ENDPOINTS_SUPPORTED, false) + ? ImmutableSet.builder() + .addAll(DEFAULT_ENDPOINTS) + .addAll(VIEW_ENDPOINTS) + .build() + : DEFAULT_ENDPOINTS; + } else { + this.endpoints = ImmutableSet.copyOf(config.endpoints()); + } this.sessions = newSessionCache(mergedProps); this.tableSessions = newSessionCache(mergedProps); diff --git a/core/src/test/java/org/apache/iceberg/rest/TestEndpoint.java b/core/src/test/java/org/apache/iceberg/rest/TestEndpoint.java new file mode 100644 index 000000000000..0aec289b46ee --- /dev/null +++ b/core/src/test/java/org/apache/iceberg/rest/TestEndpoint.java @@ -0,0 +1,74 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you 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.apache.iceberg.rest; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +public class TestEndpoint { + @Test + public void nullOrEmptyValues() { + assertThatThrownBy(() -> Endpoint.create(null, "endpoint")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid HTTP method: null or empty"); + + assertThatThrownBy(() -> Endpoint.create("", "endpoint")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid HTTP method: null or empty"); + + assertThatThrownBy(() -> Endpoint.create("GET", null)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid path: null or empty"); + + assertThatThrownBy(() -> Endpoint.create("GET", "")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid path: null or empty"); + } + + @ParameterizedTest + @ValueSource(strings = {"/path", " GET /path", "GET /path ", "GET /path"}) + public void invalidFromString(String endpoint) { + assertThatThrownBy(() -> Endpoint.fromString(endpoint)) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage( + "Invalid endpoint (must consist of two elements separated by space): %s", endpoint); + } + + @Test + public void validFromString() { + Endpoint endpoint = Endpoint.fromString("GET /path"); + assertThat(endpoint.httpMethod()).isEqualTo("GET"); + assertThat(endpoint.path()).isEqualTo("/path"); + } + + @Test + public void toStringRepresentation() { + assertThat(Endpoint.create("POST", "/path/of/resource")) + .asString() + .isEqualTo("POST /path/of/resource"); + assertThat(Endpoint.create("GET", "/")).asString().isEqualTo("GET /"); + assertThat(Endpoint.create("PUT", "/namespaces/{namespace}/{x}")) + .asString() + .isEqualTo("PUT /namespaces/{namespace}/{x}"); + } +}