This is an automated email from the ASF dual-hosted git repository.
jmclean 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 4a411c86f6 [#9081] fix(catalog): use correct operation type in catalog
update error handler in CatalogOperations.java (#9092)
4a411c86f6 is described below
commit 4a411c86f655c61be4c65edfcf6cf4b812a9a856
Author: Emmanuel Valentin <[email protected]>
AuthorDate: Wed Nov 12 17:54:17 2025 -0600
[#9081] fix(catalog): use correct operation type in catalog update error
handler in CatalogOperations.java (#9092)
### What changes were proposed in this pull request?
This PR fixes the catalog update error handler to use the correct
operation type from the incoming request instead of hardcoding or
inferring the operation type. Previously, failed disable operations were
incorrectly reported as enable operations in error messages.
Changes:
- Modified the error handler in catalog operations to preserve the
original operation type from the request
- Updated tests in `TestCatalogOperations.java` to verify correct error
reporting for both enable and disable operations
### Why are the changes needed?
When a catalog disable operation fails, the error message incorrectly
reports it as an enable operation failure. This misleads users and makes
debugging difficult.
Fix: #9081
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
1. Added test cases to ensure both enable and disable operation failures
report correctly.
---
.../server/web/rest/CatalogOperations.java | 8 +++--
.../server/web/rest/TestCatalogOperations.java | 34 ++++++++++++++++++++++
2 files changed, 39 insertions(+), 3 deletions(-)
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
index 093e17eff6..a12fc99ba5 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/CatalogOperations.java
@@ -216,12 +216,15 @@ public class CatalogOperations {
String catalogName,
CatalogSetRequest request) {
LOG.info("Received set request for catalog: {}.{}", metalake, catalogName);
+
+ OperationType op = request.isInUse() ? OperationType.ENABLE :
OperationType.DISABLE;
+
try {
return Utils.doAs(
httpRequest,
() -> {
NameIdentifier ident = NameIdentifierUtil.ofCatalog(metalake,
catalogName);
- if (request.isInUse()) {
+ if (op == OperationType.ENABLE) {
catalogDispatcher.enableCatalog(ident);
} else {
catalogDispatcher.disableCatalog(ident);
@@ -242,8 +245,7 @@ public class CatalogOperations {
request.isInUse() ? "enable" : "disable",
metalake,
catalogName);
- return ExceptionHandlers.handleCatalogException(
- OperationType.ENABLE, catalogName, metalake, e);
+ return ExceptionHandlers.handleCatalogException(op, catalogName,
metalake, e);
}
}
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
index b902d7c68e..fbf462ba25 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestCatalogOperations.java
@@ -520,6 +520,7 @@ public class TestCatalogOperations extends
BaseOperationsTest {
@Test
public void testSetCatalog() {
+ // Test enable catalog
CatalogSetRequest req = new CatalogSetRequest(true);
doNothing().when(manager).enableCatalog(any());
@@ -534,6 +535,7 @@ public class TestCatalogOperations extends
BaseOperationsTest {
BaseResponse baseResponse = resp.readEntity(BaseResponse.class);
Assertions.assertEquals(0, baseResponse.getCode());
+ // Test disable catalog
req = new CatalogSetRequest(false);
doNothing().when(manager).disableCatalog(any());
@@ -547,6 +549,38 @@ public class TestCatalogOperations extends
BaseOperationsTest {
Assertions.assertEquals(Response.Status.OK.getStatusCode(),
resp.getStatus());
baseResponse = resp.readEntity(BaseResponse.class);
Assertions.assertEquals(0, baseResponse.getCode());
+
+ // Test throw NoSuchCatalogException when enabling catalog
+ req = new CatalogSetRequest(true);
+ doThrow(new RuntimeException("mock
error")).when(manager).enableCatalog(any());
+
+ resp =
+ target("/metalakes/metalake1/catalogs/catalog1")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method("PATCH", Entity.entity(req,
MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
+ ErrorResponse errorResponse = resp.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse.getType());
+
+ // Test throw NoSuchCatalogException when disabling catalog
+ req = new CatalogSetRequest(false);
+ doThrow(new RuntimeException("mock
error")).when(manager).disableCatalog(any());
+
+ resp =
+ target("/metalakes/metalake1/catalogs/catalog1")
+ .property(HttpUrlConnectorProvider.SET_METHOD_WORKAROUND, true)
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .method("PATCH", Entity.entity(req,
MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(INTERNAL_SERVER_ERROR.getStatusCode(),
resp.getStatus());
+ ErrorResponse errorResponse1 = resp.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse1.getCode());
+ Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse1.getType());
}
private static TestCatalog buildCatalog(String metalake, String catalogName)
{
