xunliu commented on code in PR #3946:
URL: https://github.com/apache/gravitino/pull/3946#discussion_r1664000177


##########
core/src/main/java/com/datastrato/gravitino/authorization/AuthorizationRoleHook.java:
##########
@@ -0,0 +1,48 @@
+/*
+ * Copyright 2024 Datastrato Pvt Ltd.
+ * This software is licensed under the Apache License version 2.
+ */
+package com.datastrato.gravitino.authorization;
+
+import java.util.List;
+
+/** Interface for authorization Role hooks operation of the underlying access 
control system */
+public interface AuthorizationRoleHook {
+  /**
+   * Creates a new Role into underlying access control system.
+   *
+   * @param role The entity of the Role.
+   * @param securableObjects The securable objects of the Role.
+   * @return True if the create operation success; False if the create 
operation failed.
+   * @throws RuntimeException If creating the Role encounters storage issues.
+   */
+  Boolean onCreateRole(Role role, List<SecurableObject> securableObjects) 
throws RuntimeException;
+
+  /**
+   * Gets a Role from underlying access control system.
+   *
+   * @param role The name of the Role.
+   * @return The getting Role instance.
+   * @throws RuntimeException If getting the Role encounters storage issues.
+   */
+  Role onGetRole(String role) throws RuntimeException;

Review Comment:
   `onGetXXX` functions use to determine whether the specified Role/User/Group 
in the underlying ALC system.
   So, I have two solution :
   1. return Boolean type.
   2. change function name to `Boolean onCheckXXX()` or `Boolean onIfExistXXX()`
   
   What do you think?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to