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 000000000000..2a8e6d633297 --- /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 /v1/{prefix}/namespaces/{namespace}. + */ +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 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 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 supportedEndpoints, Endpoint endpoint, Supplier 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 53ce45bb0a3f..cc42604f700d 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.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 TOKEN_PREFERENCE_ORDER = ImmutableList.of( @@ -132,6 +134,33 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog .addAll(TOKEN_PREFERENCE_ORDER) .build(); + private static final Set DEFAULT_ENDPOINTS = + ImmutableSet.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 VIEW_ENDPOINTS = + ImmutableSet.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, RESTClient> clientBuilder; private final BiFunction, FileIO> ioBuilder; private Cache sessions = null; @@ -148,6 +177,7 @@ public class RESTSessionCatalog extends BaseViewSessionCatalog private boolean reportingViaRestEnabled; private Integer pageSize = null; private CloseableGroup closeables = null; + private Set endpoints; // a lazy thread pool for token refresh private volatile ScheduledExecutorService refreshExecutor = null; @@ -173,6 +203,7 @@ public RESTSessionCatalog( this.ioBuilder = ioBuilder; } + @SuppressWarnings("checkstyle:CyclomaticComplexity") @Override public void initialize(String name, Map unresolved) { Preconditions.checkArgument(unresolved != null, "Invalid configuration: null"); @@ -230,6 +261,18 @@ public void initialize(String name, Map unresolved) { Map mergedProps = config.merge(props); Map baseHeaders = configHeaders(mergedProps); + 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); this.keepTokenRefreshed = @@ -316,6 +359,10 @@ public void setConf(Object newConf) { @Override public List listTables(SessionContext context, Namespace ns) { + if (!endpoints.contains(Endpoint.V1_LIST_TABLES)) { + return ImmutableList.of(); + } + checkNamespaceIsValid(ns); Map queryParams = Maps.newHashMap(); ImmutableList.Builder tables = ImmutableList.builder(); @@ -342,6 +389,7 @@ public List listTables(SessionContext context, Namespace ns) { @Override public boolean dropTable(SessionContext context, TableIdentifier identifier) { + Endpoint.check(endpoints, Endpoint.V1_DELETE_TABLE); checkIdentifierIsValid(identifier); try { @@ -355,6 +403,7 @@ public boolean dropTable(SessionContext context, TableIdentifier identifier) { @Override public boolean purgeTable(SessionContext context, TableIdentifier identifier) { + Endpoint.check(endpoints, Endpoint.V1_DELETE_TABLE); checkIdentifierIsValid(identifier); try { @@ -372,6 +421,7 @@ public boolean purgeTable(SessionContext context, TableIdentifier identifier) { @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 void renameTable(SessionContext context, TableIdentifier from, TableIdent 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 @@ private LoadTableResponse loadInternal( @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 Table loadTable(SessionContext context, TableIdentifier identifier) { paths.table(finalIdentifier), session::headers, tableFileIO(context, response.config()), - tableMetadata); + tableMetadata, + endpoints); trackFileIO(ops); @@ -472,7 +532,7 @@ private void trackFileIO(RESTTableOperations ops) { private MetricsReporter metricsReporter( String metricsEndpoint, Supplier> 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 void invalidateTable(SessionContext context, TableIdentifier ident) {} @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 Table registerTable( paths.table(ident), session::headers, tableFileIO(context, response.config()), - response.tableMetadata()); + response.tableMetadata(), + endpoints); trackFileIO(ops); @@ -532,6 +594,7 @@ public Table registerTable( @Override public void createNamespace( SessionContext context, Namespace namespace, Map metadata) { + Endpoint.check(endpoints, Endpoint.V1_CREATE_NAMESPACE); CreateNamespaceRequest request = CreateNamespaceRequest.builder().withNamespace(namespace).setProperties(metadata).build(); @@ -546,6 +609,10 @@ public void createNamespace( @Override public List listNamespaces(SessionContext context, Namespace namespace) { + if (!endpoints.contains(Endpoint.V1_LIST_NAMESPACES)) { + return ImmutableList.of(); + } + Map queryParams = Maps.newHashMap(); if (!namespace.isEmpty()) { queryParams.put("parent", RESTUtil.encodeNamespace(namespace)); @@ -575,6 +642,7 @@ public List listNamespaces(SessionContext context, Namespace namespac @Override public Map loadNamespaceMetadata(SessionContext context, Namespace ns) { + Endpoint.check(endpoints, Endpoint.V1_LOAD_NAMESPACE); checkNamespaceIsValid(ns); // TODO: rename to LoadNamespaceResponse? @@ -589,6 +657,7 @@ public Map loadNamespaceMetadata(SessionContext context, Namespa @Override public boolean dropNamespace(SessionContext context, Namespace ns) { + Endpoint.check(endpoints, Endpoint.V1_DELETE_NAMESPACE); checkNamespaceIsValid(ns); try { @@ -603,6 +672,7 @@ public boolean dropNamespace(SessionContext context, Namespace ns) { @Override public boolean updateNamespaceMetadata( SessionContext context, Namespace ns, Map updates, Set removals) { + Endpoint.check(endpoints, Endpoint.V1_UPDATE_NAMESPACE); checkNamespaceIsValid(ns); UpdateNamespacePropertiesRequest request = @@ -719,6 +789,7 @@ public Builder withProperty(String key, String value) { @Override public Table create() { + Endpoint.check(endpoints, Endpoint.V1_CREATE_TABLE); CreateTableRequest request = CreateTableRequest.builder() .withName(ident.name()) @@ -744,7 +815,8 @@ public Table create() { paths.table(ident), session::headers, tableFileIO(context, response.config()), - response.tableMetadata()); + response.tableMetadata(), + endpoints); trackFileIO(ops); @@ -754,6 +826,7 @@ public Table create() { @Override public Transaction createTransaction() { + Endpoint.check(endpoints, Endpoint.V1_CREATE_TABLE); LoadTableResponse response = stageCreate(); String fullName = fullTableName(ident); @@ -768,7 +841,8 @@ public Transaction createTransaction() { tableFileIO(context, response.config()), RESTTableOperations.UpdateType.CREATE, createChanges(meta), - meta); + meta, + endpoints); trackFileIO(ops); @@ -778,15 +852,9 @@ public Transaction createTransaction() { @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 Transaction replaceTransaction() { tableFileIO(context, response.config()), RESTTableOperations.UpdateType.REPLACE, changes.build(), - base); + base, + endpoints); trackFileIO(ops); @@ -1085,6 +1154,7 @@ private static Cache newSessionCache(Map pr } public void commitTransaction(SessionContext context, List commits) { + Endpoint.check(endpoints, Endpoint.V1_COMMIT_TRANSACTION); List tableChanges = Lists.newArrayListWithCapacity(commits.size()); for (TableCommit commit : commits) { @@ -1102,6 +1172,10 @@ public void commitTransaction(SessionContext context, List commits) @Override public List listViews(SessionContext context, Namespace namespace) { + if (!endpoints.contains(Endpoint.V1_LIST_VIEWS)) { + return ImmutableList.of(); + } + checkNamespaceIsValid(namespace); Map queryParams = Maps.newHashMap(); ImmutableList.Builder views = ImmutableList.builder(); @@ -1128,30 +1202,29 @@ public List listViews(SessionContext context, Namespace namespa @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 RESTViewBuilder buildView(SessionContext context, TableIdentifier identif @Override public boolean dropView(SessionContext context, TableIdentifier identifier) { + Endpoint.check(endpoints, Endpoint.V1_DELETE_VIEW); checkViewIdentifierIsValid(identifier); try { @@ -1176,6 +1250,7 @@ public boolean dropView(SessionContext context, TableIdentifier identifier) { @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 ViewBuilder withLocation(String newLocation) { @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 View create() { 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 View replace() { } 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 @@ private LoadViewResponse loadView() { } 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 @@ private View replace(LoadViewResponse response) { 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 0ce1afd93a79..5f6c28b32337 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 @@ 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 @@ enum UpdateType { private final FileIO io; private final List createChanges; private final TableMetadata replaceBase; + private final Set endpoints; private UpdateType updateType; private TableMetadata current; @@ -64,8 +66,9 @@ enum UpdateType { String path, Supplier> headers, FileIO io, - TableMetadata current) { - this(client, path, headers, io, UpdateType.SIMPLE, Lists.newArrayList(), current); + TableMetadata current, + Set endpoints) { + this(client, path, headers, io, UpdateType.SIMPLE, Lists.newArrayList(), current, endpoints); } RESTTableOperations( @@ -75,7 +78,8 @@ enum UpdateType { FileIO io, UpdateType updateType, List createChanges, - TableMetadata current) { + TableMetadata current, + Set endpoints) { this.client = client; this.path = path; this.headers = headers; @@ -88,6 +92,7 @@ enum UpdateType { } else { this.current = current; } + this.endpoints = endpoints; } @Override @@ -97,12 +102,14 @@ public TableMetadata current() { @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 errorHandler; List requirements; List 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 b4dafaa9031b..466a8e66899b 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 @@ 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> headers; + private final Set endpoints; private ViewMetadata current; RESTViewOperations( - RESTClient client, String path, Supplier> headers, ViewMetadata current) { + RESTClient client, + String path, + Supplier> headers, + ViewMetadata current, + Set 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 @@ public ViewMetadata current() { @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 c68a4f450843..5ba7eae28262 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 @@ 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 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 f4efc0ff281a..da22ca287b30 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 @@ *
    *
  • defaults - properties that should be used as default configuration *
  • overrides - properties that should be used to override client configuration + *
  • endpoints - a list of endpoints that the server supports *
*/ public class ConfigResponse implements RESTResponse { private Map defaults; private Map overrides; + private List endpoints; public ConfigResponse() { // Required for Jackson deserialization } - private ConfigResponse(Map defaults, Map overrides) { + private ConfigResponse( + Map defaults, Map overrides, List endpoints) { this.defaults = defaults; this.overrides = overrides; + this.endpoints = endpoints; validate(); } @@ -80,6 +88,15 @@ public Map overrides() { 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 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 String toString() { return MoreObjects.toStringHelper(this) .add("defaults", defaults) .add("overrides", overrides) + .add("endpoints", endpoints) .toString(); } @@ -117,10 +135,12 @@ public static Builder builder() { public static class Builder { private final Map defaults; private final Map overrides; + private final List 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 Builder withOverrides(Map overridesToAdd) { return this; } + public Builder withEndpoints(List 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 3240840e3e93..acadcce6d4bf 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 @@ 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 static void toJson(ConfigResponse response, JsonGenerator gen) throws IOE 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 static ConfigResponse fromJson(JsonNode json) { 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 2c928c06e52b..6477dfcd00eb 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 @@ enum Route { 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 @@ enum Route { private final Map variables; private final Class requestClass; private final Class responseClass; + private final String resourcePath; Route(HTTPMethod method, String pattern) { this(method, pattern, null, null); @@ -188,9 +181,11 @@ enum Route { Class requestClass, Class responseClass) { this.method = method; + this.resourcePath = pattern; // parse the pattern into requirements and variables - List parts = SLASH.splitToList(pattern); + List parts = + SLASH.splitToList(pattern.replaceFirst("/v1/", "v1/").replace("/{prefix}", "")); ImmutableMap.Builder requirementsBuilder = ImmutableMap.builder(); ImmutableMap.Builder variablesBuilder = ImmutableMap.builder(); for (int pos = 0; pos < parts.size(); pos += 1) { @@ -245,6 +240,14 @@ public Class requestClass() { public Class responseClass() { return responseClass; } + + HTTPMethod method() { + return method; + } + + String resourcePath() { + return resourcePath; + } } private static OAuthTokenResponse handleOAuthRequest(Object body) { @@ -282,7 +285,14 @@ public T handleRequest( 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 T handleRequest( 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 T handleRequest( 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 T handleRequest( 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 T handleRequest( 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 T handleRequest( case DROP_VIEW: { if (null != asViewCatalog) { - CatalogHandlers.dropView(asViewCatalog, identFromPathVars(vars)); + CatalogHandlers.dropView(asViewCatalog, viewIdentFromPathVars(vars)); return null; } break; @@ -668,8 +678,13 @@ private static Namespace namespaceFromPathVars(Map pathVars) { return RESTUtil.decodeNamespace(pathVars.get("namespace")); } - private static TableIdentifier identFromPathVars(Map pathVars) { + private static TableIdentifier tableIdentFromPathVars(Map pathVars) { + return TableIdentifier.of( + namespaceFromPathVars(pathVars), RESTUtil.decodeString(pathVars.get("table"))); + } + + private static TableIdentifier viewIdentFromPathVars(Map 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 000000000000..1873d8799894 --- /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 db0969620dc9..dac2a9d25f4b 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 @@ public class TestRESTViewCatalog extends ViewCatalogTests { 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 000000000000..3d7d64ddb794 --- /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 handleRequest( + Route route, Map vars, Object body, Class 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 ec4c793c279f..81ec7cc5585c 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.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 void overridesOnly() { .isEqualTo(expectedJson); } + @Test + public void endpointsOnly() { + List 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 defaults = Maps.newHashMap(); @@ -135,4 +159,59 @@ public void roundTripSerde() { 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 defaults = Maps.newHashMap(); + defaults.put("key1", "1"); + defaults.put("key2", null); + + Map 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); + } }