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 c9467512d5 [#6105] fix(CLI): Refactor the validation logic of schema 
and table (#6109)
c9467512d5 is described below

commit c9467512d557bd1ab2337523acc1fdd42cadb409
Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com>
AuthorDate: Mon Jan 6 18:23:45 2025 +0800

    [#6105] fix(CLI): Refactor the validation logic of schema and table (#6109)
    
    ### What changes were proposed in this pull request?
    
    (Please outline the changes and how this PR fixes the issue.)
    
    ### Why are the changes needed?
    
    1. Refactor the validation logic of schema and table.
    2. Add test case.
    
    Fix: #6105
    
    ### Does this PR introduce _any_ user-facing change?
    
    No
    
    ### How was this patch tested?
    
    UT + local test
    
    Schema test
    ```bash
    gcli schema set -m demo_metalake --name Hive_catalog.default
    # Missing --property and --value options.
    
    gcli schema set -m demo_metalake --name Hive_catalog.default --property 
propertyA
    # Missing --value option.
    
    gcli schema set -m demo_metalake --name Hive_catalog.default
     --value valA
    # Missing --property option.
    
    gcli schema remove -m demo_metalake --name Hive_catalog.default
    # Missing --property option.
    ```
    
    Table test
    ```bash
    gcli table set -m demo_metalake --name Hive_catalog.default.test_dates
    # Missing --property and --value options.
    
    gcli table set -m demo_metalake --name Hive_catalog.default.test_dates
    --property propertyA
    # Missing --value option.
    
    gcli table set -m demo_metalake --name Hive_catalog.default.test_dates
     --value valA
    # Missing --property option.
    
    gcli table remove -m demo_metalake --name Hive_catalog.default.test_dates
    # Missing --property option.
    
    gcli table create -m demo_metalake --name Hive_catalog.default.test_dates
    # Missing --columnfile option.
    ```
---
 .../org/apache/gravitino/cli/ErrorMessages.java    |   3 +-
 .../apache/gravitino/cli/GravitinoCommandLine.java |  48 +++++---
 .../org/apache/gravitino/cli/commands/Command.java |  17 ++-
 .../apache/gravitino/cli/commands/CreateTable.java |   6 +
 .../cli/commands/RemoveCatalogProperty.java        |   2 +-
 .../cli/commands/RemoveMetalakeProperty.java       |   4 +-
 .../cli/commands/RemoveSchemaProperty.java         |   6 +
 .../cli/commands/RemoveTableProperty.java          |   6 +
 .../gravitino/cli/commands/SetCatalogProperty.java |   2 +-
 .../cli/commands/SetMetalakeProperty.java          |   2 +-
 .../gravitino/cli/commands/SetSchemaProperty.java  |   6 +
 .../gravitino/cli/commands/SetTableProperty.java   |   6 +
 .../apache/gravitino/cli/TestSchemaCommands.java   |  88 +++++++++++++++
 .../apache/gravitino/cli/TestTableCommands.java    | 121 ++++++++++++++++++++-
 14 files changed, 285 insertions(+), 32 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
index c6c2a8d981..c839ad162b 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java
@@ -48,12 +48,14 @@ public class ErrorMessages {
   public static final String HELP_FAILED = "Failed to load help message: ";
 
   public static final String MALFORMED_NAME = "Malformed entity name.";
+  public static final String MISSING_COLUMN_FILE = "Missing --columnfile 
option.";
   public static final String MISSING_ENTITIES = "Missing required entity 
names: ";
 
   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_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.";
   public static final String MISSING_TAG = "Missing --tag option.";
   public static final String MISSING_URI = "Missing --uri option.";
@@ -62,7 +64,6 @@ public class ErrorMessages {
 
   public static final String MULTIPLE_TAG_COMMAND_ERROR =
       "This command only supports one --tag option.";
-  public static final String MISSING_PROPERTY_AND_VALUE = "Missing --property 
and --value options.";
   public static final String MISSING_PROVIDER = "Missing --provider option.";
 
   public static final String REGISTER_FAILED = "Failed to register model: ";
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 cd1ce5f6c0..589aa437df 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
@@ -367,35 +367,39 @@ public class GravitinoCommandLine extends 
TestableCommandLine {
     switch (command) {
       case CommandActions.DETAILS:
         if (line.hasOption(GravitinoOptions.AUDIT)) {
-          newSchemaAudit(url, ignore, metalake, catalog, schema).handle();
+          newSchemaAudit(url, ignore, metalake, catalog, 
schema).validate().handle();
         } else {
-          newSchemaDetails(url, ignore, metalake, catalog, schema).handle();
+          newSchemaDetails(url, ignore, metalake, catalog, 
schema).validate().handle();
         }
         break;
 
       case CommandActions.CREATE:
         String comment = line.getOptionValue(GravitinoOptions.COMMENT);
-        newCreateSchema(url, ignore, metalake, catalog, schema, 
comment).handle();
+        newCreateSchema(url, ignore, metalake, catalog, schema, 
comment).validate().handle();
         break;
 
       case CommandActions.DELETE:
         boolean force = line.hasOption(GravitinoOptions.FORCE);
-        newDeleteSchema(url, ignore, force, metalake, catalog, 
schema).handle();
+        newDeleteSchema(url, ignore, force, metalake, catalog, 
schema).validate().handle();
         break;
 
       case CommandActions.SET:
         String property = line.getOptionValue(GravitinoOptions.PROPERTY);
         String value = line.getOptionValue(GravitinoOptions.VALUE);
-        newSetSchemaProperty(url, ignore, metalake, catalog, schema, property, 
value).handle();
+        newSetSchemaProperty(url, ignore, metalake, catalog, schema, property, 
value)
+            .validate()
+            .handle();
         break;
 
       case CommandActions.REMOVE:
         property = line.getOptionValue(GravitinoOptions.PROPERTY);
-        newRemoveSchemaProperty(url, ignore, metalake, catalog, schema, 
property).handle();
+        newRemoveSchemaProperty(url, ignore, metalake, catalog, schema, 
property)
+            .validate()
+            .handle();
         break;
 
       case CommandActions.PROPERTIES:
-        newListSchemaProperties(url, ignore, metalake, catalog, 
schema).handle();
+        newListSchemaProperties(url, ignore, metalake, catalog, 
schema).validate().handle();
         break;
 
       default:
@@ -436,17 +440,17 @@ public class GravitinoCommandLine extends 
TestableCommandLine {
     switch (command) {
       case CommandActions.DETAILS:
         if (line.hasOption(GravitinoOptions.AUDIT)) {
-          newTableAudit(url, ignore, metalake, catalog, schema, 
table).handle();
+          newTableAudit(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         } else if (line.hasOption(GravitinoOptions.INDEX)) {
-          newListIndexes(url, ignore, metalake, catalog, schema, 
table).handle();
+          newListIndexes(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         } else if (line.hasOption(GravitinoOptions.DISTRIBUTION)) {
-          newTableDistribution(url, ignore, metalake, catalog, schema, 
table).handle();
+          newTableDistribution(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         } else if (line.hasOption(GravitinoOptions.PARTITION)) {
-          newTablePartition(url, ignore, metalake, catalog, schema, 
table).handle();
+          newTablePartition(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         } else if (line.hasOption(GravitinoOptions.SORTORDER)) {
-          newTableSortOrder(url, ignore, metalake, catalog, schema, 
table).handle();
+          newTableSortOrder(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         } else {
-          newTableDetails(url, ignore, metalake, catalog, schema, 
table).handle();
+          newTableDetails(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         }
         break;
 
@@ -455,39 +459,47 @@ public class GravitinoCommandLine extends 
TestableCommandLine {
           String columnFile = line.getOptionValue(GravitinoOptions.COLUMNFILE);
           String comment = line.getOptionValue(GravitinoOptions.COMMENT);
           newCreateTable(url, ignore, metalake, catalog, schema, table, 
columnFile, comment)
+              .validate()
               .handle();
           break;
         }
       case CommandActions.DELETE:
         boolean force = line.hasOption(GravitinoOptions.FORCE);
-        newDeleteTable(url, ignore, force, metalake, catalog, schema, 
table).handle();
+        newDeleteTable(url, ignore, force, metalake, catalog, schema, 
table).validate().handle();
         break;
 
       case CommandActions.SET:
         String property = line.getOptionValue(GravitinoOptions.PROPERTY);
         String value = line.getOptionValue(GravitinoOptions.VALUE);
         newSetTableProperty(url, ignore, metalake, catalog, schema, table, 
property, value)
+            .validate()
             .handle();
         break;
 
       case CommandActions.REMOVE:
         property = line.getOptionValue(GravitinoOptions.PROPERTY);
-        newRemoveTableProperty(url, ignore, metalake, catalog, schema, table, 
property).handle();
+        newRemoveTableProperty(url, ignore, metalake, catalog, schema, table, 
property)
+            .validate()
+            .handle();
         break;
 
       case CommandActions.PROPERTIES:
-        newListTableProperties(url, ignore, metalake, catalog, schema, 
table).handle();
+        newListTableProperties(url, ignore, metalake, catalog, schema, 
table).validate().handle();
         break;
 
       case CommandActions.UPDATE:
         {
           if (line.hasOption(GravitinoOptions.COMMENT)) {
             String comment = line.getOptionValue(GravitinoOptions.COMMENT);
-            newUpdateTableComment(url, ignore, metalake, catalog, schema, 
table, comment).handle();
+            newUpdateTableComment(url, ignore, metalake, catalog, schema, 
table, comment)
+                .validate()
+                .handle();
           }
           if (line.hasOption(GravitinoOptions.RENAME)) {
             String newName = line.getOptionValue(GravitinoOptions.RENAME);
-            newUpdateTableName(url, ignore, metalake, catalog, schema, table, 
newName).handle();
+            newUpdateTableName(url, ignore, metalake, catalog, schema, table, 
newName)
+                .validate()
+                .handle();
           }
           break;
         }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java
index 98c4096cb0..ea6abdd639 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/Command.java
@@ -112,17 +112,26 @@ public abstract class Command {
   }
 
   /**
-   * Validates that both property and value parameters are not null.
+   * Validates that both property and value arguments are not null.
    *
    * @param property The property name to check
    * @param value The value associated with the property
    */
-  protected void checkProperty(String property, String value) {
+  protected void validatePropertyAndValue(String property, String value) {
     if (property == null && value == null) 
exitWithError(ErrorMessages.MISSING_PROPERTY_AND_VALUE);
     if (property == null) exitWithError(ErrorMessages.MISSING_PROPERTY);
     if (value == null) exitWithError(ErrorMessages.MISSING_VALUE);
   }
 
+  /**
+   * Validates that the property argument is not null.
+   *
+   * @param property The property name to validate
+   */
+  protected void validateProperty(String property) {
+    if (property == null) exitWithError(ErrorMessages.MISSING_PROPERTY);
+  }
+
   /**
    * Builds a {@link GravitinoClient} instance with the provided server URL 
and metalake.
    *
@@ -216,8 +225,4 @@ public abstract class Command {
       throw new IllegalArgumentException("Unsupported output format");
     }
   }
-
-  protected String getMissingEntitiesInfo(String... entities) {
-    return ErrorMessages.MISSING_ENTITIES + COMMA_JOINER.join(entities);
-  }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTable.java 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTable.java
index fefa626722..aa409941e5 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTable.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTable.java
@@ -108,4 +108,10 @@ public class CreateTable extends Command {
 
     System.out.println(table + " created");
   }
+
+  @Override
+  public Command validate() {
+    if (columnFile == null) exitWithError(ErrorMessages.MISSING_COLUMN_FILE);
+    return super.validate();
+  }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveCatalogProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveCatalogProperty.java
index c777ba1628..dc1a76765b 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveCatalogProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveCatalogProperty.java
@@ -69,7 +69,7 @@ public class RemoveCatalogProperty extends Command {
 
   @Override
   public Command validate() {
-    if (property == null) exitWithError(ErrorMessages.MISSING_PROPERTY);
+    validateProperty(property);
     return super.validate();
   }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveMetalakeProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveMetalakeProperty.java
index 0664ddaad1..ce3a50fee1 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveMetalakeProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveMetalakeProperty.java
@@ -63,7 +63,7 @@ public class RemoveMetalakeProperty extends Command {
 
   @Override
   public Command validate() {
-    if (property == null) exitWithError(ErrorMessages.MISSING_PROPERTY);
-    return this;
+    validateProperty(property);
+    return super.validate();
   }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveSchemaProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveSchemaProperty.java
index 6fc41c0125..8fedcb6216 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveSchemaProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveSchemaProperty.java
@@ -77,4 +77,10 @@ public class RemoveSchemaProperty extends Command {
 
     System.out.println(property + " property removed.");
   }
+
+  @Override
+  public Command validate() {
+    validateProperty(property);
+    return super.validate();
+  }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveTableProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveTableProperty.java
index 8b3cd2383f..af370ce64b 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveTableProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveTableProperty.java
@@ -86,4 +86,10 @@ public class RemoveTableProperty extends Command {
 
     System.out.println(property + " property removed.");
   }
+
+  @Override
+  public Command validate() {
+    validateProperty(property);
+    return super.validate();
+  }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetCatalogProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetCatalogProperty.java
index 8b511d7458..034b1b8e2a 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetCatalogProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetCatalogProperty.java
@@ -77,7 +77,7 @@ public class SetCatalogProperty extends Command {
 
   @Override
   public Command validate() {
-    checkProperty(property, value);
+    validatePropertyAndValue(property, value);
     return this;
   }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetMetalakeProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetMetalakeProperty.java
index ff945cf742..ef67d008bc 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetMetalakeProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetMetalakeProperty.java
@@ -66,7 +66,7 @@ public class SetMetalakeProperty extends Command {
 
   @Override
   public Command validate() {
-    checkProperty(property, value);
+    validatePropertyAndValue(property, value);
     return this;
   }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetSchemaProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetSchemaProperty.java
index cc6151eaa2..bd9851ba8c 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetSchemaProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetSchemaProperty.java
@@ -81,4 +81,10 @@ public class SetSchemaProperty extends Command {
 
     System.out.println(schema + " property set.");
   }
+
+  @Override
+  public Command validate() {
+    validatePropertyAndValue(property, value);
+    return this;
+  }
 }
diff --git 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTableProperty.java
 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTableProperty.java
index 0209d21825..54ab88f343 100644
--- 
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTableProperty.java
+++ 
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTableProperty.java
@@ -90,4 +90,10 @@ public class SetTableProperty extends Command {
 
     System.out.println(table + " property set.");
   }
+
+  @Override
+  public Command validate() {
+    validatePropertyAndValue(property, value);
+    return super.validate();
+  }
 }
diff --git 
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestSchemaCommands.java 
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestSchemaCommands.java
index b3f67174fb..9059afeedb 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestSchemaCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestSchemaCommands.java
@@ -19,6 +19,7 @@
 
 package org.apache.gravitino.cli;
 
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.mockito.Mockito.doReturn;
@@ -30,6 +31,7 @@ import static org.mockito.Mockito.when;
 
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
+import java.nio.charset.StandardCharsets;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.Options;
 import org.apache.gravitino.cli.commands.CreateSchema;
@@ -106,6 +108,7 @@ class TestSchemaCommands {
         .when(commandLine)
         .newSchemaDetails(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema");
+    doReturn(mockDetails).when(mockDetails).validate();
     commandLine.handleCommandLine();
     verify(mockDetails).handle();
   }
@@ -126,6 +129,7 @@ class TestSchemaCommands {
         .when(commandLine)
         .newSchemaAudit(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema");
+    doReturn(mockAudit).when(mockAudit).validate();
     commandLine.handleCommandLine();
     verify(mockAudit).handle();
   }
@@ -153,6 +157,7 @@ class TestSchemaCommands {
             "catalog",
             "schema",
             "comment");
+    doReturn(mockCreate).when(mockCreate).validate();
     commandLine.handleCommandLine();
     verify(mockCreate).handle();
   }
@@ -172,6 +177,7 @@ class TestSchemaCommands {
         .when(commandLine)
         .newDeleteSchema(
             GravitinoCommandLine.DEFAULT_URL, false, false, "metalake_demo", 
"catalog", "schema");
+    doReturn(mockDelete).when(mockDelete).validate();
     commandLine.handleCommandLine();
     verify(mockDelete).handle();
   }
@@ -192,6 +198,7 @@ class TestSchemaCommands {
         .when(commandLine)
         .newDeleteSchema(
             GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo", 
"catalog", "schema");
+    doReturn(mockDelete).when(mockDelete).validate();
     commandLine.handleCommandLine();
     verify(mockDelete).handle();
   }
@@ -221,10 +228,71 @@ class TestSchemaCommands {
             "schema",
             "property",
             "value");
+    doReturn(mockSetProperty).when(mockSetProperty).validate();
     commandLine.handleCommandLine();
     verify(mockSetProperty).handle();
   }
 
+  @Test
+  void testSetSchemaPropertyCommandWithoutPropertyAndValue() {
+    Main.useExit = false;
+    SetSchemaProperty spySetProperty =
+        spy(
+            new SetSchemaProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                null,
+                null));
+
+    assertThrows(RuntimeException.class, spySetProperty::validate);
+    verify(spySetProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_PROPERTY_AND_VALUE, output);
+  }
+
+  @Test
+  void testSetSchemaPropertyCommandWithoutProperty() {
+    Main.useExit = false;
+    SetSchemaProperty spySetProperty =
+        spy(
+            new SetSchemaProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                null,
+                "value"));
+
+    assertThrows(RuntimeException.class, spySetProperty::validate);
+    verify(spySetProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_PROPERTY, output);
+  }
+
+  @Test
+  void testSetSchemaPropertyCommandWithoutValue() {
+    Main.useExit = false;
+    SetSchemaProperty spySetProperty =
+        spy(
+            new SetSchemaProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                "property",
+                null));
+
+    assertThrows(RuntimeException.class, spySetProperty::validate);
+    verify(spySetProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_VALUE, output);
+  }
+
   @Test
   void testRemoveSchemaPropertyCommand() {
     RemoveSchemaProperty mockRemoveProperty = mock(RemoveSchemaProperty.class);
@@ -247,10 +315,29 @@ class TestSchemaCommands {
             "catalog",
             "schema",
             "property");
+    doReturn(mockRemoveProperty).when(mockRemoveProperty).validate();
     commandLine.handleCommandLine();
     verify(mockRemoveProperty).handle();
   }
 
+  @Test
+  void testRemoveSchemaPropertyCommandWithoutProperty() {
+    Main.useExit = false;
+    RemoveSchemaProperty mockRemoveProperty =
+        spy(
+            new RemoveSchemaProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "demo_metalake",
+                "catalog",
+                "schema",
+                null));
+
+    assertThrows(RuntimeException.class, mockRemoveProperty::validate);
+    String errOutput = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_PROPERTY, errOutput);
+  }
+
   @Test
   void testListSchemaPropertiesCommand() {
     ListSchemaProperties mockListProperties = mock(ListSchemaProperties.class);
@@ -266,6 +353,7 @@ class TestSchemaCommands {
         .when(commandLine)
         .newListSchemaProperties(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema");
+    doReturn(mockListProperties).when(mockListProperties).validate();
     commandLine.handleCommandLine();
     verify(mockListProperties).handle();
   }
diff --git 
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestTableCommands.java 
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestTableCommands.java
index 946c330178..0193c834a5 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestTableCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestTableCommands.java
@@ -115,6 +115,7 @@ class TestTableCommands {
         .when(commandLine)
         .newTableDetails(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
+    doReturn(mockDetails).when(mockDetails).validate();
     commandLine.handleCommandLine();
     verify(mockDetails).handle();
   }
@@ -135,6 +136,7 @@ class TestTableCommands {
         .when(commandLine)
         .newListIndexes(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
+    doReturn(mockIndex).when(mockIndex).validate();
     commandLine.handleCommandLine();
     verify(mockIndex).handle();
   }
@@ -155,6 +157,7 @@ class TestTableCommands {
         .when(commandLine)
         .newTablePartition(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
+    doReturn(mockPartition).when(mockPartition).validate();
     commandLine.handleCommandLine();
     verify(mockPartition).handle();
   }
@@ -175,6 +178,7 @@ class TestTableCommands {
         .when(commandLine)
         .newTableDistribution(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
+    doReturn(mockDistribution).when(mockDistribution).validate();
     commandLine.handleCommandLine();
     verify(mockDistribution).handle();
   }
@@ -197,7 +201,7 @@ class TestTableCommands {
         .when(commandLine)
         .newTableSortOrder(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
-
+    doReturn(mockSortOrder).when(mockSortOrder).validate();
     commandLine.handleCommandLine();
     verify(mockSortOrder).handle();
   }
@@ -218,6 +222,7 @@ class TestTableCommands {
         .when(commandLine)
         .newTableAudit(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
+    doReturn(mockAudit).when(mockAudit).validate();
     commandLine.handleCommandLine();
     verify(mockAudit).handle();
   }
@@ -243,6 +248,7 @@ class TestTableCommands {
             "catalog",
             "schema",
             "users");
+    doReturn(mockDelete).when(mockDelete).validate();
     commandLine.handleCommandLine();
     verify(mockDelete).handle();
   }
@@ -269,6 +275,7 @@ class TestTableCommands {
             "catalog",
             "schema",
             "users");
+    doReturn(mockDelete).when(mockDelete).validate();
     commandLine.handleCommandLine();
     verify(mockDelete).handle();
   }
@@ -289,12 +296,13 @@ class TestTableCommands {
         .when(commandLine)
         .newListTableProperties(
             GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", 
"catalog", "schema", "users");
+    doReturn(mockListProperties).when(mockListProperties).validate();
     commandLine.handleCommandLine();
     verify(mockListProperties).handle();
   }
 
   @Test
-  void testSetFilesetPropertyCommand() {
+  void testSetTablePropertyCommand() {
     SetTableProperty mockSetProperties = mock(SetTableProperty.class);
 
     
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
@@ -320,10 +328,74 @@ class TestTableCommands {
             "user",
             "property",
             "value");
+    doReturn(mockSetProperties).when(mockSetProperties).validate();
     commandLine.handleCommandLine();
     verify(mockSetProperties).handle();
   }
 
+  @Test
+  void testSetTablePropertyCommandWithoutPropertyAndValue() {
+    Main.useExit = false;
+    SetTableProperty spySetProperty =
+        spy(
+            new SetTableProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                "table",
+                null,
+                null));
+
+    assertThrows(RuntimeException.class, spySetProperty::validate);
+    verify(spySetProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_PROPERTY_AND_VALUE, output);
+  }
+
+  @Test
+  void testSetTablePropertyCommandWithoutProperty() {
+    Main.useExit = false;
+    SetTableProperty spySetProperty =
+        spy(
+            new SetTableProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                "table",
+                null,
+                "value"));
+
+    assertThrows(RuntimeException.class, spySetProperty::validate);
+    verify(spySetProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_PROPERTY, output);
+  }
+
+  @Test
+  void testSetTablePropertyCommandWithoutValue() {
+    Main.useExit = false;
+    SetTableProperty spySetProperty =
+        spy(
+            new SetTableProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                "table",
+                "property",
+                null));
+
+    assertThrows(RuntimeException.class, spySetProperty::validate);
+    verify(spySetProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_VALUE, output);
+  }
+
   @Test
   void testRemoveTablePropertyCommand() {
     RemoveTableProperty mockSetProperties = mock(RemoveTableProperty.class);
@@ -348,10 +420,31 @@ class TestTableCommands {
             "schema",
             "users",
             "property");
+    doReturn(mockSetProperties).when(mockSetProperties).validate();
     commandLine.handleCommandLine();
     verify(mockSetProperties).handle();
   }
 
+  @Test
+  void testRemoveTablePropertyCommandWithoutProperty() {
+    Main.useExit = false;
+    RemoveTableProperty spyRemoveProperty =
+        spy(
+            new RemoveTableProperty(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                "table",
+                null));
+
+    assertThrows(RuntimeException.class, spyRemoveProperty::validate);
+    verify(spyRemoveProperty, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_PROPERTY, output);
+  }
+
   @Test
   void testUpdateTableCommentsCommand() {
     UpdateTableComment mockUpdate = mock(UpdateTableComment.class);
@@ -375,6 +468,7 @@ class TestTableCommands {
             "schema",
             "users",
             "New comment");
+    doReturn(mockUpdate).when(mockUpdate).validate();
     commandLine.handleCommandLine();
     verify(mockUpdate).handle();
   }
@@ -402,6 +496,7 @@ class TestTableCommands {
             "schema",
             "users",
             "people");
+    doReturn(mockUpdate).when(mockUpdate).validate();
     commandLine.handleCommandLine();
     verify(mockUpdate).handle();
   }
@@ -432,10 +527,32 @@ class TestTableCommands {
             "users",
             "users.csv",
             "comment");
+    doReturn(mockCreate).when(mockCreate).validate();
     commandLine.handleCommandLine();
     verify(mockCreate).handle();
   }
 
+  @Test
+  void testCreateTableWithoutFile() {
+    Main.useExit = false;
+    CreateTable spyCreate =
+        spy(
+            new CreateTable(
+                GravitinoCommandLine.DEFAULT_URL,
+                false,
+                "metalake_demo",
+                "catalog",
+                "schema",
+                "table",
+                null,
+                "comment"));
+
+    assertThrows(RuntimeException.class, spyCreate::validate);
+    verify(spyCreate, never()).handle();
+    String output = new String(errContent.toByteArray(), 
StandardCharsets.UTF_8).trim();
+    assertEquals(ErrorMessages.MISSING_COLUMN_FILE, output);
+  }
+
   @Test
   @SuppressWarnings("DefaultCharset")
   void testListTableWithoutCatalog() {

Reply via email to