xunliu commented on code in PR #7077:
URL: https://github.com/apache/gravitino/pull/7077#discussion_r2061875518
##########
server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationConverter.java:
##########
@@ -17,11 +17,18 @@
package org.apache.gravitino.server.authorization.expression;
-import com.google.errorprone.annotations.DoNotCall;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.apache.gravitino.server.authorization.MetadataFilterHelper;
public class AuthorizationConverter {
Review Comment:
Please add comments to the class `AuthorizationConverter`.
##########
server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationConverter.java:
##########
@@ -34,8 +41,23 @@ private AuthorizationConverter() {}
* @param authorizationExpression authorization expression from {@link
MetadataFilterHelper}
* @return an OGNL expression used to call GravitinoAuthorizer
*/
- @DoNotCall
public static String convertToOgnlExpression(String authorizationExpression)
{
- throw new UnsupportedOperationException();
+ return EXPRESSION_CACHE.computeIfAbsent(
+ authorizationExpression,
+ (expression) -> {
+ Matcher matcher = PATTERN.matcher(expression);
+ StringBuffer result = new StringBuffer();
+
+ while (matcher.find()) {
+ String metadataType = matcher.group(1);
+ String privilege = matcher.group(2);
+ String replacement =
+ "authorizer.authorize(principal,METALAKE," + metadataType +
",'" + privilege + "')";
Review Comment:
Maybe we need to use `String.format()`?
##########
server-common/src/main/java/org/apache/gravitino/server/authorization/expression/AuthorizationConverter.java:
##########
@@ -17,11 +17,18 @@
package org.apache.gravitino.server.authorization.expression;
-import com.google.errorprone.annotations.DoNotCall;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
import org.apache.gravitino.server.authorization.MetadataFilterHelper;
public class AuthorizationConverter {
+ private static final Pattern PATTERN =
Pattern.compile("([A-Z_]+)::([A-Z_]+)");
+
+ private static final Map<String, String> EXPRESSION_CACHE = new
ConcurrentHashMap<>();
Review Comment:
Please add comments to the code.
##########
server-common/src/main/java/org/apache/gravitino/server/authorization/GravitinoAuthorizer.java:
##########
@@ -45,4 +45,19 @@ boolean authorize(
String metalake,
MetadataObject metadataObject,
Privilege.Name privilege);
+
+ /**
+ * Perform authorization and return the authorization result.
+ *
+ * @param principal the user principal
+ * @param metalake the metalake
+ * @param metadataObject the metadataObject.
+ * @param privilege for example, CREATE_CATALOG, CREATE_TABLE, etc.
+ * @return authorization result.
+ */
+ default boolean authorize(
+ Principal principal, String metalake, MetadataObject metadataObject,
String privilege) {
Review Comment:
Maybe we can directly pass `Privilege privilege` in the parameter?
--
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]
