Skip to content

Commit

Permalink
add flag to assume view endpoint support + some tests
Browse files Browse the repository at this point in the history
  • Loading branch information
nastra committed Aug 21, 2024
1 parent 74c5ec7 commit e261fa8
Show file tree
Hide file tree
Showing 2 changed files with 98 additions and 2 deletions.
26 changes: 24 additions & 2 deletions core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> TOKEN_PREFERENCE_ORDER =
ImmutableList.of(
Expand Down Expand Up @@ -149,6 +152,16 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
.add(ResourcePaths.V1_REPORT_METRICS)
.build();

private static final Set<Endpoint> VIEW_ENDPOINTS =
ImmutableSet.<Endpoint>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<Map<String, String>, RESTClient> clientBuilder;
private final BiFunction<SessionContext, Map<String, String>, FileIO> ioBuilder;
private Cache<String, AuthSession> sessions = null;
Expand Down Expand Up @@ -247,8 +260,17 @@ public void initialize(String name, Map<String, String> unresolved) {
// build the final configuration and set up the catalog's auth
Map<String, String> mergedProps = config.merge(props);
Map<String, String> 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.<Endpoint>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);
Expand Down
74 changes: 74 additions & 0 deletions core/src/test/java/org/apache/iceberg/rest/TestEndpoint.java
Original file line number Diff line number Diff line change
@@ -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}");
}
}

0 comments on commit e261fa8

Please sign in to comment.