[apache#5985] improvement(CLI): Fix role command that supports handli…

…ng multiple values Fix role command that supports handling multiple values, CLI can create and delete multiple roles simultaneously.
Abyss-lord · Dec 25, 2024 · 3948e02 · 3948e02
1 parent a58b7f8
commit 3948e02
Show file tree

Hide file tree

Showing 5 changed files with 50 additions and 25 deletions.
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java
@@ -32,6 +32,7 @@ public class ErrorMessages {
   public static final String MISSING_NAME = "Missing --name option.";
   public static final String MISSING_GROUP = "Missing --group option.";
   public static final String MISSING_USER = "Missing --user option.";
+  public static final String MISSING_ROLE = "Missing --role option.";
   public static final String METALAKE_EXISTS = "Metalake already exists.";
   public static final String CATALOG_EXISTS = "Catalog already exists.";
   public static final String SCHEMA_EXISTS = "Schema already exists.";
@@ -42,6 +43,8 @@ public class ErrorMessages {
   public static final String UNKNOWN_TAG = "Unknown tag.";
   public static final String MULTIPLE_TAG_COMMAND_ERROR =
       "Error: The current command only supports one --tag option.";
+  public static final String MULTIPLE_ROLE_COMMAND_ERROR =
+      "Error: The current command only supports one --role option.";
   public static final String TAG_EXISTS = "Tag already exists.";
   public static final String UNKNOWN_COLUMN = "Unknown column.";
   public static final String COLUMN_EXISTS = "Column already exists.";

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
@@ -733,17 +733,26 @@ protected void handleRoleCommand() {
     String userName = line.getOptionValue(GravitinoOptions.LOGIN);
     FullName name = new FullName(line);
     String metalake = name.getMetalakeName();
-    String role = line.getOptionValue(GravitinoOptions.ROLE);
     String[] privileges = line.getOptionValues(GravitinoOptions.PRIVILEGE);
 
     Command.setAuthenticationMode(auth, userName);
 
+    String[] roles = line.getOptionValues(GravitinoOptions.ROLE);
+    if (roles == null && !CommandActions.LIST.equals(command)) {
+      System.err.println(ErrorMessages.MISSING_ROLE);
+      Main.exit(-1);
+    }
+
+    if (roles != null) {
+      roles = Arrays.stream(roles).distinct().toArray(String[]::new);
+    }
+
     switch (command) {
       case CommandActions.DETAILS:
         if (line.hasOption(GravitinoOptions.AUDIT)) {
-          newRoleAudit(url, ignore, metalake, role).handle();
+          newRoleAudit(url, ignore, metalake, getOneRole(roles)).handle();
         } else {
-          newRoleDetails(url, ignore, metalake, role).handle();
+          newRoleDetails(url, ignore, metalake, getOneRole(roles)).handle();
         }
         break;
 
@@ -752,20 +761,22 @@ protected void handleRoleCommand() {
         break;
 
       case CommandActions.CREATE:
-        newCreateRole(url, ignore, metalake, role).handle();
+        newCreateRole(url, ignore, metalake, roles).handle();
         break;
 
       case CommandActions.DELETE:
         boolean forceDelete = line.hasOption(GravitinoOptions.FORCE);
-        newDeleteRole(url, ignore, forceDelete, metalake, role).handle();
+        newDeleteRole(url, ignore, forceDelete, metalake, roles).handle();
         break;
 
       case CommandActions.GRANT:
-        newGrantPrivilegesToRole(url, ignore, metalake, role, name, privileges).handle();
+        newGrantPrivilegesToRole(url, ignore, metalake, getOneRole(roles), name, privileges)
+            .handle();
         break;
 
       case CommandActions.REVOKE:
-        newRevokePrivilegesFromRole(url, ignore, metalake, role, name, privileges).handle();
+        newRevokePrivilegesFromRole(url, ignore, metalake, getOneRole(roles), name, privileges)
+            .handle();
         break;
 
       default:
@@ -775,6 +786,11 @@ protected void handleRoleCommand() {
     }
   }
 
+  private String getOneRole(String[] roles) {
+    Preconditions.checkArgument(roles.length <= 1, ErrorMessages.MULTIPLE_ROLE_COMMAND_ERROR);
+    return roles[0];
+  }
+
   /**
    * Handles the command execution for Columns based on command type and the command line options.
    */

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java
@@ -468,13 +468,13 @@ protected RoleAudit newRoleAudit(String url, boolean ignore, String metalake, St
     return new RoleAudit(url, ignore, metalake, role);
   }
 
-  protected CreateRole newCreateRole(String url, boolean ignore, String metalake, String role) {
-    return new CreateRole(url, ignore, metalake, role);
+  protected CreateRole newCreateRole(String url, boolean ignore, String metalake, String[] roles) {
+    return new CreateRole(url, ignore, metalake, roles);
   }
 
   protected DeleteRole newDeleteRole(
-      String url, boolean ignore, boolean force, String metalake, String role) {
-    return new DeleteRole(url, ignore, force, metalake, role);
+      String url, boolean ignore, boolean force, String metalake, String[] roles) {
+    return new DeleteRole(url, ignore, force, metalake, roles);
   }
 
   protected TagDetails newTagDetails(String url, boolean ignore, String metalake, String tag) {

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli.commands;
 
+import com.google.common.base.Joiner;
 import java.util.Collections;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoClient;
@@ -27,28 +28,30 @@
 
 public class CreateRole extends Command {
   protected String metalake;
-  protected String role;
+  protected String[] roles;
 
   /**
    * Create a new role.
    *
    * @param url The URL of the Gravitino server.
    * @param ignoreVersions If true don't check the client/server versions match.
    * @param metalake The name of the metalake.
-   * @param role The name of the role.
+   * @param roles The array of roles.
    */
-  public CreateRole(String url, boolean ignoreVersions, String metalake, String role) {
+  public CreateRole(String url, boolean ignoreVersions, String metalake, String[] roles) {
     super(url, ignoreVersions);
     this.metalake = metalake;
-    this.role = role;
+    this.roles = roles;
   }
 
   /** Create a new role. */
   @Override
   public void handle() {
     try {
       GravitinoClient client = buildClient(metalake);
-      client.createRole(role, null, Collections.EMPTY_LIST);
+      for (String role : roles) {
+        client.createRole(role, null, Collections.EMPTY_LIST);
+      }
     } catch (NoSuchMetalakeException err) {
       exitWithError(ErrorMessages.UNKNOWN_METALAKE);
     } catch (RoleAlreadyExistsException err) {
@@ -57,6 +60,6 @@ public void handle() {
       exitWithError(exp.getMessage());
     }
 
-    System.out.println(role + " created");
+    System.out.println(Joiner.on(", ").join(roles) + " created");
   }
 }
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli.commands;
 
+import com.google.common.base.Joiner;
 import org.apache.gravitino.cli.AreYouSure;
 import org.apache.gravitino.cli.ErrorMessages;
 import org.apache.gravitino.client.GravitinoClient;
@@ -28,7 +29,7 @@
 public class DeleteRole extends Command {
 
   protected String metalake;
-  protected String role;
+  protected String[] roles;
   protected boolean force;
 
   /**
@@ -38,28 +39,30 @@ public class DeleteRole extends Command {
    * @param ignoreVersions If true don't check the client/server versions match.
    * @param force Force operation.
    * @param metalake The name of the metalake.
-   * @param role The name of the role.
+   * @param roles The name of the role.
    */
   public DeleteRole(
-      String url, boolean ignoreVersions, boolean force, String metalake, String role) {
+      String url, boolean ignoreVersions, boolean force, String metalake, String[] roles) {
     super(url, ignoreVersions);
     this.metalake = metalake;
     this.force = force;
-    this.role = role;
+    this.roles = roles;
   }
 
   /** Delete a role. */
   @Override
   public void handle() {
-    boolean deleted = false;
+    boolean deleted = true;
 
     if (!AreYouSure.really(force)) {
       return;
     }
 
     try {
       GravitinoClient client = buildClient(metalake);
-      deleted = client.deleteRole(role);
+      for (String role : roles) {
+        deleted &= client.deleteRole(role);
+      }
     } catch (NoSuchMetalakeException err) {
       exitWithError(ErrorMessages.UNKNOWN_METALAKE);
     } catch (NoSuchRoleException err) {
@@ -69,9 +72,9 @@ public void handle() {
     }
 
     if (deleted) {
-      System.out.println(role + " deleted.");
+      System.out.println(Joiner.on(", ").join(roles) + " deleted.");
     } else {
-      System.out.println(role + " not deleted.");
+      System.out.println(Joiner.on(", ").join(roles) + " not deleted.");
     }
   }
 }