Core: Update REST catalog to use 204/404 for DELETE

core/src/main/java/org/apache/iceberg/rest/CatalogHandlers.java

@@ -40,6 +40,8 @@
 import org.apache.iceberg.catalog.TableIdentifier;
 import org.apache.iceberg.exceptions.AlreadyExistsException;
 import org.apache.iceberg.exceptions.CommitFailedException;
+import org.apache.iceberg.exceptions.NoSuchNamespaceException;
+import org.apache.iceberg.exceptions.NoSuchTableException;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
 import org.apache.iceberg.relocated.com.google.common.collect.Maps;
 import org.apache.iceberg.relocated.com.google.common.collect.Sets;
@@ -48,8 +50,6 @@
 import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
 import org.apache.iceberg.rest.requests.UpdateTableRequest;
 import org.apache.iceberg.rest.responses.CreateNamespaceResponse;
-import org.apache.iceberg.rest.responses.DropNamespaceResponse;
-import org.apache.iceberg.rest.responses.DropTableResponse;
 import org.apache.iceberg.rest.responses.GetNamespaceResponse;
 import org.apache.iceberg.rest.responses.ListNamespacesResponse;
 import org.apache.iceberg.rest.responses.ListTablesResponse;
@@ -117,11 +117,11 @@ public static GetNamespaceResponse loadNamespace(SupportsNamespaces catalog, Nam
         .build();
   }
 
