flyrain commented on code in PR #4059:
URL: https://github.com/apache/polaris/pull/4059#discussion_r2991667147
##########
runtime/service/src/main/java/org/apache/polaris/service/admin/PolarisServiceImpl.java:
##########
@@ -674,73 +607,97 @@ public Response revokeGrantFromCatalogRole(
return Response.status(501).build(); // not implemented
}
+ return processGrantOperation(
+ grantRequest.getGrant(), catalogName, catalogRoleName,
GrantDirection.REVOKE);
+ }
+
+ private enum GrantDirection {
+ GRANT,
+ REVOKE
+ }
+
+ private static Namespace toNamespace(List<String> parts) {
+ return Namespace.of(parts.toArray(new String[0]));
+ }
+
+ private Response processGrantOperation(
+ GrantResource grant, String catalogName, String catalogRoleName,
GrantDirection direction) {
PrivilegeResult result;
- PolarisPrivilege privilege;
- switch (grantRequest.getGrant()) {
- // The per-securable-type Privilege enums must be exact String match for
a subset of all
- // PolarisPrivilege values.
+ // The per-securable-type Privilege enums must be exact String match for a
subset of all
+ // PolarisPrivilege values.
+ switch (grant) {
case ViewGrant viewGrant:
{
- privilege =
PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString());
- String viewName = viewGrant.getViewName();
- String[] namespaceParts = viewGrant.getNamespace().toArray(new
String[0]);
+ var privilege =
PolarisPrivilege.valueOf(viewGrant.getPrivilege().toString());
+ var identifier =
+ TableIdentifier.of(toNamespace(viewGrant.getNamespace()),
viewGrant.getViewName());
result =
- adminService.revokePrivilegeOnViewFromRole(
- catalogName,
- catalogRoleName,
- TableIdentifier.of(Namespace.of(namespaceParts), viewName),
- privilege);
+ direction == GrantDirection.GRANT
+ ? adminService.grantPrivilegeOnViewToRole(
+ catalogName, catalogRoleName, identifier, privilege)
+ : adminService.revokePrivilegeOnViewFromRole(
+ catalogName, catalogRoleName, identifier, privilege);
Review Comment:
Tried locally. It turns out removing 20 lines of code by adding 80 LOC. Net
negative. Not sure if it's worth to pursue, but I'm open to suggestion
--
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]
