This is an automated email from the ASF dual-hosted git repository.
jshao 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 f893b5f90 [#6082] fix: Fix error code of creating role operation
(#6085)
f893b5f90 is described below
commit f893b5f903c3bd9616bb2be2e849716e9054fe83
Author: roryqi <[email protected]>
AuthorDate: Fri Jan 3 16:39:22 2025 +0800
[#6082] fix: Fix error code of creating role operation (#6085)
### What changes were proposed in this pull request?
We should return 400, if the role contains an error metalake metadata
object.
We should return 400, if the catalog doesn't exist.
### Why are the changes needed?
Fix: #6082
### Does this PR introduce _any_ user-facing change?
No.
### How was this patch tested?
Added UT.
---
.../apache/gravitino/utils/MetadataObjectUtil.java | 4 ++++
.../gravitino/server/web/rest/RoleOperations.java | 6 ++---
.../server/web/rest/TestRoleOperations.java | 26 +++++++++++++++++++++-
3 files changed, 32 insertions(+), 4 deletions(-)
diff --git
a/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
b/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
index 44b9d30a0..eb963182b 100644
--- a/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
+++ b/core/src/main/java/org/apache/gravitino/utils/MetadataObjectUtil.java
@@ -32,6 +32,7 @@ import org.apache.gravitino.GravitinoEnv;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.authorization.AuthorizationUtils;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
@@ -125,6 +126,9 @@ public class MetadataObjectUtil {
switch (object.type()) {
case METALAKE:
+ if (!metalake.equals(object.name())) {
+ throw new IllegalMetadataObjectException("The metalake object name
must be %s", metalake);
+ }
NameIdentifierUtil.checkMetalake(identifier);
check(env.metalakeDispatcher().metalakeExists(identifier),
exceptionToThrowSupplier);
break;
diff --git
a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
index e986753d0..bf82d78b6 100644
---
a/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
+++
b/server/src/main/java/org/apache/gravitino/server/web/rest/RoleOperations.java
@@ -142,10 +142,10 @@ public class RoleOperations {
Set<Privilege> privileges = Sets.newHashSet(object.privileges());
AuthorizationUtils.checkDuplicatedNamePrivilege(privileges);
- for (Privilege privilege : object.privileges()) {
- AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege,
object, metalake);
- }
try {
+ for (Privilege privilege : object.privileges()) {
+ AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege,
object, metalake);
+ }
MetadataObjectUtil.checkMetadataObject(metalake, object);
} catch (NoSuchMetadataObjectException nsm) {
throw new IllegalMetadataObjectException(nsm);
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
index 5a53ec5f9..6edd33393 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestRoleOperations.java
@@ -63,6 +63,7 @@ import org.apache.gravitino.dto.responses.RoleResponse;
import org.apache.gravitino.dto.util.DTOConverters;
import org.apache.gravitino.exceptions.IllegalNamespaceException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
+import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
@@ -199,8 +200,31 @@ public class TestRoleOperations extends JerseyTest {
Privileges.UseCatalog.allow().condition(),
roleDTO.securableObjects().get(0).privileges().get(0).condition());
+ // Test with a wrong metalake name
+ RoleCreateRequest reqWithWrongMetalake =
+ new RoleCreateRequest(
+ "role",
+ Collections.emptyMap(),
+ new SecurableObjectDTO[] {
+ DTOConverters.toDTO(
+ SecurableObjects.ofMetalake(
+ "unknown",
Lists.newArrayList(Privileges.UseCatalog.allow()))),
+ });
+ Response respWithWrongMetalake =
+ target("/metalakes/metalake1/roles")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .post(Entity.entity(reqWithWrongMetalake,
MediaType.APPLICATION_JSON_TYPE));
+ Assertions.assertEquals(
+ Response.Status.BAD_REQUEST.getStatusCode(),
respWithWrongMetalake.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
respWithWrongMetalake.getMediaType());
+ ErrorResponse withWrongMetalakeResponse =
respWithWrongMetalake.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(
+ ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
withWrongMetalakeResponse.getCode());
+
// Test to a catalog which doesn't exist
- when(catalogDispatcher.catalogExists(any())).thenReturn(false);
+ reset(catalogDispatcher);
+ when(catalogDispatcher.loadCatalog(any())).thenThrow(new
NoSuchCatalogException("mock error"));
Response respNotExist =
target("/metalakes/metalake1/roles")
.request(MediaType.APPLICATION_JSON_TYPE)
