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 <[email protected]>
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() {
