[iceberg] branch master updated: Core: Check for valid identifiers in REST catalog (#5107)

[dweeks]

This is an automated email from the ASF dual-hosted git repository.

dweeks pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git


The following commit(s) were added to refs/heads/master by this push:
     new e335086e6c Core: Check for valid identifiers in REST catalog (#5107)
e335086e6c is described below

commit e335086e6c3e59d506190bb5ba9013d47c515b3f
Author: Bryan Keller <brya...@gmail.com>
AuthorDate: Tue Jun 21 15:51:38 2022 -0700

    Core: Check for valid identifiers in REST catalog (#5107)
    
    * Shutdown refresh token thread during REST catalog client close
    
    * Improved shutdown of token refresh executor during REST catalog close
    
    * REST: Set table format version for create table transactions
    
    * add test for creating v2 table via transaction
    
    * Core: Check for valid identifiers in REST catalog
---
 .../apache/iceberg/rest/RESTSessionCatalog.java    | 29 ++++++++++++++++++++++
 1 file changed, 29 insertions(+)

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 13baec98fd..6a53bb984a 100644
--- a/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
+++ b/core/src/main/java/org/apache/iceberg/rest/RESTSessionCatalog.java
@@ -176,6 +176,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
 
   @Override
   public List<TableIdentifier> listTables(SessionContext context, Namespace 
ns) {
+    checkNamespaceIsValid(ns);
+
     ListTablesResponse response = client.get(
         paths.tables(ns), ListTablesResponse.class, headers(context), 
ErrorHandlers.namespaceErrorHandler());
     return response.identifiers();
@@ -183,6 +185,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
 
   @Override
   public boolean dropTable(SessionContext context, TableIdentifier identifier) 
{
+    checkIdentifierIsValid(identifier);
+
     try {
       client.delete(paths.table(identifier), null, headers(context), 
ErrorHandlers.tableErrorHandler());
       return true;
@@ -198,6 +202,9 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
 
   @Override
   public void renameTable(SessionContext context, TableIdentifier from, 
TableIdentifier to) {
+    checkIdentifierIsValid(from);
+    checkIdentifierIsValid(to);
+
     RenameTableRequest request = RenameTableRequest.builder()
         .withSource(from)
         .withDestination(to)
@@ -214,6 +221,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
 
   @Override
   public Table loadTable(SessionContext context, TableIdentifier identifier) {
+    checkIdentifierIsValid(identifier);
+
     MetadataTableType metadataType;
     LoadTableResponse response;
     try {
@@ -295,6 +304,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
 
   @Override
   public Map<String, String> loadNamespaceMetadata(SessionContext context, 
Namespace ns) {
+    checkNamespaceIsValid(ns);
+
     // TODO: rename to LoadNamespaceResponse?
     GetNamespaceResponse response = client
         .get(paths.namespace(ns), GetNamespaceResponse.class, 
headers(context), ErrorHandlers.namespaceErrorHandler());
@@ -303,6 +314,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
 
   @Override
   public boolean dropNamespace(SessionContext context, Namespace ns) {
+    checkNamespaceIsValid(ns);
+
     try {
       client.delete(paths.namespace(ns), null, headers(context), 
ErrorHandlers.namespaceErrorHandler());
       return true;
@@ -314,6 +327,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
   @Override
   public boolean updateNamespaceMetadata(SessionContext context, Namespace ns,
                                          Map<String, String> updates, 
Set<String> removals) {
+    checkNamespaceIsValid(ns);
+
     UpdateNamespacePropertiesRequest request = 
UpdateNamespacePropertiesRequest.builder()
         .updateAll(updates)
         .removeAll(removals)
@@ -404,6 +419,8 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
     private String location = null;
 
     private Builder(TableIdentifier ident, Schema schema, SessionContext 
context) {
+      checkIdentifierIsValid(ident);
+
       this.ident = ident;
       this.schema = schema;
       this.context = context;
@@ -678,6 +695,18 @@ public class RESTSessionCatalog extends BaseSessionCatalog 
implements Configurab
     }
   }
 
+  private void checkIdentifierIsValid(TableIdentifier tableIdentifier) {
+    if (tableIdentifier.namespace().isEmpty()) {
+      throw new NoSuchTableException("Invalid table identifier: %s", 
tableIdentifier);
+    }
+  }
+
+  private void checkNamespaceIsValid(Namespace namespace) {
+    if (namespace.isEmpty()) {
+      throw new NoSuchNamespaceException("Invalid namespace: %s", namespace);
+    }
+  }
+
   private static Map<String, String> configHeaders(Map<String, String> 
properties) {
     return RESTUtil.extractPrefixMap(properties, "header.");
   }