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 627230318 [#5105] improvement(server,client): Error code optimization
about access control API (#5144)
627230318 is described below
commit 627230318ad5d93d681d012509f5941093bb6ee2
Author: roryqi <[email protected]>
AuthorDate: Fri Oct 18 20:20:58 2024 +0800
[#5105] improvement(server,client): Error code optimization about access
control API (#5144)
### What changes were proposed in this pull request?
Error code optimization about access control API
### Why are the changes needed?
Fix: #5105
### Does this PR introduce _any_ user-facing change?
Yes.
### How was this patch tested?
Modify some UTs
---
.../exceptions/IllegalMetadataObjectException.java | 63 +++++++++++++++
.../gravitino/exceptions/IllegalRoleException.java | 62 +++++++++++++++
.../org/apache/gravitino/client/ErrorHandlers.java | 8 ++
.../apache/gravitino/client/GravitinoClient.java | 22 +++---
.../apache/gravitino/client/GravitinoMetalake.java | 18 +++--
.../test/authorization/AccessControlIT.java | 13 ++--
.../authorization/AccessControlDispatcher.java | 15 ++--
.../authorization/AccessControlManager.java | 9 ++-
.../gravitino/authorization/PermissionManager.java | 9 +++
.../hook/AccessControlHookDispatcher.java | 9 ++-
.../TestAccessControlManagerForPermissions.java | 19 ++---
docs/open-api/permissions.yaml | 90 ++++++++++++++++++++--
docs/open-api/roles.yaml | 19 ++++-
.../gravitino/server/web/rest/RoleOperations.java | 8 +-
.../server/web/rest/TestPermissionOperations.java | 56 +++++++++++---
.../server/web/rest/TestRoleOperations.java | 4 +-
16 files changed, 350 insertions(+), 74 deletions(-)
diff --git
a/api/src/main/java/org/apache/gravitino/exceptions/IllegalMetadataObjectException.java
b/api/src/main/java/org/apache/gravitino/exceptions/IllegalMetadataObjectException.java
new file mode 100644
index 000000000..7a955f268
--- /dev/null
+++
b/api/src/main/java/org/apache/gravitino/exceptions/IllegalMetadataObjectException.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
+
+/** An exception thrown when a metadata object is invalid. */
+public class IllegalMetadataObjectException extends IllegalArgumentException {
+ /**
+ * Constructs a new exception with the specified detail message.
+ *
+ * @param message the detail message.
+ * @param args the arguments to the message.
+ */
+ @FormatMethod
+ public IllegalMetadataObjectException(@FormatString String message,
Object... args) {
+ super(String.format(message, args));
+ }
+
+ /**
+ * Constructs a new exception with the specified detail message and cause.
+ *
+ * @param cause the cause.
+ * @param message the detail message.
+ * @param args the arguments to the message.
+ */
+ @FormatMethod
+ public IllegalMetadataObjectException(
+ Throwable cause, @FormatString String message, Object... args) {
+ super(String.format(message, args), cause);
+ }
+
+ /**
+ * Constructs a new exception with the specified cause.
+ *
+ * @param cause the cause.
+ */
+ public IllegalMetadataObjectException(Throwable cause) {
+ super(cause);
+ }
+
+ /** Constructs a new exception with the specified detail message and cause.
*/
+ public IllegalMetadataObjectException() {
+ super();
+ }
+}
diff --git
a/api/src/main/java/org/apache/gravitino/exceptions/IllegalRoleException.java
b/api/src/main/java/org/apache/gravitino/exceptions/IllegalRoleException.java
new file mode 100644
index 000000000..d5a81fe44
--- /dev/null
+++
b/api/src/main/java/org/apache/gravitino/exceptions/IllegalRoleException.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.exceptions;
+
+import com.google.errorprone.annotations.FormatMethod;
+import com.google.errorprone.annotations.FormatString;
+
+/** An exception thrown when a role is invalid. */
+public class IllegalRoleException extends IllegalArgumentException {
+ /**
+ * Constructs a new exception with the specified detail message.
+ *
+ * @param message the detail message.
+ * @param args the arguments to the message.
+ */
+ @FormatMethod
+ public IllegalRoleException(@FormatString String message, Object... args) {
+ super(String.format(message, args));
+ }
+
+ /**
+ * Constructs a new exception with the specified detail message and cause.
+ *
+ * @param cause the cause.
+ * @param message the detail message.
+ * @param args the arguments to the message.
+ */
+ @FormatMethod
+ public IllegalRoleException(Throwable cause, @FormatString String message,
Object... args) {
+ super(String.format(message, args), cause);
+ }
+
+ /**
+ * Constructs a new exception with the specified cause.
+ *
+ * @param cause the cause.
+ */
+ public IllegalRoleException(Throwable cause) {
+ super(cause);
+ }
+
+ /** Constructs a new exception with the specified detail message and cause.
*/
+ public IllegalRoleException() {
+ super();
+ }
+}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java
index a2ff07e27..db45b6436 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/ErrorHandlers.java
@@ -34,7 +34,9 @@ import
org.apache.gravitino.exceptions.ConnectionFailedException;
import org.apache.gravitino.exceptions.FilesetAlreadyExistsException;
import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.InUseException;
import org.apache.gravitino.exceptions.MetalakeAlreadyExistsException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
@@ -706,6 +708,10 @@ public class ErrorHandlers {
case ErrorConstants.ILLEGAL_ARGUMENTS_CODE:
if
(errorResponse.getType().equals(IllegalPrivilegeException.class.getSimpleName()))
{
throw new IllegalPrivilegeException(errorMessage);
+ } else if (errorResponse
+ .getType()
+ .equals(IllegalMetadataObjectException.class.getSimpleName())) {
+ throw new IllegalMetadataObjectException(errorMessage);
} else {
throw new IllegalArgumentException(errorMessage);
}
@@ -756,6 +762,8 @@ public class ErrorHandlers {
case ErrorConstants.ILLEGAL_ARGUMENTS_CODE:
if
(errorResponse.getType().equals(IllegalPrivilegeException.class.getSimpleName()))
{
throw new IllegalPrivilegeException(errorMessage);
+ } else if
(errorResponse.getType().equals(IllegalRoleException.class.getSimpleName())) {
+ throw new IllegalRoleException(errorMessage);
} else {
throw new IllegalArgumentException(errorMessage);
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
index 0f3b88133..c0310f238 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoClient.java
@@ -36,7 +36,9 @@ import org.apache.gravitino.authorization.User;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogInUseException;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
@@ -297,12 +299,12 @@ public class GravitinoClient extends GravitinoClientBase
* @return The created Role instance.
* @throws RoleAlreadyExistsException If a Role with the same name already
exists.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
- * @throws NoSuchMetadataObjectException If securable object doesn't exist
+ * @throws IllegalMetadataObjectException If securable object is invalid
* @throws RuntimeException If creating the Role encounters storage issues.
*/
public Role createRole(
String role, Map<String, String> properties, List<SecurableObject>
securableObjects)
- throws RoleAlreadyExistsException, NoSuchMetalakeException,
NoSuchMetadataObjectException {
+ throws RoleAlreadyExistsException, NoSuchMetalakeException,
IllegalMetadataObjectException {
return getMetalake().createRole(role, properties, securableObjects);
}
/**
@@ -312,12 +314,12 @@ public class GravitinoClient extends GravitinoClientBase
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchUserException If the User with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If granting roles to a user encounters storage
issues.
*/
public User grantRolesToUser(List<String> roles, String user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
return getMetalake().grantRolesToUser(roles, user);
}
@@ -328,12 +330,12 @@ public class GravitinoClient extends GravitinoClientBase
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchGroupException If the Group with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If granting roles to a group encounters storage
issues.
*/
public Group grantRolesToGroup(List<String> roles, String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
return getMetalake().grantRolesToGroup(roles, group);
}
@@ -344,12 +346,12 @@ public class GravitinoClient extends GravitinoClientBase
* @param roles The names of the Role.
* @return The User after revoked.
* @throws NoSuchUserException If the User with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If revoking roles from a user encounters storage
issues.
*/
public User revokeRolesFromUser(List<String> roles, String user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
return getMetalake().revokeRolesFromUser(roles, user);
}
@@ -360,12 +362,12 @@ public class GravitinoClient extends GravitinoClientBase
* @param roles The names of the Role.
* @return The Group after revoked.
* @throws NoSuchGroupException If the Group with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If revoking roles from a group encounters
storage issues.
*/
public Group revokeRolesFromGroup(List<String> roles, String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
return getMetalake().revokeRolesFromGroup(roles, group);
}
diff --git
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
index 441833bd4..47f42d3ad 100644
---
a/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
+++
b/clients/client-java/src/main/java/org/apache/gravitino/client/GravitinoMetalake.java
@@ -80,7 +80,9 @@ import org.apache.gravitino.dto.responses.UserResponse;
import org.apache.gravitino.exceptions.CatalogAlreadyExistsException;
import org.apache.gravitino.exceptions.CatalogInUseException;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
@@ -785,12 +787,12 @@ public class GravitinoMetalake extends MetalakeDTO
* @return The created Role instance.
* @throws RoleAlreadyExistsException If a Role with the same name already
exists.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
- * @throws NoSuchMetadataObjectException If the securable object doesn't
exist
+ * @throws IllegalMetadataObjectException If the securable object is invalid
* @throws RuntimeException If creating the Role encounters storage issues.
*/
public Role createRole(
String role, Map<String, String> properties, List<SecurableObject>
securableObjects)
- throws RoleAlreadyExistsException, NoSuchMetalakeException,
NoSuchMetadataObjectException {
+ throws RoleAlreadyExistsException, NoSuchMetalakeException,
IllegalMetadataObjectException {
RoleCreateRequest req =
new RoleCreateRequest(
role,
@@ -837,12 +839,12 @@ public class GravitinoMetalake extends MetalakeDTO
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchUserException If the User with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If granting roles to a user encounters storage
issues.
*/
public User grantRolesToUser(List<String> roles, String user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
RoleGrantRequest request = new RoleGrantRequest(roles);
request.validate();
@@ -868,7 +870,7 @@ public class GravitinoMetalake extends MetalakeDTO
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchGroupException If the Group with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If granting roles to a group encounters storage
issues.
*/
@@ -899,7 +901,7 @@ public class GravitinoMetalake extends MetalakeDTO
* @param roles The names of the Role.
* @return The User after revoked.
* @throws NoSuchUserException If the User with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If revoking roles from a user encounters storage
issues.
*/
@@ -930,12 +932,12 @@ public class GravitinoMetalake extends MetalakeDTO
* @param roles The names of the Role.
* @return The Group after revoked.
* @throws NoSuchGroupException If the Group with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name is invalid.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If revoking roles from a group encounters
storage issues.
*/
public Group revokeRolesFromGroup(List<String> roles, String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
RoleRevokeRequest request = new RoleRevokeRequest(roles);
request.validate();
diff --git
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
index 685f46597..78c294334 100644
---
a/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
+++
b/clients/client-java/src/test/java/org/apache/gravitino/client/integration/test/authorization/AccessControlIT.java
@@ -42,9 +42,10 @@ import org.apache.gravitino.authorization.SecurableObjects;
import org.apache.gravitino.authorization.User;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
-import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.exceptions.UserAlreadyExistsException;
@@ -214,7 +215,7 @@ public class AccessControlIT extends BaseIT {
"not-existed", Lists.newArrayList(Privileges.UseCatalog.allow()));
Assertions.assertThrows(
- NoSuchMetadataObjectException.class,
+ IllegalMetadataObjectException.class,
() -> metalake.createRole("not-existed", properties,
Lists.newArrayList(catalogObject)));
// Create a role with duplicated securable objects
@@ -359,12 +360,12 @@ public class AccessControlIT extends BaseIT {
// Grant a not-existed role
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() -> metalake.grantRolesToUser(Lists.newArrayList("not-existed"),
username));
// Revoke a not-existed role
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() -> metalake.revokeRolesFromUser(Lists.newArrayList("not-existed"),
username));
// Grant to a not-existed user
@@ -414,12 +415,12 @@ public class AccessControlIT extends BaseIT {
// Grant a not-existed role
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() -> metalake.grantRolesToGroup(Lists.newArrayList("not-existed"),
groupName));
// Revoke a not-existed role
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() -> metalake.revokeRolesFromGroup(Lists.newArrayList("not-existed"),
groupName));
// Grant to a not-existed group
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
index 73004280b..f5625d9d6 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlDispatcher.java
@@ -22,6 +22,7 @@ import java.util.List;
import java.util.Map;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -155,12 +156,12 @@ public interface AccessControlDispatcher {
* @param roles The names of the Role.
* @return The User after granted.
* @throws NoSuchUserException If the User with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name does not
exist.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If granting roles to a user encounters storage
issues.
*/
User grantRolesToUser(String metalake, List<String> roles, String user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException;
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException;
/**
* Grant roles to a group.
@@ -170,12 +171,12 @@ public interface AccessControlDispatcher {
* @param roles The names of the Role.
* @return The Group after granted.
* @throws NoSuchGroupException If the Group with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name does not
exist.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If granting roles to a group encounters storage
issues.
*/
Group grantRolesToGroup(String metalake, List<String> roles, String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException;
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException;
/**
* Revoke roles from a group.
@@ -185,12 +186,12 @@ public interface AccessControlDispatcher {
* @param roles The name of the Role.
* @return The Group after revoked.
* @throws NoSuchGroupException If the Group with the given name does not
exist.
- * @throws NoSuchRoleException If the Role with the given name does not
exist.
+ * @throws IllegalRoleException If the Role with the given name does not
exist.
* @throws NoSuchMetalakeException If the Metalake with the given name does
not exist.
* @throws RuntimeException If revoking roles from a group encounters
storage issues.
*/
Group revokeRolesFromGroup(String metalake, List<String> roles, String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException;
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException;
/**
* Revoke roles from a user.
@@ -205,7 +206,7 @@ public interface AccessControlDispatcher {
* @throws RuntimeException If revoking roles from a user encounters storage
issues.
*/
User revokeRolesFromUser(String metalake, List<String> roles, String user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException;
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException;
/**
* Judges whether the user is the service admin.
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
index c9adf314a..798285806 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/AccessControlManager.java
@@ -25,6 +25,7 @@ import org.apache.gravitino.Configs;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.MetadataObject;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -107,25 +108,25 @@ public class AccessControlManager implements
AccessControlDispatcher {
@Override
public User grantRolesToUser(String metalake, List<String> roles, String
user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
return permissionManager.grantRolesToUser(metalake, roles, user);
}
@Override
public Group grantRolesToGroup(String metalake, List<String> roles, String
group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
return permissionManager.grantRolesToGroup(metalake, roles, group);
}
@Override
public Group revokeRolesFromGroup(String metalake, List<String> roles,
String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
return permissionManager.revokeRolesFromGroup(metalake, roles, group);
}
@Override
public User revokeRolesFromUser(String metalake, List<String> roles, String
user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
return permissionManager.revokeRolesFromUser(metalake, roles, user);
}
diff --git
a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
index 056b18f40..02c240f30 100644
---
a/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
+++
b/core/src/main/java/org/apache/gravitino/authorization/PermissionManager.java
@@ -33,6 +33,7 @@ import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.Entity;
import org.apache.gravitino.EntityStore;
import org.apache.gravitino.MetadataObject;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchEntityException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
@@ -129,6 +130,8 @@ class PermissionManager {
} catch (NoSuchEntityException nse) {
LOG.warn("Failed to grant, user {} does not exist in the metalake {}",
user, metalake, nse);
throw new NoSuchUserException(USER_DOES_NOT_EXIST_MSG, user, metalake);
+ } catch (NoSuchRoleException nsr) {
+ throw new IllegalRoleException(nsr);
} catch (IOException ioe) {
LOG.error(
"Failed to grant role {} to user {} in the metalake {} due to
storage issues",
@@ -208,6 +211,8 @@ class PermissionManager {
} catch (NoSuchEntityException nse) {
LOG.warn("Failed to grant, group {} does not exist in the metalake {}",
group, metalake, nse);
throw new NoSuchGroupException(GROUP_DOES_NOT_EXIST_MSG, group,
metalake);
+ } catch (NoSuchRoleException nsr) {
+ throw new IllegalRoleException(nsr);
} catch (IOException ioe) {
LOG.error(
"Failed to grant role {} to group {} in the metalake {} due to
storage issues",
@@ -288,6 +293,8 @@ class PermissionManager {
LOG.warn(
"Failed to revoke, group {} does not exist in the metalake {}",
group, metalake, nse);
throw new NoSuchGroupException(GROUP_DOES_NOT_EXIST_MSG, group,
metalake);
+ } catch (NoSuchRoleException nsr) {
+ throw new IllegalRoleException(nsr);
} catch (IOException ioe) {
LOG.error(
"Failed to revoke role {} from group {} in the metalake {} due to
storage issues",
@@ -366,6 +373,8 @@ class PermissionManager {
} catch (NoSuchEntityException nse) {
LOG.warn("Failed to revoke, user {} does not exist in the metalake {}",
user, metalake, nse);
throw new NoSuchUserException(USER_DOES_NOT_EXIST_MSG, user, metalake);
+ } catch (NoSuchRoleException nsr) {
+ throw new IllegalRoleException(nsr);
} catch (IOException ioe) {
LOG.error(
"Failed to revoke role {} from user {} in the metalake {} due to
storage issues",
diff --git
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
index 125df0b2e..f5f5a2764 100644
---
a/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
+++
b/core/src/main/java/org/apache/gravitino/hook/AccessControlHookDispatcher.java
@@ -33,6 +33,7 @@ import org.apache.gravitino.authorization.Role;
import org.apache.gravitino.authorization.SecurableObject;
import org.apache.gravitino.authorization.User;
import org.apache.gravitino.exceptions.GroupAlreadyExistsException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
@@ -111,25 +112,25 @@ public class AccessControlHookDispatcher implements
AccessControlDispatcher {
@Override
public User grantRolesToUser(String metalake, List<String> roles, String
user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
return dispatcher.grantRolesToUser(metalake, roles, user);
}
@Override
public Group grantRolesToGroup(String metalake, List<String> roles, String
group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
return dispatcher.grantRolesToGroup(metalake, roles, group);
}
@Override
public Group revokeRolesFromGroup(String metalake, List<String> roles,
String group)
- throws NoSuchGroupException, NoSuchRoleException,
NoSuchMetalakeException {
+ throws NoSuchGroupException, IllegalRoleException,
NoSuchMetalakeException {
return dispatcher.revokeRolesFromGroup(metalake, roles, group);
}
@Override
public User revokeRolesFromUser(String metalake, List<String> roles, String
user)
- throws NoSuchUserException, NoSuchRoleException, NoSuchMetalakeException
{
+ throws NoSuchUserException, IllegalRoleException,
NoSuchMetalakeException {
return dispatcher.revokeRolesFromUser(metalake, roles, user);
}
diff --git
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
index e7e792536..9387fef0d 100644
---
a/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
+++
b/core/src/test/java/org/apache/gravitino/authorization/TestAccessControlManagerForPermissions.java
@@ -40,6 +40,7 @@ import org.apache.gravitino.Namespace;
import org.apache.gravitino.catalog.CatalogManager;
import org.apache.gravitino.connector.BaseCatalog;
import org.apache.gravitino.connector.authorization.AuthorizationPlugin;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchGroupException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.exceptions.NoSuchRoleException;
@@ -215,9 +216,9 @@ public class TestAccessControlManagerForPermissions {
NoSuchMetalakeException.class,
() -> accessControlManager.grantRolesToUser(notExist, ROLE, USER));
- // Throw NoSuchRoleException
+ // Throw IllegalRoleException
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() -> accessControlManager.grantRolesToUser(METALAKE,
Lists.newArrayList(notExist), USER));
// Throw NoSuchUserException
@@ -249,9 +250,9 @@ public class TestAccessControlManagerForPermissions {
NoSuchMetalakeException.class,
() -> accessControlManager.revokeRolesFromUser(notExist, ROLE, USER));
- // Throw NoSuchRoleException
+ // Throw IllegalRoleException
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() ->
accessControlManager.revokeRolesFromUser(METALAKE,
Lists.newArrayList(notExist), USER));
@@ -293,9 +294,9 @@ public class TestAccessControlManagerForPermissions {
NoSuchMetalakeException.class,
() -> accessControlManager.grantRolesToGroup(notExist, ROLE, GROUP));
- // Throw NoSuchRoleException
+ // Throw IllegalRoleException
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() ->
accessControlManager.grantRolesToGroup(METALAKE,
Lists.newArrayList(notExist), GROUP));
@@ -328,9 +329,9 @@ public class TestAccessControlManagerForPermissions {
NoSuchMetalakeException.class,
() -> accessControlManager.revokeRolesFromGroup(notExist, ROLE,
GROUP));
- // Throw NoSuchRoleException
+ // Throw IllegalRoleException
Assertions.assertThrows(
- NoSuchRoleException.class,
+ IllegalRoleException.class,
() ->
accessControlManager.revokeRolesFromGroup(
METALAKE, Lists.newArrayList(notExist), GROUP));
@@ -375,7 +376,7 @@ public class TestAccessControlManagerForPermissions {
Assertions.assertEquals(2, objects.size());
- // Throw NoSuchRoleException
+ // Throw IllegalRoleException
Assertions.assertThrows(
NoSuchRoleException.class,
() ->
diff --git a/docs/open-api/permissions.yaml b/docs/open-api/permissions.yaml
index 1a19a9e2b..0da45d9ca 100644
--- a/docs/open-api/permissions.yaml
+++ b/docs/open-api/permissions.yaml
@@ -49,6 +49,16 @@ paths:
UserResponse:
$ref: "./users.yaml#/components/examples/UserResponse"
+ "400":
+ description: Parameter is invalid - The specified role is invalid in
the metalake
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ IllegalRoleException:
+ $ref: "#/components/examples/IllegalRoleException"
+
"404":
description: Not Found - The specified user or role does not exist
in the specified metalake
content:
@@ -60,8 +70,6 @@ paths:
$ref:
"./metalakes.yaml#/components/examples/NoSuchMetalakeException"
NoSuchUserException:
$ref: "./users.yaml#/components/examples/NoSuchUserException"
- NoSuchRoleException:
- $ref: "./roles.yaml#/components/examples/NoSuchRoleException"
"5xx":
$ref: "./openapi.yaml#/components/responses/ServerErrorResponse"
@@ -96,6 +104,16 @@ paths:
UserResponse:
$ref: "./users.yaml#/components/examples/UserResponse"
+ "400":
+ description: Parameter is invalid - The specified role is invalid in
the metalake
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ IllegalRoleException:
+ $ref: "#/components/examples/IllegalRoleException"
+
"404":
description: Not Found - The specified user or role does not exist
in the specified metalake
content:
@@ -107,8 +125,6 @@ paths:
$ref:
"./metalakes.yaml#/components/examples/NoSuchMetalakeException"
NoSuchUserException:
$ref: "./users.yaml#/components/examples/NoSuchUserException"
- NoSuchRoleException:
- $ref: "./roles.yaml#/components/examples/NoSuchRoleException"
"5xx":
$ref: "./openapi.yaml#/components/responses/ServerErrorResponse"
@@ -143,6 +159,16 @@ paths:
GroupResponse:
$ref: "./groups.yaml#/components/examples/GroupResponse"
+ "400":
+ description: Parameter is invalid - The specified role is invalid in
the metalake
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ IllegalRoleException:
+ $ref: "#/components/examples/IllegalRoleException"
+
"404":
description: Not Found - The specified group or role does not exist
in the specified metalake
content:
@@ -154,8 +180,6 @@ paths:
$ref:
"./metalakes.yaml#/components/examples/NoSuchMetalakeException"
NoSuchGroupException:
$ref:
"./groups.yaml#/components/examples/NoSuchGroupException"
- NoSuchRoleException:
- $ref: "./roles.yaml#/components/examples/NoSuchRoleException"
"5xx":
$ref: "./openapi.yaml#/components/responses/ServerErrorResponse"
@@ -190,6 +214,16 @@ paths:
GroupResponse:
$ref: "./groups.yaml#/components/examples/GroupResponse"
+ "400":
+ description: Parameter is invalid - The specified role is invalid in
the metalake
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ IllegalRoleException:
+ $ref: "#/components/examples/IllegalRoleException"
+
"404":
description: Not Found - The specified group or role does not exist
in the specified metalake
content:
@@ -201,8 +235,6 @@ paths:
$ref:
"./metalakes.yaml#/components/examples/NoSuchMetalakeException"
NoSuchGroupException:
$ref:
"./groups.yaml#/components/examples/NoSuchGroupException"
- NoSuchRoleException:
- $ref: "./roles.yaml#/components/examples/NoSuchRoleException"
"5xx":
$ref: "./openapi.yaml#/components/responses/ServerErrorResponse"
@@ -239,6 +271,16 @@ paths:
GroupResponse:
$ref: "./roles.yaml#/components/examples/RoleResponse"
+ "400":
+ description: Parameter is invalid - The specified privilege is
invalid
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ IllegalPrivilegeException:
+ $ref: "#/components/examples/IllegalPrivilegeException"
+
"404":
description: Not Found - The specified medata object or role does
not exist in the specified metalake
content:
@@ -288,6 +330,16 @@ paths:
GroupResponse:
$ref: "./roles.yaml#/components/examples/RoleResponse"
+ "400":
+ description: Parameter is invalid - The specified privilege is
invalid
+ content:
+ application/vnd.gravitino.v1+json:
+ schema:
+ $ref: "./openapi.yaml#/components/schemas/ErrorModel"
+ examples:
+ IllegalPrivilegeException:
+ $ref: "#/components/examples/IllegalPrivilegeException"
+
"404":
description: Not Found - The specified medata object or role does
not exist in the specified metalake
content:
@@ -381,4 +433,26 @@ components:
"name": "SELECT_TABLE",
"condition": "ALLOW"
} ]
+ }
+
+ IllegalRoleException:
+ value: {
+ "code": 1001,
+ "type": "IllegalRoleException",
+ "message": "Role role1 does not exist",
+ "stack": [
+ "org.apache.gravitino.exceptions.IllegalRoleException: Role role1
does not exist",
+ "..."
+ ]
+ }
+
+ IllegalPrivilegeException:
+ value: {
+ "code": 1001,
+ "type": "IllegalPrivilegeException",
+ "message": "Doesn't support duplicated privilege name SELECT_TABLE
with different condition",
+ "stack": [
+ "org.apache.gravitino.exceptions.IllegalPrivilegeException: Doesn't
support duplicated privilege name SELECT_TABLE with different condition",
+ "..."
+ ]
}
\ No newline at end of file
diff --git a/docs/open-api/roles.yaml b/docs/open-api/roles.yaml
index 8bc452a20..986d0fdc6 100644
--- a/docs/open-api/roles.yaml
+++ b/docs/open-api/roles.yaml
@@ -75,15 +75,15 @@ paths:
RoleResponse:
$ref: "#/components/examples/RoleResponse"
- "404":
- description: Not Found - The specified securable object does not
exist in the specified metalake
+ "400":
+ description: Parameter is invalid - The specified securable object
is invalid the specified metalake
content:
application/vnd.gravitino.v1+json:
schema:
$ref: "./openapi.yaml#/components/schemas/ErrorModel"
examples:
NoSuchMetadataObjectException:
- $ref: "#/components/examples/NoSuchMetadataObjectException"
+ $ref: "#/components/examples/IllegalMetadataObjectException"
"409":
description: Conflict - The target role already exists in the
specified metalake
@@ -360,13 +360,24 @@ components:
]
}
+ IllegalMetadataObjectException:
+ value: {
+ "code": 1001,
+ "type": "IllegalMetadataObjectException",
+ "message": "Metadata object does not exist",
+ "stack": [
+ "org.apache.gravitino.exceptions.IllegalMetadataObjectException:
Metadata object does not exist",
+ "..."
+ ]
+ }
+
NoSuchMetadataObjectException:
value: {
"code": 1003,
"type": "NoSuchMetadataObjectException",
"message": "Metadata object does not exist",
"stack": [
- "org.apache.gravitino.exceptions.NoSuchUserException: Metadata
object does not exist",
+ "org.apache.gravitino.exceptions.NoSuchMetadataObjectException:
Metadata object does not exist",
"..."
]
}
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 91ebaf5b4..e986753d0 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
@@ -50,6 +50,8 @@ import org.apache.gravitino.dto.responses.DeleteResponse;
import org.apache.gravitino.dto.responses.NameListResponse;
import org.apache.gravitino.dto.responses.RoleResponse;
import org.apache.gravitino.dto.util.DTOConverters;
+import org.apache.gravitino.exceptions.IllegalMetadataObjectException;
+import org.apache.gravitino.exceptions.NoSuchMetadataObjectException;
import org.apache.gravitino.lock.LockType;
import org.apache.gravitino.lock.TreeLockUtils;
import org.apache.gravitino.metrics.MetricNames;
@@ -143,7 +145,11 @@ public class RoleOperations {
for (Privilege privilege : object.privileges()) {
AuthorizationUtils.checkPrivilege((PrivilegeDTO) privilege,
object, metalake);
}
- MetadataObjectUtil.checkMetadataObject(metalake, object);
+ try {
+ MetadataObjectUtil.checkMetadataObject(metalake, object);
+ } catch (NoSuchMetadataObjectException nsm) {
+ throw new IllegalMetadataObjectException(nsm);
+ }
}
List<SecurableObject> securableObjects =
diff --git
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
index e927a0a4e..8876e9035 100644
---
a/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
+++
b/server/src/test/java/org/apache/gravitino/server/web/rest/TestPermissionOperations.java
@@ -55,8 +55,8 @@ import org.apache.gravitino.dto.responses.GroupResponse;
import org.apache.gravitino.dto.responses.RoleResponse;
import org.apache.gravitino.dto.responses.UserResponse;
import org.apache.gravitino.exceptions.IllegalPrivilegeException;
+import org.apache.gravitino.exceptions.IllegalRoleException;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
-import org.apache.gravitino.exceptions.NoSuchRoleException;
import org.apache.gravitino.exceptions.NoSuchUserException;
import org.apache.gravitino.lock.LockManager;
import org.apache.gravitino.meta.AuditInfo;
@@ -186,8 +186,8 @@ public class TestPermissionOperations extends JerseyTest {
Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
errorResponse.getCode());
Assertions.assertEquals(NoSuchUserException.class.getSimpleName(),
errorResponse.getType());
- // Test to throw NoSuchRoleException
- doThrow(new NoSuchRoleException("mock error"))
+ // Test to throw IllegalRoleException
+ doThrow(new IllegalRoleException("mock error"))
.when(manager)
.grantRolesToUser(any(), any(), any());
resp1 =
@@ -196,12 +196,12 @@ public class TestPermissionOperations extends JerseyTest {
.accept("application/vnd.gravitino.v1+json")
.put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
- Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
resp1.getStatus());
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp1.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp1.getMediaType());
errorResponse = resp1.readEntity(ErrorResponse.class);
- Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
errorResponse.getCode());
- Assertions.assertEquals(NoSuchRoleException.class.getSimpleName(),
errorResponse.getType());
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(IllegalRoleException.class.getSimpleName(),
errorResponse.getType());
// Test to throw internal RuntimeException
doThrow(new RuntimeException("mock
error")).when(manager).grantRolesToUser(any(), any(), any());
@@ -284,8 +284,8 @@ public class TestPermissionOperations extends JerseyTest {
Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
errorResponse.getCode());
Assertions.assertEquals(NoSuchUserException.class.getSimpleName(),
errorResponse.getType());
- // Test to throw NoSuchRoleException
- doThrow(new NoSuchRoleException("mock error"))
+ // Test to throw IllegalRoleException
+ doThrow(new IllegalRoleException("mock error"))
.when(manager)
.grantRolesToGroup(any(), any(), any());
resp1 =
@@ -294,12 +294,12 @@ public class TestPermissionOperations extends JerseyTest {
.accept("application/vnd.gravitino.v1+json")
.put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
- Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
resp1.getStatus());
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
resp1.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
resp1.getMediaType());
errorResponse = resp1.readEntity(ErrorResponse.class);
- Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
errorResponse.getCode());
- Assertions.assertEquals(NoSuchRoleException.class.getSimpleName(),
errorResponse.getType());
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(IllegalRoleException.class.getSimpleName(),
errorResponse.getType());
// Test to throw internal RuntimeException
doThrow(new RuntimeException("mock error"))
@@ -362,6 +362,23 @@ public class TestPermissionOperations extends JerseyTest {
ErrorResponse errorResponse = resp3.readEntity(ErrorResponse.class);
Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse.getCode());
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse.getType());
+
+ // Test to throw IllegalRoleException
+ doThrow(new IllegalRoleException("mock error"))
+ .when(manager)
+ .revokeRolesFromUser(any(), any(), any());
+ Response nsrResponse =
+ target("/metalakes/metalake1/permissions/users/user/revoke")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
nsrResponse.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
nsrResponse.getMediaType());
+
+ errorResponse = nsrResponse.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(IllegalRoleException.class.getSimpleName(),
errorResponse.getType());
}
@Test
@@ -407,6 +424,23 @@ public class TestPermissionOperations extends JerseyTest {
ErrorResponse errorResponse = resp3.readEntity(ErrorResponse.class);
Assertions.assertEquals(ErrorConstants.INTERNAL_ERROR_CODE,
errorResponse.getCode());
Assertions.assertEquals(RuntimeException.class.getSimpleName(),
errorResponse.getType());
+
+ // Test to throw IllegalRoleException
+ doThrow(new IllegalRoleException("mock error"))
+ .when(manager)
+ .revokeRolesFromGroup(any(), any(), any());
+ Response nsrResponse =
+ target("/metalakes/metalake1/permissions/groups/group/revoke")
+ .request(MediaType.APPLICATION_JSON_TYPE)
+ .accept("application/vnd.gravitino.v1+json")
+ .put(Entity.entity(request, MediaType.APPLICATION_JSON_TYPE));
+
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
nsrResponse.getStatus());
+ Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
nsrResponse.getMediaType());
+
+ errorResponse = nsrResponse.readEntity(ErrorResponse.class);
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
errorResponse.getCode());
+ Assertions.assertEquals(IllegalRoleException.class.getSimpleName(),
errorResponse.getType());
}
@Test
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 55fa7dd3a..5a53ec5f9 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
@@ -206,10 +206,10 @@ public class TestRoleOperations extends JerseyTest {
.request(MediaType.APPLICATION_JSON_TYPE)
.accept("application/vnd.gravitino.v1+json")
.post(Entity.entity(req, MediaType.APPLICATION_JSON_TYPE));
- Assertions.assertEquals(Response.Status.NOT_FOUND.getStatusCode(),
respNotExist.getStatus());
+ Assertions.assertEquals(Response.Status.BAD_REQUEST.getStatusCode(),
respNotExist.getStatus());
Assertions.assertEquals(MediaType.APPLICATION_JSON_TYPE,
respNotExist.getMediaType());
ErrorResponse notExistResponse =
respNotExist.readEntity(ErrorResponse.class);
- Assertions.assertEquals(ErrorConstants.NOT_FOUND_CODE,
notExistResponse.getCode());
+ Assertions.assertEquals(ErrorConstants.ILLEGAL_ARGUMENTS_CODE,
notExistResponse.getCode());
// Test to throw NoSuchMetalakeException
when(catalogDispatcher.catalogExists(any())).thenReturn(true);
