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

Refactor the validation logic of the Tag and fix the test case.
Abyss-lord · Jan 7, 2025 · cd55663 · cd55663
1 parent dbd50ec
commit cd55663
Show file tree

Hide file tree

Showing 8 changed files with 188 additions and 143 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
@@ -642,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;
 
@@ -735,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);
     }

diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java b/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();
+  }
 }
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTag.java b/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();
+  }
 }
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllTags.java b/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();
+  }
 }
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTagProperty.java b/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();
+  }
 }
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/TagEntity.java b/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();
+  }
 }
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UntagEntity.java b/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();
+  }
 }