-  public static DropNamespaceResponse dropNamespace(SupportsNamespaces catalog, Namespace namespace) {
+  public static void dropNamespace(SupportsNamespaces catalog, Namespace namespace) {
     boolean dropped = catalog.dropNamespace(namespace);
-    return DropNamespaceResponse.builder()
-        .dropped(dropped)
-        .build();
+    if (!dropped) {
+      throw new NoSuchNamespaceException("Namespace does not exist: %s", namespace);
+    }
   }
 
   public static UpdateNamespacePropertiesResponse updateNamespaceProperties(
@@ -199,9 +199,11 @@ public static LoadTableResponse createTable(Catalog catalog, Namespace namespace
     throw new IllegalStateException("Cannot wrap catalog that does not produce BaseTable");
   }
 
-  public static DropTableResponse dropTable(Catalog catalog, TableIdentifier ident) {
+  public static void dropTable(Catalog catalog, TableIdentifier ident) {
     boolean dropped = catalog.dropTable(ident);
-    return DropTableResponse.builder().dropped(dropped).build();
+    if (!dropped) {
+      throw new NoSuchTableException("Table does not exist: %s", ident);
+    }
   }
 
   public static LoadTableResponse loadTable(Catalog catalog, TableIdentifier ident) {

core/src/main/java/org/apache/iceberg/rest/HTTPClient.java

@@ -85,7 +85,7 @@ private static String extractResponseBodyAsString(CloseableHttpResponse response
   // Per the spec, the only currently defined / used "success" responses are 200 and 202.
   private static boolean isSuccessful(CloseableHttpResponse response) {
     int code = response.getCode();
-    return code ==  HttpStatus.SC_OK || code == HttpStatus.SC_ACCEPTED;
+    return code ==  HttpStatus.SC_OK || code == HttpStatus.SC_ACCEPTED || code == HttpStatus.SC_NO_CONTENT;
   }
 
   private static ErrorResponse buildDefaultErrorResponse(CloseableHttpResponse response) {
@@ -165,7 +165,7 @@ private <T> T execute(
     try (CloseableHttpResponse response = httpClient.execute(request)) {
 
       // Skip parsing the response stream for any successful request not expecting a response body
-      if (responseType == null && isSuccessful(response)) {
+      if (response.getCode() == HttpStatus.SC_NO_CONTENT || (responseType == null && isSuccessful(response))) {
         return null;
       }
 

core/src/main/java/org/apache/iceberg/rest/RESTCatalog.java

@@ -53,8 +53,6 @@
 import org.apache.iceberg.rest.requests.CreateTableRequest;
 import org.apache.iceberg.rest.requests.UpdateNamespacePropertiesRequest;
 import org.apache.iceberg.rest.responses.CreateNamespaceResponse;
-import org.apache.iceberg.rest.responses.DropNamespaceResponse;
-import org.apache.iceberg.rest.responses.DropTableResponse;
 import org.apache.iceberg.rest.responses.GetNamespaceResponse;
 import org.apache.iceberg.rest.responses.ListNamespacesResponse;
 import org.apache.iceberg.rest.responses.ListTablesResponse;
@@ -105,9 +103,12 @@ public List<TableIdentifier> listTables(Namespace namespace) {
   public boolean dropTable(TableIdentifier identifier, boolean purge) {
     String tablePath = tablePath(identifier);
     // TODO: support purge flagN
-    DropTableResponse response = client.delete(
-        tablePath, DropTableResponse.class, ErrorHandlers.tableErrorHandler());
-    return response.isDropped();
+    try {
+      client.delete(tablePath, null, ErrorHandlers.tableErrorHandler());
+      return true;
+    } catch (NoSuchTableException e) {
+      return false;
+    }
   }
 
   @Override
@@ -162,9 +163,12 @@ public Map<String, String> loadNamespaceMetadata(Namespace namespace) throws NoS
   @Override
   public boolean dropNamespace(Namespace namespace) throws NamespaceNotEmptyException {
     String ns = RESTUtil.urlEncode(namespace);
-    DropNamespaceResponse response = client
-        .delete("v1/namespaces/" + ns, DropNamespaceResponse.class, ErrorHandlers.namespaceErrorHandler());
-    return response.isDropped();
+    try {
+      client.delete("v1/namespaces/" + ns, null, ErrorHandlers.namespaceErrorHandler());
+      return true;
+    } catch (NoSuchNamespaceException e) {
+      return false;
+    }
   }
 
   @Override

core/src/test/java/org/apache/iceberg/rest/RESTCatalogAdapter.java

@@ -180,8 +180,8 @@ public <T> T handleRequest(Route route, Map<String, String> vars, Object body, C
 
       case DROP_NAMESPACE:
         if (asNamespaceCatalog != null) {
-          Namespace namespace = namespaceFromPathVars(vars);
-          return castResponse(responseType, CatalogHandlers.dropNamespace(asNamespaceCatalog, namespace));
+          CatalogHandlers.dropNamespace(asNamespaceCatalog, namespaceFromPathVars(vars));
+          return null;
         }
         break;
 
@@ -212,8 +212,8 @@ public <T> T handleRequest(Route route, Map<String, String> vars, Object body, C
       }
 
       case DROP_TABLE: {
-        TableIdentifier ident = identFromPathVars(vars);
-        return castResponse(responseType, CatalogHandlers.dropTable(catalog, ident));
+        CatalogHandlers.dropTable(catalog, identFromPathVars(vars));
+        return null;
       }
 
       case LOAD_TABLE: {
@@ -230,7 +230,7 @@ public <T> T handleRequest(Route route, Map<String, String> vars, Object body, C
       default:
     }
 
-    return null; // will be converted to a 400
+    return null;
   }
 
   public <T> T execute(HTTPMethod method, String path, Object body, Class<T> responseType,
@@ -239,16 +239,7 @@ public <T> T execute(HTTPMethod method, String path, Object body, Class<T> respo
     Pair<Route, Map<String, String>> routeAndVars = Route.from(method, path);
     if (routeAndVars != null) {
       try {
-        T response = handleRequest(routeAndVars.first(), routeAndVars.second(), body, responseType);
-        if (response != null) {
-          return response;
-        }
-
-        // if a response was not returned, there was no handler for the route
-        errorBuilder
-            .responseCode(400)
-            .withType("BadRequestException")
-            .withMessage(String.format("No handler for request: %s %s", method, path));
+        return handleRequest(routeAndVars.first(), routeAndVars.second(), body, responseType);
 
       } catch (RuntimeException e) {
         configureResponseFromException(e, errorBuilder);