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 9a5fef924 [#5831] fix(CLI): Fix CLi gives unexpected output when input
tag set command (#5897)
9a5fef924 is described below
commit 9a5fef924ee3488ef99bb00c75913f885a84dff3
Author: Lord of Abyss <[email protected]>
AuthorDate: Tue Dec 31 08:30:37 2024 +0800
[#5831] fix(CLI): Fix CLi gives unexpected output when input tag set
command (#5897)
### What changes were proposed in this pull request?
Running the command `gcli tag set --metalake metalake_demo` or `gcli tag
remove --metalake metalake_demo` gives unexpected output.it should give
some help information.
### Why are the changes needed?
Fix: #5831
### Does this PR introduce _any_ user-facing change?
NO
### How was this patch tested?
local test + UT.
```bash
gcli tag set -m demo_metalake
# Missing --tag option.
gcli tag set -m demo_metalake --tag tagA
# Missing --name option.
gcli tag remove -m demo_metalake
# Missing --tag option.
gcli tag remove -m demo_metalake --tag tagA
# Missing --name option.
gcli tag set --tag tagA --property test
# Command cannot be executed. The set command only supports configuring tag
properties or attaching tags to entity.
gcli tag set --tag tagA --value val1
# Command cannot be executed. The set command only supports configuring tag
properties or attaching tags to entity.
gcli tag set --metalake demo_metalake --name Hive_catalog --tag tagB tagC
# Hive_catalog now tagged with tagA,tagB,tagC
gcli tag remove --metalake demo_metalake --name Hive_catalog --tag tagA tagC
# Hive_catalog removed tag(s): [tagA, tagC], now tagged with [tagB]
gcli tag remove --metalake demo_metalake --name Hive_catalog --tag tagA
tagB tagC
# Hive_catalog removed tag(s): [tagA, tagC], now tagged with []
```
---------
Co-authored-by: Justin Mclean <[email protected]>
Co-authored-by: Shaofeng Shi <[email protected]>
Co-authored-by: roryqi <[email protected]>
Co-authored-by: Xun <[email protected]>
Co-authored-by: JUN <[email protected]>
Co-authored-by: fsalhi2 <[email protected]>
Co-authored-by: Jerry Shao <[email protected]>
Co-authored-by: Cheng-Yi Shih
<[email protected]>
Co-authored-by: Qiming Teng <[email protected]>
Co-authored-by: FANNG <[email protected]>
Co-authored-by: Eric Chang <[email protected]>
Co-authored-by: cai can <[email protected]>
Co-authored-by: caican <[email protected]>
Co-authored-by: Qi Yu <[email protected]>
Co-authored-by: Jimmy Lee <[email protected]>
---
.../java/org/apache/gravitino/cli/FullName.java | 9 ++
.../apache/gravitino/cli/GravitinoCommandLine.java | 11 ++
.../apache/gravitino/cli/commands/UntagEntity.java | 4 +-
.../org/apache/gravitino/cli/TestFulllName.java | 15 +++
.../org/apache/gravitino/cli/TestTagCommands.java | 139 +++++++++++++++++++++
5 files changed, 177 insertions(+), 1 deletion(-)
diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java
index a2be2e52c..f2eef2a5a 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/FullName.java
@@ -148,6 +148,15 @@ public class FullName {
return null;
}
+ /**
+ * Are there any names that can be retrieved?
+ *
+ * @return True if the name exists, or false if it does not.
+ */
+ public Boolean hasName() {
+ return line.hasOption(GravitinoOptions.NAME);
+ }
+
/**
* Helper method to retrieve a specific part of the full name based on the
position of the part.
*
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 06ac09d67..7869dd97b 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
@@ -667,7 +667,14 @@ public class GravitinoCommandLine extends
TestableCommandLine {
if (propertySet != null && valueSet != null) {
newSetTagProperty(url, ignore, metalake, getOneTag(tags),
propertySet, valueSet).handle();
} else if (propertySet == null && valueSet == null) {
+ if (!name.hasName()) {
+ System.err.println(ErrorMessages.MISSING_NAME);
+ Main.exit(-1);
+ }
newTagEntity(url, ignore, metalake, name, tags).handle();
+ } else {
+ System.err.println("The set command only supports tag properties or
attaching tags.");
+ Main.exit(-1);
}
break;
@@ -681,6 +688,10 @@ public class GravitinoCommandLine extends
TestableCommandLine {
if (propertyRemove != null) {
newRemoveTagProperty(url, ignore, metalake, getOneTag(tags),
propertyRemove).handle();
} else {
+ if (!name.hasName()) {
+ System.err.println(ErrorMessages.MISSING_NAME);
+ Main.exit(-1);
+ }
newUntagEntity(url, ignore, metalake, name, tags).handle();
}
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UntagEntity.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UntagEntity.java
index 36b806056..8f4a4a9cf 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UntagEntity.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/UntagEntity.java
@@ -19,6 +19,7 @@
package org.apache.gravitino.cli.commands;
+import com.google.common.base.Joiner;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.Schema;
@@ -32,6 +33,7 @@ import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.rel.Table;
public class UntagEntity extends Command {
+ public static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls();
protected final String metalake;
protected final FullName name;
protected final String[] tags;
@@ -91,7 +93,7 @@ public class UntagEntity extends Command {
} catch (NoSuchCatalogException err) {
exitWithError(ErrorMessages.UNKNOWN_CATALOG);
} catch (NoSuchSchemaException err) {
- exitWithError(ErrorMessages.UNKNOWN_TABLE);
+ exitWithError(ErrorMessages.UNKNOWN_SCHEMA);
} catch (NoSuchTableException err) {
exitWithError(ErrorMessages.UNKNOWN_TABLE);
} catch (Exception exp) {
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestFulllName.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestFulllName.java
index 4b5e1fed7..e5ec92e10 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestFulllName.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestFulllName.java
@@ -111,6 +111,21 @@ public class TestFulllName {
assertNull(namePart);
}
+ @Test
+ public void hasPartName() throws ParseException {
+ String[] argsWithoutName = {"catalog", "details", "--metalake",
"metalake"};
+ CommandLine commandLineWithoutName = new DefaultParser().parse(options,
argsWithoutName);
+ FullName fullNameWithoutName = new FullName(commandLineWithoutName);
+ assertFalse(fullNameWithoutName.hasName());
+
+ String[] argsWithName = {
+ "catalog", "details", "--metalake", "metalake", "--name", "Hive_catalog"
+ };
+ CommandLine commandLineWithName = new DefaultParser().parse(options,
argsWithName);
+ FullName fullNameWithName = new FullName(commandLineWithName);
+ assertTrue(fullNameWithName.hasName());
+ }
+
@Test
public void hasPartNameMetalake() throws Exception {
String[] args = {"metalake", "details", "--metalake", "metalake"};
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestTagCommands.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestTagCommands.java
index 8d7ce17bd..74932ca87 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestTagCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestTagCommands.java
@@ -22,6 +22,7 @@ package org.apache.gravitino.cli;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.mockito.ArgumentMatchers.argThat;
+import static org.mockito.ArgumentMatchers.isNull;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.eq;
@@ -141,6 +142,30 @@ class TestTagCommands {
verify(mockCreate).handle();
}
+ @Test
+ void testCreateCommandWithoutTagOption() {
+ Main.useExit = false;
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(false);
+
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.CREATE));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ verify(commandLine, never())
+ .newCreateTags(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ isNull(),
+ isNull());
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(output, ErrorMessages.MISSING_TAG);
+ }
+
@Test
void testCreateTagsCommand() {
CreateTag mockCreate = mock(CreateTag.class);
@@ -272,6 +297,62 @@ class TestTagCommands {
verify(mockSetProperty).handle();
}
+ @Test
+ void testSetTagPropertyCommandWithoutPropertyOption() {
+ Main.useExit = false;
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
+ when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)).thenReturn(new
String[] {"tagA"});
+
when(mockCommandLine.hasOption(GravitinoOptions.PROPERTY)).thenReturn(false);
+ when(mockCommandLine.hasOption(GravitinoOptions.VALUE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.VALUE)).thenReturn("value");
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.SET));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ verify(commandLine, never())
+ .newSetTagProperty(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ eq("tagA"),
+ isNull(),
+ eq("value"));
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(output, "The set command only supports tag properties or
attaching tags.");
+ }
+
+ @Test
+ void testSetTagPropertyCommandWithoutValueOption() {
+ Main.useExit = false;
+
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
+ when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
+ when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)).thenReturn(new
String[] {"tagA"});
+
when(mockCommandLine.hasOption(GravitinoOptions.PROPERTY)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.PROPERTY)).thenReturn("property");
+ when(mockCommandLine.hasOption(GravitinoOptions.VALUE)).thenReturn(false);
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.SET));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ verify(commandLine, never())
+ .newSetTagProperty(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ eq("tagA"),
+ eq("property"),
+ isNull());
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(output, "The set command only supports tag properties or
attaching tags.");
+ }
+
@Test
void testSetMultipleTagPropertyCommandError() {
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
@@ -339,6 +420,7 @@ class TestTagCommands {
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(false);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.FORCE)).thenReturn(true);
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog.schema.table");
GravitinoCommandLine commandLine =
spy(
@@ -447,6 +529,32 @@ class TestTagCommands {
verify(mockTagEntity).handle();
}
+ @Test
+ void testTagEntityCommandWithoutName() {
+ 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);
+ when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
+ when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)).thenReturn(new
String[] {"tagA"});
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.SET));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ verify(commandLine, never())
+ .newTagEntity(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ isNull(),
+ argThat(
+ argument -> argument != null && argument.length > 0 &&
"tagA".equals(argument[0])));
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(output, ErrorMessages.MISSING_NAME);
+ }
+
@Test
void testTagsEntityCommand() {
TagEntity mockTagEntity = mock(TagEntity.class);
@@ -516,6 +624,37 @@ class TestTagCommands {
verify(mockUntagEntity).handle();
}
+ @Test
+ void testUntagEntityCommandWithoutName() {
+ 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);
+ when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true);
+ when(mockCommandLine.getOptionValues(GravitinoOptions.TAG))
+ .thenReturn(new String[] {"tagA", "tagB"});
+ GravitinoCommandLine commandLine =
+ spy(
+ new GravitinoCommandLine(
+ mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.REMOVE));
+
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
+ verify(commandLine, never())
+ .newUntagEntity(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ isNull(),
+ argThat(
+ argument ->
+ argument != null
+ && argument.length > 0
+ && "tagA".equals(argument[0])
+ && "tagB".equals(argument[1])));
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(output, ErrorMessages.MISSING_NAME);
+ }
+
@Test
void testUntagsEntityCommand() {
UntagEntity mockUntagEntity = mock(UntagEntity.class);
