This is an automated email from the ASF dual-hosted git repository.

jmclean 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 5bc8fefe3 [#5569] improvement(CLI): Add ability to grant or revoke 
multiple roles or groups at once in the Gravitino CLI (#5811)
5bc8fefe3 is described below

commit 5bc8fefe381934aba25fb2a7f27370bbd747e516
Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com>
AuthorDate: Wed Dec 11 05:33:33 2024 +0800

    [#5569] improvement(CLI): Add ability to grant or revoke multiple roles or 
groups at once in the Gravitino CLI (#5811)
    
    ### What changes were proposed in this pull request?
    
    Add ability to grant or revoke multiple roles or groups at once in the
    Gravitino CLI,
    
    ### Why are the changes needed?
    
    Fix:[#5569](https://github.com/apache/gravitino/issues/5569)
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    #### I. bash test
    
    **Grant\revoke role to user**
    1. Grant single role to a user
    
    
![image](https://github.com/user-attachments/assets/a8990b1e-dc74-4cf3-bcd8-6e09f32f43d6)
    2. Grant multiple role to a user
    
    
![image](https://github.com/user-attachments/assets/dc828341-7124-487d-b7ca-db402fb2c196)
    3. Revoke multiple role to from user
    
    
![image](https://github.com/user-attachments/assets/87603fc4-9b7d-4bbf-8af9-b9d2dc67d86b)
    
    **Grant\revoke role to group**
    1.  Grant single role to a group
    
    
![image](https://github.com/user-attachments/assets/549aeec3-c55b-4d76-a6f5-f8baf06788e3)
    2. Grant multiple role to a group
    
    
![image](https://github.com/user-attachments/assets/fbe05fcf-2232-4e36-8dbd-2d7b87e4fb9e)
    3. Revoke multiple role to from group
    
    
![image](https://github.com/user-attachments/assets/8ddd3d1a-a550-40f7-b292-004a42225fa2)
    
    #### II. unit test
    
    Please check the
    
[commit](https://github.com/apache/gravitino/commit/788396aab379638707ca5733e89069fab0e0002f)
---
 .../apache/gravitino/cli/GravitinoCommandLine.java | 23 +++++---
 .../org/apache/gravitino/cli/GravitinoOptions.java |  4 +-
 .../apache/gravitino/cli/TestGroupCommands.java    | 66 ++++++++++++++++++++-
 .../org/apache/gravitino/cli/TestUserCommands.java | 67 ++++++++++++++++++++++
 4 files changed, 148 insertions(+), 12 deletions(-)

diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java 
b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
index 58255849a..843593487 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli;
 
+import com.google.common.base.Joiner;
 import com.google.common.base.Preconditions;
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -49,6 +50,8 @@ public class GravitinoCommandLine extends TestableCommandLine 
{
 
   public static final String CMD = "gcli"; // recommended name
   public static final String DEFAULT_URL = "http://localhost:8090";;
+  // This joiner is used to join multiple outputs to be displayed, e.g. roles 
or groups
+  private static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls();
 
   /**
    * Gravitino Command line.
@@ -382,15 +385,17 @@ public class GravitinoCommandLine extends 
TestableCommandLine {
       boolean force = line.hasOption(GravitinoOptions.FORCE);
       newDeleteUser(url, ignore, force, metalake, user).handle();
     } else if (CommandActions.REVOKE.equals(command)) {
-      String role = line.getOptionValue(GravitinoOptions.ROLE);
-      if (role != null) {
+      String[] roles = line.getOptionValues(GravitinoOptions.ROLE);
+      for (String role : roles) {
         newRemoveRoleFromUser(url, ignore, metalake, user, role).handle();
       }
+      System.out.printf("Remove roles %s from user %s%n", 
COMMA_JOINER.join(roles), user);
     } else if (CommandActions.GRANT.equals(command)) {
-      String role = line.getOptionValue(GravitinoOptions.ROLE);
-      if (role != null) {
+      String[] roles = line.getOptionValues(GravitinoOptions.ROLE);
+      for (String role : roles) {
         newAddRoleToUser(url, ignore, metalake, user, role).handle();
       }
+      System.out.printf("Grant roles %s to user %s%n", String.join(", ", 
roles), user);
     } else {
       System.err.println(ErrorMessages.UNSUPPORTED_ACTION);
     }
@@ -417,15 +422,17 @@ public class GravitinoCommandLine extends 
TestableCommandLine {
       boolean force = line.hasOption(GravitinoOptions.FORCE);
       newDeleteGroup(url, ignore, force, metalake, group).handle();
     } else if (CommandActions.REVOKE.equals(command)) {
-      String role = line.getOptionValue(GravitinoOptions.ROLE);
-      if (role != null) {
+      String[] roles = line.getOptionValues(GravitinoOptions.ROLE);
+      for (String role : roles) {
         newRemoveRoleFromGroup(url, ignore, metalake, group, role).handle();
       }
+      System.out.printf("Remove roles %s from group %s%n", 
COMMA_JOINER.join(roles), group);
     } else if (CommandActions.GRANT.equals(command)) {
-      String role = line.getOptionValue(GravitinoOptions.ROLE);
-      if (role != null) {
+      String[] roles = line.getOptionValues(GravitinoOptions.ROLE);
+      for (String role : roles) {
         newAddRoleToGroup(url, ignore, metalake, group, role).handle();
       }
+      System.out.printf("Grant roles %s to group %s%n", 
COMMA_JOINER.join(roles), group);
     } else {
       System.err.println(ErrorMessages.UNSUPPORTED_ACTION);
     }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java 
b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
index 7b7728a0b..91993226b 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoOptions.java
@@ -100,12 +100,12 @@ public class GravitinoOptions {
     options.addOption(createArgOption(AUTO, "column value auto-increments 
(true/false)"));
     options.addOption(createArgOption(DEFAULT, "default column value"));
     options.addOption(createSimpleOption("o", OWNER, "display entity owner"));
-    options.addOption(createArgOption("r", ROLE, "role name"));
     options.addOption(createArgOption(COLUMNFILE, "CSV file describing 
columns"));
 
-    // Properties and tags can have multiple values
+    // Options that support multiple values
     options.addOption(createArgsOption("p", PROPERTIES, "property name/value 
pairs"));
     options.addOption(createArgsOption("t", TAG, "tag name"));
+    options.addOption(createArgsOption("r", ROLE, "role name"));
 
     // Force delete entities and rename metalake operations
     options.addOption(createSimpleOption("f", FORCE, "force operation"));
diff --git 
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java 
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
index 59af42803..780f33844 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java
@@ -135,7 +135,6 @@ class TestGroupCommands {
     verify(mockDelete).handle();
   }
 
-  @Test
   void testRemoveRoleFromGroupCommand() {
     RemoveRoleFromGroup mockRemove = mock(RemoveRoleFromGroup.class);
     
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
@@ -156,7 +155,6 @@ class TestGroupCommands {
     verify(mockRemove).handle();
   }
 
-  @Test
   void testAddRoleToGroupCommand() {
     AddRoleToGroup mockAdd = mock(AddRoleToGroup.class);
     
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
@@ -176,4 +174,68 @@ class TestGroupCommands {
     commandLine.handleCommandLine();
     verify(mockAdd).handle();
   }
+
+  @Test
+  void testRemoveRolesFromGroupCommand() {
+    RemoveRoleFromGroup mockRemoveFirstRole = mock(RemoveRoleFromGroup.class);
+    RemoveRoleFromGroup mockRemoveSecondRole = mock(RemoveRoleFromGroup.class);
+    
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+    when(mockCommandLine.hasOption(GravitinoOptions.GROUP)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.GROUP)).thenReturn("groupA");
+    when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true);
+    when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE))
+        .thenReturn(new String[] {"admin", "role1"});
+    GravitinoCommandLine commandLine =
+        spy(
+            new GravitinoCommandLine(
+                mockCommandLine, mockOptions, CommandEntities.GROUP, 
CommandActions.REVOKE));
+    // Verify first role
+    doReturn(mockRemoveFirstRole)
+        .when(commandLine)
+        .newRemoveRoleFromGroup(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"groupA", "admin");
+    commandLine.handleCommandLine();
+    verify(mockRemoveFirstRole).handle();
+
+    // Verify second role
+    doReturn(mockRemoveSecondRole)
+        .when(commandLine)
+        .newRemoveRoleFromGroup(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"groupA", "role1");
+    commandLine.handleCommandLine();
+    verify(mockRemoveSecondRole).handle();
+  }
+
+  @Test
+  void testAddRolesToGroupCommand() {
+    AddRoleToGroup mockAddFirstRole = mock(AddRoleToGroup.class);
+    AddRoleToGroup mockAddSecondRole = mock(AddRoleToGroup.class);
+    
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+    when(mockCommandLine.hasOption(GravitinoOptions.GROUP)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.GROUP)).thenReturn("groupA");
+    when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true);
+    when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE))
+        .thenReturn(new String[] {"admin", "role1"});
+    GravitinoCommandLine commandLine =
+        spy(
+            new GravitinoCommandLine(
+                mockCommandLine, mockOptions, CommandEntities.GROUP, 
CommandActions.GRANT));
+    // Verify first role
+    doReturn(mockAddFirstRole)
+        .when(commandLine)
+        .newAddRoleToGroup(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"groupA", "admin");
+    commandLine.handleCommandLine();
+    verify(mockAddFirstRole).handle();
+
+    // Verify second role
+    doReturn(mockAddSecondRole)
+        .when(commandLine)
+        .newAddRoleToGroup(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"groupA", "role1");
+    commandLine.handleCommandLine();
+    verify(mockAddSecondRole).handle();
+  }
 }
diff --git 
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java 
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
index ab6ff3a7c..0628d1f63 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java
@@ -27,6 +27,7 @@ import static org.mockito.Mockito.when;
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Options;
+import org.apache.gravitino.cli.commands.AddRoleToUser;
 import org.apache.gravitino.cli.commands.CreateUser;
 import org.apache.gravitino.cli.commands.DeleteUser;
 import org.apache.gravitino.cli.commands.ListUsers;
@@ -173,4 +174,70 @@ class TestUserCommands {
     commandLine.handleCommandLine();
     verify(mockAdd).handle();
   }
+
+  @Test
+  void testRemoveRolesFromUserCommand() {
+    RemoveRoleFromUser mockRemoveFirstRole = mock(RemoveRoleFromUser.class);
+    RemoveRoleFromUser mockRemoveSecondRole = mock(RemoveRoleFromUser.class);
+
+    
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+    when(mockCommandLine.hasOption(GravitinoOptions.USER)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.USER)).thenReturn("user");
+    when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true);
+    when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE))
+        .thenReturn(new String[] {"admin", "role1"});
+    GravitinoCommandLine commandLine =
+        spy(
+            new GravitinoCommandLine(
+                mockCommandLine, mockOptions, CommandEntities.USER, 
CommandActions.REVOKE));
+    // Verify first role
+    doReturn(mockRemoveFirstRole)
+        .when(commandLine)
+        .newRemoveRoleFromUser(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", 
"admin");
+    commandLine.handleCommandLine();
+    verify(mockRemoveFirstRole).handle();
+
+    // Verify second role
+    doReturn(mockRemoveSecondRole)
+        .when(commandLine)
+        .newRemoveRoleFromUser(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", 
"role1");
+    commandLine.handleCommandLine();
+    verify(mockRemoveSecondRole).handle();
+  }
+
+  @Test
+  void testAddRolesToUserCommand() {
+    AddRoleToUser mockAddFirstRole = mock(AddRoleToUser.class);
+    AddRoleToUser mockAddSecondRole = mock(AddRoleToUser.class);
+
+    
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+    when(mockCommandLine.hasOption(GravitinoOptions.USER)).thenReturn(true);
+    
when(mockCommandLine.getOptionValue(GravitinoOptions.USER)).thenReturn("user");
+    when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true);
+    when(mockCommandLine.getOptionValues(GravitinoOptions.ROLE))
+        .thenReturn(new String[] {"admin", "role1"});
+    GravitinoCommandLine commandLine =
+        spy(
+            new GravitinoCommandLine(
+                mockCommandLine, mockOptions, CommandEntities.USER, 
CommandActions.GRANT));
+    // Verify first role
+    doReturn(mockAddFirstRole)
+        .when(commandLine)
+        .newAddRoleToUser(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", 
"admin");
+    commandLine.handleCommandLine();
+    verify(mockAddFirstRole).handle();
+
+    // Verify second role
+    doReturn(mockAddSecondRole)
+        .when(commandLine)
+        .newAddRoleToUser(
+            GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", 
"role1");
+    commandLine.handleCommandLine();
+    verify(mockAddSecondRole).handle();
+  }
 }

Reply via email to