This is an automated email from the ASF dual-hosted git repository.
liuxun 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 7a1abf595 [#5926] fix(CLI): Fix Missleading error message in Gravitino
CLI when missing schema (#5939)
7a1abf595 is described below
commit 7a1abf595c95c2a09dfc94828f7228aa0aec4002
Author: Lord of Abyss <[email protected]>
AuthorDate: Mon Dec 23 17:03:24 2024 +0800
[#5926] fix(CLI): Fix Missleading error message in Gravitino CLI when
missing schema (#5939)
### What changes were proposed in this pull request?
Fix schema arguments validation just like table commands. running
command like `gcli schema details --metalake metalake_demo --name
catalog_postgres -I` give Missleading error message as below.
```bash
Malformed entity name.
Invalid string to encode: null
```
It should display a friendly error message.
### Why are the changes needed?
Fix: #5926
### Does this PR introduce _any_ user-facing change?
NO
### How was this patch tested?
```bash
bin/gcli.sh schema details --m demo_metalake -i
# output
# Missing --name option.
# Missing --name option.
# Missing required argument(s): catalog, schema
bin/gcli.sh schema details --m demo_metalake --name Hive_catalog -i
# output
# Malformed entity name.
# Missing required argument(s): schema
bin/gcli.sh schema details --m demo_metalake --name Hive_catalog.default -i
# ouput: default,Default Hive database
bin/gcli.sh schema list --m demo_metalake -i
# output:
# Missing --name option.
# Missing required argument(s): catalog
bin/gcli.sh schema list --m demo_metalake -i --name Hive_catalog
# correct output
```
---
.../apache/gravitino/cli/GravitinoCommandLine.java | 16 ++++
.../apache/gravitino/cli/TestSchemaCommands.java | 87 ++++++++++++++++++++++
2 files changed, 103 insertions(+)
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 f5b7e28a5..9545b2a66 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
@@ -21,6 +21,7 @@ package org.apache.gravitino.cli;
import com.google.common.base.Joiner;
import com.google.common.base.Preconditions;
+import com.google.common.collect.Lists;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
@@ -318,14 +319,29 @@ public class GravitinoCommandLine extends
TestableCommandLine {
String catalog = name.getCatalogName();
Command.setAuthenticationMode(auth, userName);
+ List<String> missingEntities = Lists.newArrayList();
+ if (metalake == null) missingEntities.add(CommandEntities.METALAKE);
+ if (catalog == null) missingEntities.add(CommandEntities.CATALOG);
// Handle the CommandActions.LIST action separately as it doesn't use
`schema`
if (CommandActions.LIST.equals(command)) {
+ if (!missingEntities.isEmpty()) {
+ System.err.println("Missing required argument(s): " +
COMMA_JOINER.join(missingEntities));
+ Main.exit(-1);
+ }
newListSchema(url, ignore, metalake, catalog).handle();
return;
}
String schema = name.getSchemaName();
+ if (schema == null) {
+ missingEntities.add(CommandEntities.SCHEMA);
+ }
+
+ if (!missingEntities.isEmpty()) {
+ System.err.println("Missing required argument(s): " +
COMMA_JOINER.join(missingEntities));
+ Main.exit(-1);
+ }
switch (command) {
case CommandActions.DETAILS:
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 89cc72bcd..190e86635 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,12 +19,17 @@
package org.apache.gravitino.cli;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
+import java.io.ByteArrayOutputStream;
+import java.io.PrintStream;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.Options;
import org.apache.gravitino.cli.commands.CreateSchema;
@@ -35,6 +40,7 @@ import org.apache.gravitino.cli.commands.RemoveSchemaProperty;
import org.apache.gravitino.cli.commands.SchemaAudit;
import org.apache.gravitino.cli.commands.SchemaDetails;
import org.apache.gravitino.cli.commands.SetSchemaProperty;
+import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
@@ -42,10 +48,28 @@ class TestSchemaCommands {
private CommandLine mockCommandLine;
private Options mockOptions;
+ private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
+ private final ByteArrayOutputStream errContent = new ByteArrayOutputStream();
+ private final PrintStream originalOut = System.out;
+ private final PrintStream originalErr = System.err;
+
@BeforeEach
void setUp() {
mockCommandLine = mock(CommandLine.class);
mockOptions = mock(Options.class);
+ System.setOut(new PrintStream(outContent));
+ System.setErr(new PrintStream(errContent));
+ }
+
+ @AfterEach
+ void restoreExitFlg() {
+ Main.useExit = true;
+ }
+
+ @AfterEach
+ public void restoreStreams() {
+ System.setOut(originalOut);
+ System.setErr(originalErr);
}
@Test
@@ -245,4 +269,67 @@ class TestSchemaCommands {
commandLine.handleCommandLine();
verify(mockListProperties).handle();
}
+
+ @Test
+ @SuppressWarnings("DefaultCharset")
+ void testListSchemaWithoutCatalog() {
+ Main.useExit = false;
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(false);
+
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.SCHEMA,
CommandActions.LIST));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ verify(commandLine, never())
+ .newListSchema(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", null);
+ assertTrue(
+ errContent.toString().contains("Missing required argument(s): " +
CommandEntities.CATALOG));
+ }
+
+ @Test
+ @SuppressWarnings("DefaultCharset")
+ void testDetailsSchemaWithoutCatalog() {
+ Main.useExit = false;
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(false);
+
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.SCHEMA,
CommandActions.DETAILS));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ assertTrue(
+ errContent
+ .toString()
+ .contains(
+ "Missing required argument(s): "
+ + CommandEntities.CATALOG
+ + ", "
+ + CommandEntities.SCHEMA));
+ }
+
+ @Test
+ @SuppressWarnings("DefaultCharset")
+ void testDetailsSchemaWithoutSchema() {
+ Main.useExit = false;
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog");
+
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.SCHEMA,
CommandActions.DETAILS));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ assertTrue(
+ errContent.toString().contains("Missing required argument(s): " +
CommandEntities.SCHEMA));
+ }
}
