You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by et...@apache.org on 2024/09/13 16:04:20 UTC
(iceberg) branch main updated: Core: Allow servers to express supported endpoints via endpoint field in ConfigResponse (#10929)
This is an automated email from the ASF dual-hosted git repository.
etudenhoefner pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/main by this push:
new a2b8008da7 Core: Allow servers to express supported endpoints via endpoint field in ConfigResponse (#10929)
a2b8008da7 is described below
commit a2b8008da7bc26e03248a35eeee60d1cc7e8499d
Author: Eduard Tudenhoefner <et...@gmail.com>
AuthorDate: Fri Sep 13 18:04:14 2024 +0200
Core: Allow servers to express supported endpoints via endpoint field in ConfigResponse (#10929)
---
.../java/org/apache/iceberg/rest/Endpoint.java | 161 +++++++++++++++++++++
.../apache/iceberg/rest/RESTSessionCatalog.java | 156 +++++++++++++++-----
.../apache/iceberg/rest/RESTTableOperations.java | 13 +-
.../apache/iceberg/rest/RESTViewOperations.java | 11 +-
.../org/apache/iceberg/rest/ResourcePaths.java | 14 ++
.../iceberg/rest/responses/ConfigResponse.java | 29 +++-
.../rest/responses/ConfigResponseParser.java | 16 ++
.../apache/iceberg/rest/RESTCatalogAdapter.java | 103 +++++++------
.../java/org/apache/iceberg/rest/TestEndpoint.java | 108 ++++++++++++++
.../apache/iceberg/rest/TestRESTViewCatalog.java | 8 +-
.../TestRESTViewCatalogWithAssumedViewSupport.java | 95 ++++++++++++
.../rest/responses/TestConfigResponseParser.java | 79 ++++++++++
12 files changed, 704 insertions(+), 89 deletions(-)
diff --git a/core/src/main/java/org/apache/iceberg/rest/Endpoint.java b/core/src/main/java/org/apache/iceberg/rest/Endpoint.java
new file mode 100644
index 0000000000..2a8e6d6332
--- /dev/null
+++ b/core/src/main/java/org/apache/iceberg/rest/Endpoint.java
@@ -0,0 +1,161 @@
+/*
+ * 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 java.util.List;
+import java.util.Objects;
+import java.util.Set;
+import org.apache.hc.core5.http.Method;
+import org.apache.iceberg.relocated.com.google.common.base.Joiner;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Splitter;
+import org.apache.iceberg.relocated.com.google.common.base.Strings;
+import org.apache.iceberg.relocated.com.google.common.base.Supplier;
+
+/**
+ * Holds an endpoint definition that consists of the HTTP method (GET, POST, DELETE, ...) and the
+ * resource path as defined in the Iceberg OpenAPI REST specification without parameter
+ * substitution, such as <b>/v1/{prefix}/namespaces/{namespace}</b>.
+ */
+public class Endpoint {
+
+ // namespace endpoints
+ public static final Endpoint V1_LIST_NAMESPACES =
+ Endpoint.create("GET", ResourcePaths.V1_NAMESPACES);
+ public static final Endpoint V1_LOAD_NAMESPACE =
+ Endpoint.create("GET", ResourcePaths.V1_NAMESPACE);
+ public static final Endpoint V1_CREATE_NAMESPACE =
+ Endpoint.create("POST", ResourcePaths.V1_NAMESPACES);
+ public static final Endpoint V1_UPDATE_NAMESPACE =
+ Endpoint.create("POST", ResourcePaths.V1_NAMESPACE_PROPERTIES);
+ public static final Endpoint V1_DELETE_NAMESPACE =
+ Endpoint.create("DELETE", ResourcePaths.V1_NAMESPACE);
+ public static final Endpoint V1_COMMIT_TRANSACTION =
+ Endpoint.create("POST", ResourcePaths.V1_TRANSACTIONS_COMMIT);
+
+ // table endpoints
+ public static final Endpoint V1_LIST_TABLES = Endpoint.create("GET", ResourcePaths.V1_TABLES);
+ public static final Endpoint V1_LOAD_TABLE = Endpoint.create("GET", ResourcePaths.V1_TABLE);
+ public static final Endpoint V1_CREATE_TABLE = Endpoint.create("POST", ResourcePaths.V1_TABLES);
+ public static final Endpoint V1_UPDATE_TABLE = Endpoint.create("POST", ResourcePaths.V1_TABLE);
+ public static final Endpoint V1_DELETE_TABLE = Endpoint.create("DELETE", ResourcePaths.V1_TABLE);
+ public static final Endpoint V1_RENAME_TABLE =
+ Endpoint.create("POST", ResourcePaths.V1_TABLE_RENAME);
+ public static final Endpoint V1_REGISTER_TABLE =
+ Endpoint.create("POST", ResourcePaths.V1_TABLE_REGISTER);
+ public static final Endpoint V1_REPORT_METRICS =
+ Endpoint.create("POST", ResourcePaths.V1_TABLE_METRICS);
+
+ // view endpoints
+ public static final Endpoint V1_LIST_VIEWS = Endpoint.create("GET", ResourcePaths.V1_VIEWS);
+ public static final Endpoint V1_LOAD_VIEW = Endpoint.create("GET", ResourcePaths.V1_VIEW);
+ public static final Endpoint V1_CREATE_VIEW = Endpoint.create("POST", ResourcePaths.V1_VIEWS);
+ public static final Endpoint V1_UPDATE_VIEW = Endpoint.create("POST", ResourcePaths.V1_VIEW);
+ public static final Endpoint V1_DELETE_VIEW = Endpoint.create("DELETE", ResourcePaths.V1_VIEW);
+ public static final Endpoint V1_RENAME_VIEW =
+ Endpoint.create("POST", ResourcePaths.V1_VIEW_RENAME);
+
+ private static final Splitter ENDPOINT_SPLITTER = Splitter.on(" ");
+ private static final Joiner ENDPOINT_JOINER = Joiner.on(" ");
+ private final String httpMethod;
+ private final String path;
+
+ private Endpoint(String httpMethod, String path) {
+ Preconditions.checkArgument(
+ !Strings.isNullOrEmpty(httpMethod), "Invalid HTTP method: null or empty");
+ Preconditions.checkArgument(!Strings.isNullOrEmpty(path), "Invalid path: null or empty");
+ this.httpMethod = Method.normalizedValueOf(httpMethod).toString();
+ this.path = path;
+ }
+
+ public String httpMethod() {
+ return httpMethod;
+ }
+
+ public String path() {
+ return path;
+ }
+
+ public static Endpoint create(String httpMethod, String path) {
+ return new Endpoint(httpMethod, path);
+ }
+
+ @Override
+ public String toString() {
+ return ENDPOINT_JOINER.join(httpMethod(), path());
+ }
+
+ public static Endpoint fromString(String endpoint) {
+ List<String> elements = ENDPOINT_SPLITTER.splitToList(endpoint);
+ Preconditions.checkArgument(
+ elements.size() == 2,
+ "Invalid endpoint (must consist of two elements separated by a single space): %s",
+ endpoint);
+ return create(elements.get(0), elements.get(1));
+ }
+
+ /**
+ * Checks if the set of endpoints support the given {@link Endpoint}.
+ *
+ * @param supportedEndpoints The set of supported endpoints to check
+ * @param endpoint The endpoint to check against the set of supported endpoints
+ * @throws UnsupportedOperationException if the given {@link Endpoint} is not included in the set
+ * of endpoints.
+ */
+ public static void check(Set<Endpoint> supportedEndpoints, Endpoint endpoint) {
+ if (!supportedEndpoints.contains(endpoint)) {
+ throw new UnsupportedOperationException(
+ String.format("Server does not support endpoint: %s", endpoint));
+ }
+ }
+
+ /**
+ * Checks if the set of endpoints support the given {@link Endpoint}.
+ *
+ * @param supportedEndpoints The set of supported endpoints to check
+ * @param endpoint The endpoint to check against the set of supported endpoints
+ * @param supplier The supplier throwing a {@link RuntimeException} if the given {@link Endpoint}
+ * is not included in the set of endpoints.
+ */
+ public static void check(
+ Set<Endpoint> supportedEndpoints, Endpoint endpoint, Supplier<RuntimeException> supplier) {
+ if (!supportedEndpoints.contains(endpoint)) {
+ throw supplier.get();
+ }
+ }
+
+ @Override
+ public boolean equals(Object o) {
+ if (this == o) {
+ return true;
+ }
+
+ if (!(o instanceof Endpoint)) {
+ return false;
+ }
+
+ Endpoint endpoint = (Endpoint) o;
+ return Objects.equals(httpMethod, endpoint.httpMethod) && Objects.equals(path, endpoint.path);
+ }
+
+ @Override
+ public int hashCode() {
+ return Objects.hash(httpMethod, path);
+ }
+}
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 53ce45bb0a..cc42604f70 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
@@ -58,7 +58,6 @@ import org.apache.iceberg.exceptions.AlreadyExistsException;
import org.apache.iceberg.exceptions.NoSuchNamespaceException;
import org.apache.iceberg.exceptions.NoSuchTableException;
import org.apache.iceberg.exceptions.NoSuchViewException;
-import org.apache.iceberg.exceptions.RESTException;
import org.apache.iceberg.hadoop.Configurable;
import org.apache.iceberg.io.CloseableGroup;
import org.apache.iceberg.io.FileIO;
@@ -116,6 +115,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
+ 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(
@@ -132,6 +134,33 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
.addAll(TOKEN_PREFERENCE_ORDER)
.build();
+ private static final Set<Endpoint> DEFAULT_ENDPOINTS =
+ ImmutableSet.<Endpoint>builder()
+ .add(Endpoint.V1_LIST_NAMESPACES)
+ .add(Endpoint.V1_LOAD_NAMESPACE)
+ .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_CREATE_TABLE)
+ .add(Endpoint.V1_UPDATE_TABLE)
+ .add(Endpoint.V1_DELETE_TABLE)
+ .add(Endpoint.V1_RENAME_TABLE)
+ .add(Endpoint.V1_REGISTER_TABLE)
+ .add(Endpoint.V1_REPORT_METRICS)
+ .build();
+
+ private static final Set<Endpoint> VIEW_ENDPOINTS =
+ ImmutableSet.<Endpoint>builder()
+ .add(Endpoint.V1_LIST_VIEWS)
+ .add(Endpoint.V1_LOAD_VIEW)
+ .add(Endpoint.V1_CREATE_VIEW)
+ .add(Endpoint.V1_UPDATE_VIEW)
+ .add(Endpoint.V1_DELETE_VIEW)
+ .add(Endpoint.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;
@@ -148,6 +177,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
private boolean reportingViaRestEnabled;
private Integer pageSize = null;
private CloseableGroup closeables = null;
+ private Set<Endpoint> endpoints;
// a lazy thread pool for token refresh
private volatile ScheduledExecutorService refreshExecutor = null;
@@ -173,6 +203,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
this.ioBuilder = ioBuilder;
}
+ @SuppressWarnings("checkstyle:CyclomaticComplexity")
@Override
public void initialize(String name, Map<String, String> unresolved) {
Preconditions.checkArgument(unresolved != null, "Invalid configuration: null");
@@ -230,6 +261,18 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
Map<String, String> mergedProps = config.merge(props);
Map<String, String> baseHeaders = configHeaders(mergedProps);
+ 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);
this.keepTokenRefreshed =
@@ -316,6 +359,10 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public List<TableIdentifier> listTables(SessionContext context, Namespace ns) {
+ if (!endpoints.contains(Endpoint.V1_LIST_TABLES)) {
+ return ImmutableList.of();
+ }
+
checkNamespaceIsValid(ns);
Map<String, String> queryParams = Maps.newHashMap();
ImmutableList.Builder<TableIdentifier> tables = ImmutableList.builder();
@@ -342,6 +389,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public boolean dropTable(SessionContext context, TableIdentifier identifier) {
+ Endpoint.check(endpoints, Endpoint.V1_DELETE_TABLE);
checkIdentifierIsValid(identifier);
try {
@@ -355,6 +403,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public boolean purgeTable(SessionContext context, TableIdentifier identifier) {
+ Endpoint.check(endpoints, Endpoint.V1_DELETE_TABLE);
checkIdentifierIsValid(identifier);
try {
@@ -372,6 +421,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public void renameTable(SessionContext context, TableIdentifier from, TableIdentifier to) {
+ Endpoint.check(endpoints, Endpoint.V1_RENAME_TABLE);
checkIdentifierIsValid(from);
checkIdentifierIsValid(to);
@@ -384,6 +434,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
private LoadTableResponse loadInternal(
SessionContext context, TableIdentifier identifier, SnapshotMode mode) {
+ Endpoint.check(endpoints, Endpoint.V1_LOAD_TABLE);
return client.get(
paths.table(identifier),
mode.params(),
@@ -394,6 +445,14 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public Table loadTable(SessionContext context, TableIdentifier identifier) {
+ Endpoint.check(
+ endpoints,
+ Endpoint.V1_LOAD_TABLE,
+ () ->
+ new NoSuchTableException(
+ "Unable to load table %s.%s: Server does not support endpoint %s",
+ name(), identifier, Endpoint.V1_LOAD_TABLE));
+
checkIdentifierIsValid(identifier);
MetadataTableType metadataType;
@@ -448,7 +507,8 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
paths.table(finalIdentifier),
session::headers,
tableFileIO(context, response.config()),
- tableMetadata);
+ tableMetadata,
+ endpoints);
trackFileIO(ops);
@@ -472,7 +532,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
private MetricsReporter metricsReporter(
String metricsEndpoint, Supplier<Map<String, String>> headers) {
- if (reportingViaRestEnabled) {
+ if (reportingViaRestEnabled && endpoints.contains(Endpoint.V1_REPORT_METRICS)) {
RESTMetricsReporter restMetricsReporter =
new RESTMetricsReporter(client, metricsEndpoint, headers);
return MetricsReporters.combine(reporter, restMetricsReporter);
@@ -493,6 +553,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public Table registerTable(
SessionContext context, TableIdentifier ident, String metadataFileLocation) {
+ Endpoint.check(endpoints, Endpoint.V1_REGISTER_TABLE);
checkIdentifierIsValid(ident);
Preconditions.checkArgument(
@@ -521,7 +582,8 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
paths.table(ident),
session::headers,
tableFileIO(context, response.config()),
- response.tableMetadata());
+ response.tableMetadata(),
+ endpoints);
trackFileIO(ops);
@@ -532,6 +594,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public void createNamespace(
SessionContext context, Namespace namespace, Map<String, String> metadata) {
+ Endpoint.check(endpoints, Endpoint.V1_CREATE_NAMESPACE);
CreateNamespaceRequest request =
CreateNamespaceRequest.builder().withNamespace(namespace).setProperties(metadata).build();
@@ -546,6 +609,10 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public List<Namespace> listNamespaces(SessionContext context, Namespace namespace) {
+ if (!endpoints.contains(Endpoint.V1_LIST_NAMESPACES)) {
+ return ImmutableList.of();
+ }
+
Map<String, String> queryParams = Maps.newHashMap();
if (!namespace.isEmpty()) {
queryParams.put("parent", RESTUtil.encodeNamespace(namespace));
@@ -575,6 +642,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public Map<String, String> loadNamespaceMetadata(SessionContext context, Namespace ns) {
+ Endpoint.check(endpoints, Endpoint.V1_LOAD_NAMESPACE);
checkNamespaceIsValid(ns);
// TODO: rename to LoadNamespaceResponse?
@@ -589,6 +657,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public boolean dropNamespace(SessionContext context, Namespace ns) {
+ Endpoint.check(endpoints, Endpoint.V1_DELETE_NAMESPACE);
checkNamespaceIsValid(ns);
try {
@@ -603,6 +672,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public boolean updateNamespaceMetadata(
SessionContext context, Namespace ns, Map<String, String> updates, Set<String> removals) {
+ Endpoint.check(endpoints, Endpoint.V1_UPDATE_NAMESPACE);
checkNamespaceIsValid(ns);
UpdateNamespacePropertiesRequest request =
@@ -719,6 +789,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public Table create() {
+ Endpoint.check(endpoints, Endpoint.V1_CREATE_TABLE);
CreateTableRequest request =
CreateTableRequest.builder()
.withName(ident.name())
@@ -744,7 +815,8 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
paths.table(ident),
session::headers,
tableFileIO(context, response.config()),
- response.tableMetadata());
+ response.tableMetadata(),
+ endpoints);
trackFileIO(ops);
@@ -754,6 +826,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public Transaction createTransaction() {
+ Endpoint.check(endpoints, Endpoint.V1_CREATE_TABLE);
LoadTableResponse response = stageCreate();
String fullName = fullTableName(ident);
@@ -768,7 +841,8 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
tableFileIO(context, response.config()),
RESTTableOperations.UpdateType.CREATE,
createChanges(meta),
- meta);
+ meta,
+ endpoints);
trackFileIO(ops);
@@ -778,15 +852,9 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public Transaction replaceTransaction() {
- try {
- if (viewExists(context, ident)) {
- throw new AlreadyExistsException("View with same name already exists: %s", ident);
- }
- } catch (RESTException | UnsupportedOperationException e) {
- // don't fail if the server doesn't support views, which could be due to:
- // 1. server or backing catalog doesn't support views
- // 2. newer client talks to an older server that doesn't support views
- LOG.debug("Failed to check whether view {} exists", ident, e);
+ Endpoint.check(endpoints, Endpoint.V1_UPDATE_TABLE);
+ if (viewExists(context, ident)) {
+ throw new AlreadyExistsException("View with same name already exists: %s", ident);
}
LoadTableResponse response = loadInternal(context, ident, snapshotMode);
@@ -832,7 +900,8 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
tableFileIO(context, response.config()),
RESTTableOperations.UpdateType.REPLACE,
changes.build(),
- base);
+ base,
+ endpoints);
trackFileIO(ops);
@@ -1085,6 +1154,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
}
public void commitTransaction(SessionContext context, List<TableCommit> commits) {
+ Endpoint.check(endpoints, Endpoint.V1_COMMIT_TRANSACTION);
List<UpdateTableRequest> tableChanges = Lists.newArrayListWithCapacity(commits.size());
for (TableCommit commit : commits) {
@@ -1102,6 +1172,10 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public List<TableIdentifier> listViews(SessionContext context, Namespace namespace) {
+ if (!endpoints.contains(Endpoint.V1_LIST_VIEWS)) {
+ return ImmutableList.of();
+ }
+
checkNamespaceIsValid(namespace);
Map<String, String> queryParams = Maps.newHashMap();
ImmutableList.Builder<TableIdentifier> views = ImmutableList.builder();
@@ -1128,30 +1202,29 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public View loadView(SessionContext context, TableIdentifier identifier) {
+ Endpoint.check(
+ endpoints,
+ Endpoint.V1_LOAD_VIEW,
+ () ->
+ new NoSuchViewException(
+ "Unable to load view %s.%s: Server does not support endpoint %s",
+ name(), identifier, Endpoint.V1_LOAD_VIEW));
+
checkViewIdentifierIsValid(identifier);
- LoadViewResponse response;
- try {
- response =
- client.get(
- paths.view(identifier),
- LoadViewResponse.class,
- headers(context),
- ErrorHandlers.viewErrorHandler());
- } catch (UnsupportedOperationException | RESTException e) {
- // Normally, copying an exception message is a bad practice but engines may show just the
- // message and suppress the exception cause when the view does not exist. Since 401 and 403
- // responses can trigger this case, including the message increases the chances that the "Not
- // authorized" or "Forbidden" message is preserved and shown.
- throw new NoSuchViewException(
- e, "Unable to load view %s.%s: %s", name(), identifier, e.getMessage());
- }
+ LoadViewResponse response =
+ client.get(
+ paths.view(identifier),
+ LoadViewResponse.class,
+ headers(context),
+ ErrorHandlers.viewErrorHandler());
AuthSession session = tableSession(response.config(), session(context));
ViewMetadata metadata = response.metadata();
RESTViewOperations ops =
- new RESTViewOperations(client, paths.view(identifier), session::headers, metadata);
+ new RESTViewOperations(
+ client, paths.view(identifier), session::headers, metadata, endpoints);
return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
}
@@ -1163,6 +1236,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public boolean dropView(SessionContext context, TableIdentifier identifier) {
+ Endpoint.check(endpoints, Endpoint.V1_DELETE_VIEW);
checkViewIdentifierIsValid(identifier);
try {
@@ -1176,6 +1250,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public void renameView(SessionContext context, TableIdentifier from, TableIdentifier to) {
+ Endpoint.check(endpoints, Endpoint.V1_RENAME_VIEW);
checkViewIdentifierIsValid(from);
checkViewIdentifierIsValid(to);
@@ -1247,6 +1322,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
@Override
public View create() {
+ Endpoint.check(endpoints, Endpoint.V1_CREATE_VIEW);
Preconditions.checkState(
!representations.isEmpty(), "Cannot create view without specifying a query");
Preconditions.checkState(null != schema, "Cannot create view without specifying schema");
@@ -1284,7 +1360,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
AuthSession session = tableSession(response.config(), session(context));
RESTViewOperations ops =
new RESTViewOperations(
- client, paths.view(identifier), session::headers, response.metadata());
+ client, paths.view(identifier), session::headers, response.metadata(), endpoints);
return new BaseView(ops, ViewUtil.fullViewName(name(), identifier));
}
@@ -1308,6 +1384,14 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
}
private LoadViewResponse loadView() {
+ Endpoint.check(
+ endpoints,
+ Endpoint.V1_LOAD_VIEW,
+ () ->
+ new NoSuchViewException(
+ "Unable to load view %s.%s: Server does not support endpoint %s",
+ name(), identifier, Endpoint.V1_LOAD_VIEW));
+
return client.get(
paths.view(identifier),
LoadViewResponse.class,
@@ -1316,6 +1400,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
}
private View replace(LoadViewResponse response) {
+ Endpoint.check(endpoints, Endpoint.V1_UPDATE_VIEW);
Preconditions.checkState(
!representations.isEmpty(), "Cannot replace view without specifying a query");
Preconditions.checkState(null != schema, "Cannot replace view without specifying schema");
@@ -1354,7 +1439,8 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog
AuthSession session = tableSession(response.config(), session(context));
RESTViewOperations ops =
- new RESTViewOperations(client, paths.view(identifier), session::headers, metadata);
+ new RESTViewOperations(
+ client, paths.view(identifier), session::headers, metadata, endpoints);
ops.commit(metadata, replacement);
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java b/core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java
index 0ce1afd93a..5f6c28b323 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTTableOperations.java
@@ -21,6 +21,7 @@ package org.apache.iceberg.rest;
import java.util.List;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.function.Consumer;
import java.util.function.Supplier;
import org.apache.iceberg.LocationProviders;
@@ -56,6 +57,7 @@ class RESTTableOperations implements TableOperations {
private final FileIO io;
private final List<MetadataUpdate> createChanges;
private final TableMetadata replaceBase;
+ private final Set<Endpoint> endpoints;
private UpdateType updateType;
private TableMetadata current;
@@ -64,8 +66,9 @@ class RESTTableOperations implements TableOperations {
String path,
Supplier<Map<String, String>> headers,
FileIO io,
- TableMetadata current) {
- this(client, path, headers, io, UpdateType.SIMPLE, Lists.newArrayList(), current);
+ TableMetadata current,
+ Set<Endpoint> endpoints) {
+ this(client, path, headers, io, UpdateType.SIMPLE, Lists.newArrayList(), current, endpoints);
}
RESTTableOperations(
@@ -75,7 +78,8 @@ class RESTTableOperations implements TableOperations {
FileIO io,
UpdateType updateType,
List<MetadataUpdate> createChanges,
- TableMetadata current) {
+ TableMetadata current,
+ Set<Endpoint> endpoints) {
this.client = client;
this.path = path;
this.headers = headers;
@@ -88,6 +92,7 @@ class RESTTableOperations implements TableOperations {
} else {
this.current = current;
}
+ this.endpoints = endpoints;
}
@Override
@@ -97,12 +102,14 @@ class RESTTableOperations implements TableOperations {
@Override
public TableMetadata refresh() {
+ Endpoint.check(endpoints, Endpoint.V1_LOAD_TABLE);
return updateCurrentMetadata(
client.get(path, LoadTableResponse.class, headers, ErrorHandlers.tableErrorHandler()));
}
@Override
public void commit(TableMetadata base, TableMetadata metadata) {
+ Endpoint.check(endpoints, Endpoint.V1_UPDATE_TABLE);
Consumer<ErrorResponse> errorHandler;
List<UpdateRequirement> requirements;
List<MetadataUpdate> updates;
diff --git a/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java b/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java
index b4dafaa903..466a8e6689 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTViewOperations.java
@@ -20,6 +20,7 @@ package org.apache.iceberg.rest;
import java.util.Map;
import java.util.Objects;
+import java.util.Set;
import java.util.function.Supplier;
import org.apache.iceberg.UpdateRequirements;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
@@ -32,15 +33,21 @@ class RESTViewOperations implements ViewOperations {
private final RESTClient client;
private final String path;
private final Supplier<Map<String, String>> headers;
+ private final Set<Endpoint> endpoints;
private ViewMetadata current;
RESTViewOperations(
- RESTClient client, String path, Supplier<Map<String, String>> headers, ViewMetadata current) {
+ RESTClient client,
+ String path,
+ Supplier<Map<String, String>> headers,
+ ViewMetadata current,
+ Set<Endpoint> endpoints) {
Preconditions.checkArgument(null != current, "Invalid view metadata: null");
this.client = client;
this.path = path;
this.headers = headers;
this.current = current;
+ this.endpoints = endpoints;
}
@Override
@@ -50,12 +57,14 @@ class RESTViewOperations implements ViewOperations {
@Override
public ViewMetadata refresh() {
+ Endpoint.check(endpoints, Endpoint.V1_LOAD_VIEW);
return updateCurrentMetadata(
client.get(path, LoadViewResponse.class, headers, ErrorHandlers.viewErrorHandler()));
}
@Override
public void commit(ViewMetadata base, ViewMetadata metadata) {
+ Endpoint.check(endpoints, Endpoint.V1_UPDATE_VIEW);
// this is only used for replacing view metadata
Preconditions.checkState(base != null, "Invalid base metadata: null");
diff --git a/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java b/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
index c68a4f4508..5ba7eae282 100644
--- a/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
+++ b/core/src/main/java/org/apache/iceberg/rest/ResourcePaths.java
@@ -26,6 +26,20 @@ import org.apache.iceberg.relocated.com.google.common.base.Joiner;
public class ResourcePaths {
private static final Joiner SLASH = Joiner.on("/").skipNulls();
private static final String PREFIX = "prefix";
+ public static final String V1_NAMESPACES = "/v1/{prefix}/namespaces";
+ public static final String V1_NAMESPACE = "/v1/{prefix}/namespaces/{namespace}";
+ public static final String V1_NAMESPACE_PROPERTIES =
+ "/v1/{prefix}/namespaces/{namespace}/properties";
+ public static final String V1_TABLES = "/v1/{prefix}/namespaces/{namespace}/tables";
+ public static final String V1_TABLE = "/v1/{prefix}/namespaces/{namespace}/tables/{table}";
+ public static final String V1_TABLE_REGISTER = "/v1/{prefix}/namespaces/{namespace}/register";
+ public static final String V1_TABLE_METRICS =
+ "/v1/{prefix}/namespaces/{namespace}/tables/{table}/metrics";
+ public static final String V1_TABLE_RENAME = "/v1/{prefix}/tables/rename";
+ public static final String V1_TRANSACTIONS_COMMIT = "/v1/{prefix}/transactions/commit";
+ public static final String V1_VIEWS = "/v1/{prefix}/namespaces/{namespace}/views";
+ public static final String V1_VIEW = "/v1/{prefix}/namespaces/{namespace}/views/{view}";
+ public static final String V1_VIEW_RENAME = "/v1/{prefix}/views/rename";
public static ResourcePaths forCatalogProperties(Map<String, String> properties) {
return new ResourcePaths(properties.get(PREFIX));
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java
index f4efc0ff28..da22ca287b 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponse.java
@@ -18,12 +18,16 @@
*/
package org.apache.iceberg.rest.responses;
+import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.apache.iceberg.relocated.com.google.common.base.MoreObjects;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.rest.Endpoint;
import org.apache.iceberg.rest.RESTResponse;
/**
@@ -39,20 +43,24 @@ import org.apache.iceberg.rest.RESTResponse;
* <ul>
* <li>defaults - properties that should be used as default configuration
* <li>overrides - properties that should be used to override client configuration
+ * <li>endpoints - a list of endpoints that the server supports
* </ul>
*/
public class ConfigResponse implements RESTResponse {
private Map<String, String> defaults;
private Map<String, String> overrides;
+ private List<Endpoint> endpoints;
public ConfigResponse() {
// Required for Jackson deserialization
}
- private ConfigResponse(Map<String, String> defaults, Map<String, String> overrides) {
+ private ConfigResponse(
+ Map<String, String> defaults, Map<String, String> overrides, List<Endpoint> endpoints) {
this.defaults = defaults;
this.overrides = overrides;
+ this.endpoints = endpoints;
validate();
}
@@ -80,6 +88,15 @@ public class ConfigResponse implements RESTResponse {
return overrides != null ? overrides : ImmutableMap.of();
}
+ /**
+ * The list of available endpoints that the server supports
+ *
+ * @return A list of available endpoints that the server supports
+ */
+ public List<Endpoint> endpoints() {
+ return null != endpoints ? endpoints : ImmutableList.of();
+ }
+
/**
* Merge client-provided config with server side provided configuration to return a single
* properties map which will be used for instantiating and configuring the REST catalog.
@@ -107,6 +124,7 @@ public class ConfigResponse implements RESTResponse {
return MoreObjects.toStringHelper(this)
.add("defaults", defaults)
.add("overrides", overrides)
+ .add("endpoints", endpoints)
.toString();
}
@@ -117,10 +135,12 @@ public class ConfigResponse implements RESTResponse {
public static class Builder {
private final Map<String, String> defaults;
private final Map<String, String> overrides;
+ private final List<Endpoint> endpoints;
private Builder() {
this.defaults = Maps.newHashMap();
this.overrides = Maps.newHashMap();
+ this.endpoints = Lists.newArrayList();
}
public Builder withDefault(String key, String value) {
@@ -153,8 +173,13 @@ public class ConfigResponse implements RESTResponse {
return this;
}
+ public Builder withEndpoints(List<Endpoint> endpointsToAdd) {
+ endpoints.addAll(endpointsToAdd);
+ return this;
+ }
+
public ConfigResponse build() {
- return new ConfigResponse(defaults, overrides);
+ return new ConfigResponse(defaults, overrides, endpoints);
}
}
}
diff --git a/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java
index 3240840e3e..acadcce6d4 100644
--- a/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java
+++ b/core/src/main/java/org/apache/iceberg/rest/responses/ConfigResponseParser.java
@@ -21,13 +21,16 @@ package org.apache.iceberg.rest.responses;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.databind.JsonNode;
import java.io.IOException;
+import java.util.stream.Collectors;
import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.rest.Endpoint;
import org.apache.iceberg.util.JsonUtil;
public class ConfigResponseParser {
private static final String DEFAULTS = "defaults";
private static final String OVERRIDES = "overrides";
+ private static final String ENDPOINTS = "endpoints";
private ConfigResponseParser() {}
@@ -46,6 +49,12 @@ public class ConfigResponseParser {
JsonUtil.writeStringMap(DEFAULTS, response.defaults(), gen);
JsonUtil.writeStringMap(OVERRIDES, response.overrides(), gen);
+ if (!response.endpoints().isEmpty()) {
+ JsonUtil.writeStringArray(
+ ENDPOINTS,
+ response.endpoints().stream().map(Endpoint::toString).collect(Collectors.toList()),
+ gen);
+ }
gen.writeEndObject();
}
@@ -67,6 +76,13 @@ public class ConfigResponseParser {
builder.withOverrides(JsonUtil.getStringMapNullableValues(OVERRIDES, json));
}
+ if (json.hasNonNull(ENDPOINTS)) {
+ builder.withEndpoints(
+ JsonUtil.getStringList(ENDPOINTS, json).stream()
+ .map(Endpoint::fromString)
+ .collect(Collectors.toList()));
+ }
+
return builder.build();
}
}
diff --git a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
index 2c928c06e5..6477dfcd00 100644
--- a/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
+++ b/core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java
@@ -19,9 +19,11 @@
package org.apache.iceberg.rest;
import java.io.IOException;
+import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.function.Consumer;
+import java.util.stream.Collectors;
import org.apache.iceberg.BaseTable;
import org.apache.iceberg.BaseTransaction;
import org.apache.iceberg.Table;
@@ -115,61 +117,51 @@ public class RESTCatalogAdapter implements RESTClient {
SEPARATE_AUTH_TOKENS_URI(
HTTPMethod.POST, "https://auth-server.com/token", null, OAuthTokenResponse.class),
CONFIG(HTTPMethod.GET, "v1/config", null, ConfigResponse.class),
- LIST_NAMESPACES(HTTPMethod.GET, "v1/namespaces", null, ListNamespacesResponse.class),
+ LIST_NAMESPACES(
+ HTTPMethod.GET, ResourcePaths.V1_NAMESPACES, null, ListNamespacesResponse.class),
CREATE_NAMESPACE(
HTTPMethod.POST,
- "v1/namespaces",
+ ResourcePaths.V1_NAMESPACES,
CreateNamespaceRequest.class,
CreateNamespaceResponse.class),
- LOAD_NAMESPACE(HTTPMethod.GET, "v1/namespaces/{namespace}", null, GetNamespaceResponse.class),
- DROP_NAMESPACE(HTTPMethod.DELETE, "v1/namespaces/{namespace}"),
+ LOAD_NAMESPACE(HTTPMethod.GET, ResourcePaths.V1_NAMESPACE, null, GetNamespaceResponse.class),
+ DROP_NAMESPACE(HTTPMethod.DELETE, ResourcePaths.V1_NAMESPACE),
UPDATE_NAMESPACE(
HTTPMethod.POST,
- "v1/namespaces/{namespace}/properties",
+ ResourcePaths.V1_NAMESPACE_PROPERTIES,
UpdateNamespacePropertiesRequest.class,
UpdateNamespacePropertiesResponse.class),
- LIST_TABLES(HTTPMethod.GET, "v1/namespaces/{namespace}/tables", null, ListTablesResponse.class),
+ LIST_TABLES(HTTPMethod.GET, ResourcePaths.V1_TABLES, null, ListTablesResponse.class),
CREATE_TABLE(
HTTPMethod.POST,
- "v1/namespaces/{namespace}/tables",
+ ResourcePaths.V1_TABLES,
CreateTableRequest.class,
LoadTableResponse.class),
- LOAD_TABLE(
- HTTPMethod.GET, "v1/namespaces/{namespace}/tables/{name}", null, LoadTableResponse.class),
+ LOAD_TABLE(HTTPMethod.GET, ResourcePaths.V1_TABLE, null, LoadTableResponse.class),
REGISTER_TABLE(
HTTPMethod.POST,
- "v1/namespaces/{namespace}/register",
+ ResourcePaths.V1_TABLE_REGISTER,
RegisterTableRequest.class,
LoadTableResponse.class),
UPDATE_TABLE(
- HTTPMethod.POST,
- "v1/namespaces/{namespace}/tables/{name}",
- UpdateTableRequest.class,
- LoadTableResponse.class),
- DROP_TABLE(HTTPMethod.DELETE, "v1/namespaces/{namespace}/tables/{name}"),
- RENAME_TABLE(HTTPMethod.POST, "v1/tables/rename", RenameTableRequest.class, null),
+ HTTPMethod.POST, ResourcePaths.V1_TABLE, UpdateTableRequest.class, LoadTableResponse.class),
+ DROP_TABLE(HTTPMethod.DELETE, ResourcePaths.V1_TABLE),
+ RENAME_TABLE(HTTPMethod.POST, ResourcePaths.V1_TABLE_RENAME, RenameTableRequest.class, null),
REPORT_METRICS(
+ HTTPMethod.POST, ResourcePaths.V1_TABLE_METRICS, ReportMetricsRequest.class, null),
+ COMMIT_TRANSACTION(
HTTPMethod.POST,
- "v1/namespaces/{namespace}/tables/{name}/metrics",
- ReportMetricsRequest.class,
+ ResourcePaths.V1_TRANSACTIONS_COMMIT,
+ CommitTransactionRequest.class,
null),
- COMMIT_TRANSACTION(
- HTTPMethod.POST, "v1/transactions/commit", CommitTransactionRequest.class, null),
- LIST_VIEWS(HTTPMethod.GET, "v1/namespaces/{namespace}/views", null, ListTablesResponse.class),
- LOAD_VIEW(
- HTTPMethod.GET, "v1/namespaces/{namespace}/views/{name}", null, LoadViewResponse.class),
+ LIST_VIEWS(HTTPMethod.GET, ResourcePaths.V1_VIEWS, null, ListTablesResponse.class),
+ LOAD_VIEW(HTTPMethod.GET, ResourcePaths.V1_VIEW, null, LoadViewResponse.class),
CREATE_VIEW(
- HTTPMethod.POST,
- "v1/namespaces/{namespace}/views",
- CreateViewRequest.class,
- LoadViewResponse.class),
+ HTTPMethod.POST, ResourcePaths.V1_VIEWS, CreateViewRequest.class, LoadViewResponse.class),
UPDATE_VIEW(
- HTTPMethod.POST,
- "v1/namespaces/{namespace}/views/{name}",
- UpdateTableRequest.class,
- LoadViewResponse.class),
- RENAME_VIEW(HTTPMethod.POST, "v1/views/rename", RenameTableRequest.class, null),
- DROP_VIEW(HTTPMethod.DELETE, "v1/namespaces/{namespace}/views/{name}");
+ HTTPMethod.POST, ResourcePaths.V1_VIEW, UpdateTableRequest.class, LoadViewResponse.class),
+ RENAME_VIEW(HTTPMethod.POST, ResourcePaths.V1_VIEW_RENAME, RenameTableRequest.class, null),
+ DROP_VIEW(HTTPMethod.DELETE, ResourcePaths.V1_VIEW);
private final HTTPMethod method;
private final int requiredLength;
@@ -177,6 +169,7 @@ public class RESTCatalogAdapter implements RESTClient {
private final Map<Integer, String> variables;
private final Class<? extends RESTRequest> requestClass;
private final Class<? extends RESTResponse> responseClass;
+ private final String resourcePath;
Route(HTTPMethod method, String pattern) {
this(method, pattern, null, null);
@@ -188,9 +181,11 @@ public class RESTCatalogAdapter implements RESTClient {
Class<? extends RESTRequest> requestClass,
Class<? extends RESTResponse> responseClass) {
this.method = method;
+ this.resourcePath = pattern;
// parse the pattern into requirements and variables
- List<String> parts = SLASH.splitToList(pattern);
+ List<String> parts =
+ SLASH.splitToList(pattern.replaceFirst("/v1/", "v1/").replace("/{prefix}", ""));
ImmutableMap.Builder<Integer, String> requirementsBuilder = ImmutableMap.builder();
ImmutableMap.Builder<Integer, String> variablesBuilder = ImmutableMap.builder();
for (int pos = 0; pos < parts.size(); pos += 1) {
@@ -245,6 +240,14 @@ public class RESTCatalogAdapter implements RESTClient {
public Class<? extends RESTResponse> responseClass() {
return responseClass;
}
+
+ HTTPMethod method() {
+ return method;
+ }
+
+ String resourcePath() {
+ return resourcePath;
+ }
}
private static OAuthTokenResponse handleOAuthRequest(Object body) {
@@ -282,7 +285,14 @@ public class RESTCatalogAdapter implements RESTClient {
return castResponse(responseType, handleOAuthRequest(body));
case CONFIG:
- return castResponse(responseType, ConfigResponse.builder().build());
+ return castResponse(
+ responseType,
+ ConfigResponse.builder()
+ .withEndpoints(
+ Arrays.stream(Route.values())
+ .map(r -> Endpoint.create(r.method.name(), r.resourcePath))
+ .collect(Collectors.toList()))
+ .build());
case LIST_NAMESPACES:
if (asNamespaceCatalog != null) {
@@ -371,16 +381,16 @@ public class RESTCatalogAdapter implements RESTClient {
case DROP_TABLE:
{
if (PropertyUtil.propertyAsBoolean(vars, "purgeRequested", false)) {
- CatalogHandlers.purgeTable(catalog, identFromPathVars(vars));
+ CatalogHandlers.purgeTable(catalog, tableIdentFromPathVars(vars));
} else {
- CatalogHandlers.dropTable(catalog, identFromPathVars(vars));
+ CatalogHandlers.dropTable(catalog, tableIdentFromPathVars(vars));
}
return null;
}
case LOAD_TABLE:
{
- TableIdentifier ident = identFromPathVars(vars);
+ TableIdentifier ident = tableIdentFromPathVars(vars);
return castResponse(responseType, CatalogHandlers.loadTable(catalog, ident));
}
@@ -394,7 +404,7 @@ public class RESTCatalogAdapter implements RESTClient {
case UPDATE_TABLE:
{
- TableIdentifier ident = identFromPathVars(vars);
+ TableIdentifier ident = tableIdentFromPathVars(vars);
UpdateTableRequest request = castRequest(UpdateTableRequest.class, body);
return castResponse(responseType, CatalogHandlers.updateTable(catalog, ident, request));
}
@@ -452,7 +462,7 @@ public class RESTCatalogAdapter implements RESTClient {
case LOAD_VIEW:
{
if (null != asViewCatalog) {
- TableIdentifier ident = identFromPathVars(vars);
+ TableIdentifier ident = viewIdentFromPathVars(vars);
return castResponse(responseType, CatalogHandlers.loadView(asViewCatalog, ident));
}
break;
@@ -461,7 +471,7 @@ public class RESTCatalogAdapter implements RESTClient {
case UPDATE_VIEW:
{
if (null != asViewCatalog) {
- TableIdentifier ident = identFromPathVars(vars);
+ TableIdentifier ident = viewIdentFromPathVars(vars);
UpdateTableRequest request = castRequest(UpdateTableRequest.class, body);
return castResponse(
responseType, CatalogHandlers.updateView(asViewCatalog, ident, request));
@@ -482,7 +492,7 @@ public class RESTCatalogAdapter implements RESTClient {
case DROP_VIEW:
{
if (null != asViewCatalog) {
- CatalogHandlers.dropView(asViewCatalog, identFromPathVars(vars));
+ CatalogHandlers.dropView(asViewCatalog, viewIdentFromPathVars(vars));
return null;
}
break;
@@ -668,8 +678,13 @@ public class RESTCatalogAdapter implements RESTClient {
return RESTUtil.decodeNamespace(pathVars.get("namespace"));
}
- private static TableIdentifier identFromPathVars(Map<String, String> pathVars) {
+ private static TableIdentifier tableIdentFromPathVars(Map<String, String> pathVars) {
+ return TableIdentifier.of(
+ namespaceFromPathVars(pathVars), RESTUtil.decodeString(pathVars.get("table")));
+ }
+
+ private static TableIdentifier viewIdentFromPathVars(Map<String, String> pathVars) {
return TableIdentifier.of(
- namespaceFromPathVars(pathVars), RESTUtil.decodeString(pathVars.get("name")));
+ namespaceFromPathVars(pathVars), RESTUtil.decodeString(pathVars.get("view")));
}
}
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 0000000000..1873d87998
--- /dev/null
+++ b/core/src/test/java/org/apache/iceberg/rest/TestEndpoint.java
@@ -0,0 +1,108 @@
+/*
+ * 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.assertThatCode;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableSet;
+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 invalidValues() {
+ 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");
+
+ assertThatThrownBy(() -> Endpoint.create("invalid", "/"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage("No enum constant org.apache.hc.core5.http.Method.INVALID");
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"/path", " GET /path", "GET /path ", "GET /path", "GET /path /other"})
+ public void invalidFromString(String endpoint) {
+ assertThatThrownBy(() -> Endpoint.fromString(endpoint))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Invalid endpoint (must consist of two elements separated by a single 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", "/")).asString().isEqualTo("PUT /");
+ assertThat(Endpoint.create("PUT", "/namespaces/{namespace}/{x}"))
+ .asString()
+ .isEqualTo("PUT /namespaces/{namespace}/{x}");
+ }
+
+ @Test
+ public void supportedEndpoints() {
+ assertThatCode(
+ () -> Endpoint.check(ImmutableSet.of(Endpoint.V1_LOAD_TABLE), Endpoint.V1_LOAD_TABLE))
+ .doesNotThrowAnyException();
+
+ assertThatCode(
+ () ->
+ Endpoint.check(
+ ImmutableSet.of(Endpoint.V1_LOAD_TABLE, Endpoint.V1_LOAD_VIEW),
+ Endpoint.V1_LOAD_TABLE))
+ .doesNotThrowAnyException();
+ }
+
+ @Test
+ public void unsupportedEndpoints() {
+ assertThatThrownBy(() -> Endpoint.check(ImmutableSet.of(), Endpoint.V1_LOAD_TABLE))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessage("Server does not support endpoint: %s", Endpoint.V1_LOAD_TABLE);
+
+ assertThatThrownBy(
+ () -> Endpoint.check(ImmutableSet.of(Endpoint.V1_LOAD_VIEW), Endpoint.V1_LOAD_TABLE))
+ .isInstanceOf(UnsupportedOperationException.class)
+ .hasMessage("Server does not support endpoint: %s", Endpoint.V1_LOAD_TABLE);
+ }
+}
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 db0969620d..dac2a9d25f 100644
--- a/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalog.java
@@ -57,11 +57,11 @@ import org.mockito.Mockito;
public class TestRESTViewCatalog extends ViewCatalogTests<RESTCatalog> {
private static final ObjectMapper MAPPER = RESTObjectMapper.mapper();
- @TempDir private Path temp;
+ @TempDir protected Path temp;
- private RESTCatalog restCatalog;
- private InMemoryCatalog backendCatalog;
- private Server httpServer;
+ protected RESTCatalog restCatalog;
+ protected InMemoryCatalog backendCatalog;
+ protected Server httpServer;
@BeforeEach
public void createCatalog() throws Exception {
diff --git a/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalogWithAssumedViewSupport.java b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalogWithAssumedViewSupport.java
new file mode 100644
index 0000000000..3d7d64ddb7
--- /dev/null
+++ b/core/src/test/java/org/apache/iceberg/rest/TestRESTViewCatalogWithAssumedViewSupport.java
@@ -0,0 +1,95 @@
+/*
+ * 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.apache.iceberg.rest.RESTCatalogAdapter.Route.CONFIG;
+
+import java.io.File;
+import java.util.Map;
+import java.util.UUID;
+import org.apache.iceberg.CatalogProperties;
+import org.apache.iceberg.catalog.SessionCatalog;
+import org.apache.iceberg.inmemory.InMemoryCatalog;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.rest.responses.ConfigResponse;
+import org.eclipse.jetty.server.Server;
+import org.eclipse.jetty.server.handler.gzip.GzipHandler;
+import org.eclipse.jetty.servlet.ServletContextHandler;
+import org.eclipse.jetty.servlet.ServletHolder;
+import org.junit.jupiter.api.BeforeEach;
+
+public class TestRESTViewCatalogWithAssumedViewSupport extends TestRESTViewCatalog {
+
+ @BeforeEach
+ public void createCatalog() throws Exception {
+ File warehouse = temp.toFile();
+
+ this.backendCatalog = new InMemoryCatalog();
+ this.backendCatalog.initialize(
+ "in-memory",
+ ImmutableMap.of(CatalogProperties.WAREHOUSE_LOCATION, warehouse.getAbsolutePath()));
+
+ RESTCatalogAdapter adaptor =
+ new RESTCatalogAdapter(backendCatalog) {
+
+ @Override
+ public <T extends RESTResponse> T handleRequest(
+ Route route, Map<String, String> vars, Object body, Class<T> responseType) {
+ if (CONFIG == route) {
+ // simulate a legacy server that doesn't send back supported endpoints
+ return castResponse(responseType, ConfigResponse.builder().build());
+ }
+
+ return super.handleRequest(route, vars, body, responseType);
+ }
+ };
+
+ ServletContextHandler servletContext =
+ new ServletContextHandler(ServletContextHandler.NO_SESSIONS);
+ servletContext.setContextPath("/");
+ servletContext.addServlet(new ServletHolder(new RESTCatalogServlet(adaptor)), "/*");
+ servletContext.setHandler(new GzipHandler());
+
+ this.httpServer = new Server(0);
+ httpServer.setHandler(servletContext);
+ httpServer.start();
+
+ SessionCatalog.SessionContext context =
+ new SessionCatalog.SessionContext(
+ UUID.randomUUID().toString(),
+ "user",
+ ImmutableMap.of("credential", "user:12345"),
+ ImmutableMap.of());
+
+ this.restCatalog =
+ new RESTCatalog(
+ context,
+ (config) -> HTTPClient.builder(config).uri(config.get(CatalogProperties.URI)).build());
+ restCatalog.initialize(
+ "prod",
+ ImmutableMap.of(
+ CatalogProperties.URI,
+ httpServer.getURI().toString(),
+ "credential",
+ "catalog:12345",
+ // assume that the server supports view endpoints
+ RESTSessionCatalog.VIEW_ENDPOINTS_SUPPORTED,
+ "true"));
+ }
+}
diff --git a/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java b/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java
index ec4c793c27..81ec7cc558 100644
--- a/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java
+++ b/core/src/test/java/org/apache/iceberg/rest/responses/TestConfigResponseParser.java
@@ -22,8 +22,11 @@ import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;
import com.fasterxml.jackson.databind.JsonNode;
+import java.util.List;
import java.util.Map;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.relocated.com.google.common.collect.Maps;
+import org.apache.iceberg.rest.Endpoint;
import org.junit.jupiter.api.Test;
public class TestConfigResponseParser {
@@ -106,6 +109,27 @@ public class TestConfigResponseParser {
.isEqualTo(expectedJson);
}
+ @Test
+ public void endpointsOnly() {
+ List<Endpoint> endpoints =
+ ImmutableList.of(
+ Endpoint.V1_LOAD_NAMESPACE, Endpoint.V1_LIST_NAMESPACES, Endpoint.V1_CREATE_NAMESPACE);
+ ConfigResponse response = ConfigResponse.builder().withEndpoints(endpoints).build();
+
+ String expectedJson =
+ "{\n"
+ + " \"defaults\" : { },\n"
+ + " \"overrides\" : { },\n"
+ + " \"endpoints\" : [ \"GET /v1/{prefix}/namespaces/{namespace}\", \"GET /v1/{prefix}/namespaces\", \"POST /v1/{prefix}/namespaces\" ]\n"
+ + "}";
+
+ String json = ConfigResponseParser.toJson(response, true);
+ assertThat(json).isEqualTo(expectedJson);
+ assertThat(ConfigResponseParser.toJson(ConfigResponseParser.fromJson(json), true))
+ .isEqualTo(expectedJson);
+ assertThat(ConfigResponseParser.fromJson(json).endpoints()).isEqualTo(response.endpoints());
+ }
+
@Test
public void roundTripSerde() {
Map<String, String> defaults = Maps.newHashMap();
@@ -135,4 +159,59 @@ public class TestConfigResponseParser {
assertThat(ConfigResponseParser.toJson(ConfigResponseParser.fromJson(json), true))
.isEqualTo(expectedJson);
}
+
+ @Test
+ public void invalidEndpoint() {
+ assertThatThrownBy(
+ () ->
+ ConfigResponseParser.fromJson(
+ "{\"endpoints\":[\"GET_v1/namespaces/{namespace}\"]}"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Invalid endpoint (must consist of two elements separated by a single space): GET_v1/namespaces/{namespace}");
+
+ assertThatThrownBy(
+ () ->
+ ConfigResponseParser.fromJson(
+ "{\"endpoints\":[\"GET v1/namespaces/{namespace} INVALID\"]}"))
+ .isInstanceOf(IllegalArgumentException.class)
+ .hasMessage(
+ "Invalid endpoint (must consist of two elements separated by a single space): GET v1/namespaces/{namespace} INVALID");
+ }
+
+ @Test
+ public void roundTripSerdeWithEndpoints() {
+ Map<String, String> defaults = Maps.newHashMap();
+ defaults.put("key1", "1");
+ defaults.put("key2", null);
+
+ Map<String, String> overrides = Maps.newHashMap();
+ overrides.put("key3", "23");
+ overrides.put("key4", null);
+
+ ConfigResponse response =
+ ConfigResponse.builder()
+ .withDefaults(defaults)
+ .withOverrides(overrides)
+ .withEndpoints(ImmutableList.of(Endpoint.V1_LOAD_TABLE, Endpoint.V1_LOAD_VIEW))
+ .build();
+
+ String expectedJson =
+ "{\n"
+ + " \"defaults\" : {\n"
+ + " \"key1\" : \"1\",\n"
+ + " \"key2\" : null\n"
+ + " },\n"
+ + " \"overrides\" : {\n"
+ + " \"key3\" : \"23\",\n"
+ + " \"key4\" : null\n"
+ + " },\n"
+ + " \"endpoints\" : [ \"GET /v1/{prefix}/namespaces/{namespace}/tables/{table}\", \"GET /v1/{prefix}/namespaces/{namespace}/views/{view}\" ]\n"
+ + "}";
+
+ String json = ConfigResponseParser.toJson(response, true);
+ assertThat(json).isEqualTo(expectedJson);
+ assertThat(ConfigResponseParser.toJson(ConfigResponseParser.fromJson(json), true))
+ .isEqualTo(expectedJson);
+ }
}