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() {