[apache#6123] fix(CLI): Refactor the validation logic of tag and role (…

…apache#6127)

### What changes were proposed in this pull request?

Refactor the validation logic of the Tag and Role, meantime fix the test
case.

### Why are the changes needed?

Fix: apache#6123 

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

ut + local test

**Role test**
```bash
gcli role grant -m demo_metalake --role admin
# Missing --privilege option.

gcli role revoke -m demo_metalake --role admin
# Missing --privilege option.
```

**Tag test**

```bash
gcli tag set -m demo_metalake 
# Missing --name option.

gcli tag set -m demo_metalake  --name catalog.schema.table --property property --tag tagA
# Missing --value option.

gcli tag set -m demo_metalake  --name catalog.schema.table --value value --tag tagA
# Missing --property option.

gcli tag remove -m demo_metalake --tag tagA
# Missing --name option.

gcli tag remove -m demo_metalake
# Missing --name option.

gcli tag delete -m demo_metalake
# Missing --tag option.

gcli tag create -m demo_metalake
# Missing --tag option.
```

clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java

@@ -55,6 +55,7 @@ public class ErrorMessages {
   public static final String MISSING_GROUP = "Missing --group option.";
   public static final String MISSING_METALAKE = "Missing --metalake option.";
   public static final String MISSING_NAME = "Missing --name option.";
+  public static final String MISSING_PRIVILEGES = "Missing --privilege option.";
   public static final String MISSING_PROPERTY = "Missing --property option.";
   public static final String MISSING_PROPERTY_AND_VALUE = "Missing --property and --value options.";
   public static final String MISSING_ROLE = "Missing --role option.";
@@ -63,6 +64,8 @@ public class ErrorMessages {
   public static final String MISSING_USER = "Missing --user option.";
   public static final String MISSING_VALUE = "Missing --value option.";
 
+  public static final String MULTIPLE_ROLE_COMMAND_ERROR =
+      "This command only supports one --role option.";
   public static final String MULTIPLE_TAG_COMMAND_ERROR =
       "This command only supports one --tag option.";
   public static final String MISSING_PROVIDER = "Missing --provider option.";

clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java

@@ -20,7 +20,6 @@
 package org.apache.gravitino.cli;
 
 import com.google.common.base.Joiner;
-import com.google.common.base.Preconditions;
 import com.google.common.collect.Lists;
 import java.io.BufferedReader;
 import java.io.IOException;
@@ -643,88 +642,77 @@ protected void handleTagCommand() {
     Command.setAuthenticationMode(auth, userName);
 
     String[] tags = line.getOptionValues(GravitinoOptions.TAG);
-    if (tags == null
-        && !((CommandActions.REMOVE.equals(command) && line.hasOption(GravitinoOptions.FORCE))
-            || CommandActions.LIST.equals(command))) {
-      System.err.println(ErrorMessages.MISSING_TAG);
-      Main.exit(-1);
-    }
 
     if (tags != null) {
       tags = Arrays.stream(tags).distinct().toArray(String[]::new);
     }
 
     switch (command) {
       case CommandActions.DETAILS:
-        newTagDetails(url, ignore, metalake, getOneTag(tags)).handle();
+        newTagDetails(url, ignore, metalake, getOneTag(tags)).validate().handle();
         break;
 
       case CommandActions.LIST:
         if (!name.hasCatalogName()) {
-          newListTags(url, ignore, metalake).handle();
+          newListTags(url, ignore, metalake).validate().handle();
         } else {
-          newListEntityTags(url, ignore, metalake, name).handle();
+          newListEntityTags(url, ignore, metalake, name).validate().handle();
         }
         break;
 
       case CommandActions.CREATE:
         String comment = line.getOptionValue(GravitinoOptions.COMMENT);
-        newCreateTags(url, ignore, metalake, tags, comment).handle();
+        newCreateTags(url, ignore, metalake, tags, comment).validate().handle();
         break;
 
       case CommandActions.DELETE:
         boolean forceDelete = line.hasOption(GravitinoOptions.FORCE);
-        newDeleteTag(url, ignore, forceDelete, metalake, tags).handle();
+        newDeleteTag(url, ignore, forceDelete, metalake, tags).validate().handle();
         break;
 
       case CommandActions.SET:
         String propertySet = line.getOptionValue(GravitinoOptions.PROPERTY);
         String valueSet = line.getOptionValue(GravitinoOptions.VALUE);
-        if (propertySet != null && valueSet != null) {
-          newSetTagProperty(url, ignore, metalake, getOneTag(tags), propertySet, valueSet).handle();
-        } else if (propertySet == null && valueSet == null) {
-          if (!name.hasName()) {
-            System.err.println(ErrorMessages.MISSING_NAME);
-            Main.exit(-1);
-          }
-          newTagEntity(url, ignore, metalake, name, tags).handle();
+        if (propertySet == null && valueSet == null) {
+          newTagEntity(url, ignore, metalake, name, tags).validate().handle();
         } else {
-          System.err.println(ErrorMessages.INVALID_SET_COMMAND);
-          Main.exit(-1);
+          newSetTagProperty(url, ignore, metalake, getOneTag(tags), propertySet, valueSet)
+              .validate()
+              .handle();
         }
         break;
 
       case CommandActions.REMOVE:
         boolean isTag = line.hasOption(GravitinoOptions.TAG);
         if (!isTag) {
           boolean forceRemove = line.hasOption(GravitinoOptions.FORCE);
-          newRemoveAllTags(url, ignore, metalake, name, forceRemove).handle();
+          newRemoveAllTags(url, ignore, metalake, name, forceRemove).validate().handle();
         } else {
           String propertyRemove = line.getOptionValue(GravitinoOptions.PROPERTY);
           if (propertyRemove != null) {
-            newRemoveTagProperty(url, ignore, metalake, getOneTag(tags), propertyRemove).handle();
+            newRemoveTagProperty(url, ignore, metalake, getOneTag(tags), propertyRemove)
+                .validate()
+                .handle();
           } else {
-            if (!name.hasName()) {
-              System.err.println(ErrorMessages.MISSING_NAME);
-              Main.exit(-1);
-            }
-            newUntagEntity(url, ignore, metalake, name, tags).handle();
+            newUntagEntity(url, ignore, metalake, name, tags).validate().handle();
           }
         }
         break;
 
       case CommandActions.PROPERTIES:
-        newListTagProperties(url, ignore, metalake, getOneTag(tags)).handle();
+        newListTagProperties(url, ignore, metalake, getOneTag(tags)).validate().handle();
         break;
 
       case CommandActions.UPDATE:
         if (line.hasOption(GravitinoOptions.COMMENT)) {
           String updateComment = line.getOptionValue(GravitinoOptions.COMMENT);
-          newUpdateTagComment(url, ignore, metalake, getOneTag(tags), updateComment).handle();
+          newUpdateTagComment(url, ignore, metalake, getOneTag(tags), updateComment)
+              .validate()
+              .handle();
         }
         if (line.hasOption(GravitinoOptions.RENAME)) {
           String newName = line.getOptionValue(GravitinoOptions.RENAME);
-          newUpdateTagName(url, ignore, metalake, getOneTag(tags), newName).handle();
+          newUpdateTagName(url, ignore, metalake, getOneTag(tags), newName).validate().handle();
         }
         break;
 
@@ -736,7 +724,7 @@ protected void handleTagCommand() {
   }
 
   private String getOneTag(String[] tags) {
-    if (tags.length > 1) {
+    if (tags == null || tags.length > 1) {
       System.err.println(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR);
       Main.exit(-1);
     }
@@ -767,34 +755,34 @@ protected void handleRoleCommand() {
     switch (command) {
       case CommandActions.DETAILS:
         if (line.hasOption(GravitinoOptions.AUDIT)) {
-          newRoleAudit(url, ignore, metalake, getOneRole(roles, CommandActions.DETAILS)).handle();
+          newRoleAudit(url, ignore, metalake, getOneRole(roles)).validate().handle();
         } else {
-          newRoleDetails(url, ignore, metalake, getOneRole(roles, CommandActions.DETAILS)).handle();
+          newRoleDetails(url, ignore, metalake, getOneRole(roles)).validate().handle();
         }
         break;
 
       case CommandActions.LIST:
-        newListRoles(url, ignore, metalake).handle();
+        newListRoles(url, ignore, metalake).validate().handle();
         break;
 
       case CommandActions.CREATE:
-        newCreateRole(url, ignore, metalake, roles).handle();
+        newCreateRole(url, ignore, metalake, roles).validate().handle();
         break;
 
       case CommandActions.DELETE:
         boolean forceDelete = line.hasOption(GravitinoOptions.FORCE);
-        newDeleteRole(url, ignore, forceDelete, metalake, roles).handle();
+        newDeleteRole(url, ignore, forceDelete, metalake, roles).validate().handle();
         break;
 
       case CommandActions.GRANT:
-        newGrantPrivilegesToRole(
-                url, ignore, metalake, getOneRole(roles, CommandActions.GRANT), name, privileges)
+        newGrantPrivilegesToRole(url, ignore, metalake, getOneRole(roles), name, privileges)
+            .validate()
             .handle();
         break;
 
       case CommandActions.REVOKE:
-        newRevokePrivilegesFromRole(
-                url, ignore, metalake, getOneRole(roles, CommandActions.REMOVE), name, privileges)
+        newRevokePrivilegesFromRole(url, ignore, metalake, getOneRole(roles), name, privileges)
+            .validate()
             .handle();
         break;
 
@@ -805,9 +793,12 @@ url, ignore, metalake, getOneRole(roles, CommandActions.REMOVE), name, privilege
     }
   }
 
-  private String getOneRole(String[] roles, String command) {
-    Preconditions.checkArgument(
-        roles.length == 1, command + " requires only one role, but multiple are currently passed.");
+  private String getOneRole(String[] roles) {
+    if (roles == null || roles.length != 1) {
+      System.err.println(ErrorMessages.MULTIPLE_ROLE_COMMAND_ERROR);
+      Main.exit(-1);
+    }
+
     return roles[0];
   }
 

clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java

@@ -103,4 +103,10 @@ private void handleMultipleTags() {
       System.out.println("Tags " + String.join(",", remaining) + " not created");
     }
   }
+
+  @Override
+  public Command validate() {
+    if (tags == null) exitWithError(ErrorMessages.MISSING_TAG);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTag.java

@@ -116,4 +116,10 @@ private void handleOnlyOneTag() {
       System.out.println("Tag " + tags[0] + " not deleted.");
     }
   }
+
+  @Override
+  public Command validate() {
+    if (tags == null) exitWithError(ErrorMessages.MISSING_TAG);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java

@@ -103,4 +103,10 @@ public void handle() {
     String all = String.join(",", privileges);
     System.out.println(role + " granted " + all + " on " + entity.getName());
   }
+
+  @Override
+  public Command validate() {
+    if (privileges == null) exitWithError(ErrorMessages.MISSING_PRIVILEGES);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllTags.java

@@ -118,4 +118,10 @@ public void handle() {
       System.out.println(entity + " has no tags");
     }
   }
+
+  @Override
+  public Command validate() {
+    if (name == null || !name.hasName()) exitWithError(ErrorMessages.MISSING_NAME);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java

@@ -103,4 +103,10 @@ public void handle() {
     String all = String.join(",", privileges);
     System.out.println(role + " revoked " + all + " on " + entity.getName());
   }
+
+  @Override
+  public Command validate() {
+    if (privileges == null) exitWithError(ErrorMessages.MISSING_PRIVILEGES);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTagProperty.java

@@ -74,4 +74,10 @@ public void handle() {
 
     System.out.println(tag + " property set.");
   }
+
+  @Override
+  public Command validate() {
+    validatePropertyAndValue(property, value);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/TagEntity.java

@@ -105,4 +105,10 @@ public void handle() {
 
     System.out.println(entity + " now tagged with " + all);
   }
+
+  @Override
+  public Command validate() {
+    if (name == null || !name.hasName()) exitWithError(ErrorMessages.MISSING_NAME);
+    return super.validate();
+  }
 }

clients/cli/src/main/java/org/apache/gravitino/cli/commands/UntagEntity.java

@@ -113,4 +113,10 @@ public void handle() {
       System.out.println(entity + " removed tag " + tags[0].toString() + " now tagged with " + all);
     }
   }
+
+  @Override
+  public Command validate() {
+    if (name == null || !name.hasName()) exitWithError(ErrorMessages.MISSING_NAME);
+    return super.validate();
+  }
 }