This is an automated email from the ASF dual-hosted git repository.
roryqi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 8d7139fdfd [#11132] fix(iceberg-rest): Fix NamespaceNotEmptyException
to return HTTP 409 (#11168)
8d7139fdfd is described below
commit 8d7139fdfd67a902d46c0485ff517b75f109dce0
Author: MaSai <[email protected]>
AuthorDate: Thu May 21 15:09:22 2026 +0800
[#11132] fix(iceberg-rest): Fix NamespaceNotEmptyException to return HTTP
409 (#11168)
### What changes were proposed in this pull request?
Map `NamespaceNotEmptyException` to HTTP 409 Conflict in
`IcebergExceptionMapper`, aligned with the Iceberg REST spec and
upstream `RESTCatalogAdapter`. Stop converting
`NamespaceNotEmptyException` to `BadRequestException` in
`convertToIcebergException`. Update unit tests and add REST test
coverage for dropping a non-empty namespace.
### Why are the changes needed?
The Iceberg REST spec requires HTTP 409 Conflict when `dropNamespace` is
called on a non-empty namespace. Gravitino currently returns 400 Bad
Request, which is non-conformant.
Fix: #11132
### Does this PR introduce _any_ user-facing change?
Yes. Clients receive HTTP 409 Conflict instead of 400 Bad Request when
dropping a non-empty namespace. The error type remains
`NamespaceNotEmptyException`.
### How was this patch tested?
- `./gradlew :iceberg:iceberg-rest-server:test --tests
"org.apache.gravitino.iceberg.service.TestIcebergExceptionMapper"
-PskipITs`
- `./gradlew :iceberg:iceberg-rest-server:test --tests
"org.apache.gravitino.iceberg.service.rest.TestIcebergNamespaceOperations#testDropNamespace"
-PskipITs`
- `./gradlew :iceberg:iceberg-rest-server:test --tests
"org.apache.gravitino.iceberg.integration.test.IcebergRESTMemoryCatalogIT#testDropNameSpace"
-PskipITs`
---
.../iceberg/service/IcebergExceptionMapper.java | 5 ++--
.../service/TestIcebergExceptionMapper.java | 2 +-
.../rest/TestIcebergNamespaceOperations.java | 33 ++++++++++++++++++++++
3 files changed, 36 insertions(+), 4 deletions(-)
diff --git
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
index 203b91d760..daaf1db3aa 100644
---
a/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
+++
b/iceberg/iceberg-rest-server/src/main/java/org/apache/gravitino/iceberg/service/IcebergExceptionMapper.java
@@ -59,7 +59,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
.put(IllegalArgumentException.class, 400)
.put(ValidationException.class, 400)
.put(IllegalNameIdentifierException.class, 400)
- .put(NamespaceNotEmptyException.class, 400)
+ .put(NamespaceNotEmptyException.class, 409)
.put(NotAuthorizedException.class, 401)
.put(UnauthorizedException.class, 401)
.put(AuthenticationTimeoutException.class, 419)
@@ -111,8 +111,7 @@ public class IcebergExceptionMapper implements
ExceptionMapper<Exception> {
}
if (e instanceof IllegalArgumentException
|| e instanceof IllegalNameIdentifierException
- || e instanceof ValidationException
- || e instanceof NamespaceNotEmptyException) {
+ || e instanceof ValidationException) {
return new BadRequestException("%s", message);
}
if (EXCEPTION_ERROR_CODES.containsKey(e.getClass())) {
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
index dff3ce88ee..eef610f6b0 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/TestIcebergExceptionMapper.java
@@ -48,7 +48,7 @@ public class TestIcebergExceptionMapper {
public void testIcebergExceptionMapper() {
checkExceptionStatus(new IllegalArgumentException(""), 400);
checkExceptionStatus(new ValidationException(""), 400);
- checkExceptionStatus(new NamespaceNotEmptyException(""), 400);
+ checkExceptionStatus(new NamespaceNotEmptyException(""), 409);
checkExceptionStatus(new NotAuthorizedException(""), 401);
checkExceptionStatus(new TokenExpiredException("expired"), 419);
checkExceptionStatus(new AuthenticationTimeoutException("expired"), 419);
diff --git
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
index 3c83534a18..86e6b3e38d 100644
---
a/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
+++
b/iceberg/iceberg-rest-server/src/test/java/org/apache/gravitino/iceberg/service/rest/TestIcebergNamespaceOperations.java
@@ -48,10 +48,14 @@ import
org.apache.gravitino.listener.api.event.IcebergRegisterTablePreEvent;
import org.apache.gravitino.listener.api.event.IcebergUpdateNamespaceEvent;
import
org.apache.gravitino.listener.api.event.IcebergUpdateNamespaceFailureEvent;
import org.apache.gravitino.listener.api.event.IcebergUpdateNamespacePreEvent;
+import org.apache.iceberg.Schema;
import org.apache.iceberg.catalog.Namespace;
+import org.apache.iceberg.rest.requests.CreateTableRequest;
import org.apache.iceberg.rest.requests.ImmutableRegisterTableRequest;
import org.apache.iceberg.rest.requests.RegisterTableRequest;
import org.apache.iceberg.rest.responses.LoadTableResponse;
+import org.apache.iceberg.types.Types.NestedField;
+import org.apache.iceberg.types.Types.StringType;
import org.glassfish.jersey.internal.inject.AbstractBinder;
import org.glassfish.jersey.server.ResourceConfig;
import org.junit.jupiter.api.Assertions;
@@ -62,6 +66,9 @@ import org.mockito.Mockito;
public class TestIcebergNamespaceOperations extends IcebergNamespaceTestBase {
+ private static final Schema DROP_NONEMPTY_TABLE_SCHEMA =
+ new Schema(NestedField.required(1, "foo_string", StringType.get()));
+
private DummyEventListener dummyEventListener;
@Override
@@ -70,6 +77,7 @@ public class TestIcebergNamespaceOperations extends
IcebergNamespaceTestBase {
ResourceConfig resourceConfig =
IcebergRestTestUtil.getIcebergResourceConfig(
MockIcebergNamespaceOperations.class, true,
Arrays.asList(dummyEventListener));
+ resourceConfig.register(MockIcebergTableOperations.class);
// register a mock HttpServletRequest with user info
resourceConfig.register(
@@ -183,6 +191,31 @@ public class TestIcebergNamespaceOperations extends
IcebergNamespaceTestBase {
verifyCreateNamespaceSucc(Namespace.of("drop_foo3", "a"));
verifyDropNamespaceFail(404, Namespace.of("drop_foo3", "b"));
verifyDropNamespaceSucc(Namespace.of("drop_foo3", "a"));
+
+ // drop non-empty namespace should return 409 Conflict per Iceberg REST
spec
+ Namespace nonEmptyNs = Namespace.of("drop_nonempty_foo");
+ verifyCreateNamespaceSucc(nonEmptyNs);
+ verifyCreateTableSucc(nonEmptyNs, "drop_nonempty_table");
+ verifyDropNamespaceFail(409, nonEmptyNs);
+ verifyDropTableSucc(nonEmptyNs, "drop_nonempty_table");
+ verifyDropNamespaceSucc(nonEmptyNs);
+ }
+
+ private void verifyCreateTableSucc(Namespace ns, String tableName) {
+ CreateTableRequest createTableRequest =
+ CreateTableRequest.builder()
+ .withName(tableName)
+ .withSchema(DROP_NONEMPTY_TABLE_SCHEMA)
+ .build();
+ Response response =
+ getTableClientBuilder(ns, Optional.empty())
+ .post(Entity.entity(createTableRequest,
MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(Status.OK.getStatusCode(), response.getStatus());
+ }
+
+ private void verifyDropTableSucc(Namespace ns, String tableName) {
+ Response response = getTableClientBuilder(ns,
Optional.of(tableName)).delete();
+ Assertions.assertEquals(Status.NO_CONTENT.getStatusCode(),
response.getStatus());
}
@Test
