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 6ad3d3bf9c [#6123] fix(CLI): Refactor the validation logic of tag and
role (#6127)
6ad3d3bf9c is described below
commit 6ad3d3bf9c6db1c67ad7ccfd24d5ebfd2d06454a
Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com>
AuthorDate: Tue Jan 7 14:37:40 2025 +0800
[#6123] fix(CLI): Refactor the validation logic of tag and role (#6127)
### What changes were proposed in this pull request?
Refactor the validation logic of the Tag and Role, meantime fix the test
case.
### Why are the changes needed?
Fix: #6123
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
ut + local test
**Role test**
```bash
gcli role grant -m demo_metalake --role admin
# Missing --privilege option.
gcli role revoke -m demo_metalake --role admin
# Missing --privilege option.
```
**Tag test**
```bash
gcli tag set -m demo_metalake
# Missing --name option.
gcli tag set -m demo_metalake --name catalog.schema.table --property
property --tag tagA
# Missing --value option.
gcli tag set -m demo_metalake --name catalog.schema.table --value value
--tag tagA
# Missing --property option.
gcli tag remove -m demo_metalake --tag tagA
# Missing --name option.
gcli tag remove -m demo_metalake
# Missing --name option.
gcli tag delete -m demo_metalake
# Missing --tag option.
gcli tag create -m demo_metalake
# Missing --tag option.
```
---
.../org/apache/gravitino/cli/ErrorMessages.java | 3 +
.../apache/gravitino/cli/GravitinoCommandLine.java | 81 +++----
.../apache/gravitino/cli/commands/CreateTag.java | 6 +
.../apache/gravitino/cli/commands/DeleteTag.java | 6 +
.../cli/commands/GrantPrivilegesToRole.java | 6 +
.../gravitino/cli/commands/RemoveAllTags.java | 6 +
.../cli/commands/RevokePrivilegesFromRole.java | 6 +
.../gravitino/cli/commands/SetTagProperty.java | 6 +
.../apache/gravitino/cli/commands/TagEntity.java | 6 +
.../apache/gravitino/cli/commands/UntagEntity.java | 6 +
.../org/apache/gravitino/cli/TestRoleCommands.java | 39 +++-
.../org/apache/gravitino/cli/TestTagCommands.java | 242 +++++++++++----------
12 files changed, 256 insertions(+), 157 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 abc6421d95..ecf1dbff4c 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
@@ -55,6 +55,7 @@ public class ErrorMessages {
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_PRIVILEGES = "Missing --privilege
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.";
@@ -63,6 +64,8 @@ public class ErrorMessages {
public static final String MISSING_USER = "Missing --user option.";
public static final String MISSING_VALUE = "Missing --value option.";
+ public static final String MULTIPLE_ROLE_COMMAND_ERROR =
+ "This command only supports one --role option.";
public static final String MULTIPLE_TAG_COMMAND_ERROR =
"This command only supports one --tag option.";
public static final String MISSING_PROVIDER = "Missing --provider option.";
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 07a1ecd5b7..442ec2d1c3 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
@@ -20,7 +20,6 @@
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;
@@ -643,12 +642,6 @@ public class GravitinoCommandLine extends
TestableCommandLine {
Command.setAuthenticationMode(auth, userName);
String[] tags = line.getOptionValues(GravitinoOptions.TAG);
- if (tags == null
- && !((CommandActions.REMOVE.equals(command) &&
line.hasOption(GravitinoOptions.FORCE))
- || CommandActions.LIST.equals(command))) {
- System.err.println(ErrorMessages.MISSING_TAG);
- Main.exit(-1);
- }
if (tags != null) {
tags = Arrays.stream(tags).distinct().toArray(String[]::new);
@@ -656,41 +649,36 @@ public class GravitinoCommandLine extends
TestableCommandLine {
switch (command) {
case CommandActions.DETAILS:
- newTagDetails(url, ignore, metalake, getOneTag(tags)).handle();
+ newTagDetails(url, ignore, metalake,
getOneTag(tags)).validate().handle();
break;
case CommandActions.LIST:
if (!name.hasCatalogName()) {
- newListTags(url, ignore, metalake).handle();
+ newListTags(url, ignore, metalake).validate().handle();
} else {
- newListEntityTags(url, ignore, metalake, name).handle();
+ newListEntityTags(url, ignore, metalake, name).validate().handle();
}
break;
case CommandActions.CREATE:
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
- newCreateTags(url, ignore, metalake, tags, comment).handle();
+ newCreateTags(url, ignore, metalake, tags,
comment).validate().handle();
break;
case CommandActions.DELETE:
boolean forceDelete = line.hasOption(GravitinoOptions.FORCE);
- newDeleteTag(url, ignore, forceDelete, metalake, tags).handle();
+ newDeleteTag(url, ignore, forceDelete, metalake,
tags).validate().handle();
break;
case CommandActions.SET:
String propertySet = line.getOptionValue(GravitinoOptions.PROPERTY);
String valueSet = line.getOptionValue(GravitinoOptions.VALUE);
- 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();
+ if (propertySet == null && valueSet == null) {
+ newTagEntity(url, ignore, metalake, name, tags).validate().handle();
} else {
- System.err.println(ErrorMessages.INVALID_SET_COMMAND);
- Main.exit(-1);
+ newSetTagProperty(url, ignore, metalake, getOneTag(tags),
propertySet, valueSet)
+ .validate()
+ .handle();
}
break;
@@ -698,33 +686,33 @@ public class GravitinoCommandLine extends
TestableCommandLine {
boolean isTag = line.hasOption(GravitinoOptions.TAG);
if (!isTag) {
boolean forceRemove = line.hasOption(GravitinoOptions.FORCE);
- newRemoveAllTags(url, ignore, metalake, name, forceRemove).handle();
+ newRemoveAllTags(url, ignore, metalake, name,
forceRemove).validate().handle();
} else {
String propertyRemove =
line.getOptionValue(GravitinoOptions.PROPERTY);
if (propertyRemove != null) {
- newRemoveTagProperty(url, ignore, metalake, getOneTag(tags),
propertyRemove).handle();
+ newRemoveTagProperty(url, ignore, metalake, getOneTag(tags),
propertyRemove)
+ .validate()
+ .handle();
} else {
- if (!name.hasName()) {
- System.err.println(ErrorMessages.MISSING_NAME);
- Main.exit(-1);
- }
- newUntagEntity(url, ignore, metalake, name, tags).handle();
+ newUntagEntity(url, ignore, metalake, name,
tags).validate().handle();
}
}
break;
case CommandActions.PROPERTIES:
- newListTagProperties(url, ignore, metalake, getOneTag(tags)).handle();
+ newListTagProperties(url, ignore, metalake,
getOneTag(tags)).validate().handle();
break;
case CommandActions.UPDATE:
if (line.hasOption(GravitinoOptions.COMMENT)) {
String updateComment = line.getOptionValue(GravitinoOptions.COMMENT);
- newUpdateTagComment(url, ignore, metalake, getOneTag(tags),
updateComment).handle();
+ newUpdateTagComment(url, ignore, metalake, getOneTag(tags),
updateComment)
+ .validate()
+ .handle();
}
if (line.hasOption(GravitinoOptions.RENAME)) {
String newName = line.getOptionValue(GravitinoOptions.RENAME);
- newUpdateTagName(url, ignore, metalake, getOneTag(tags),
newName).handle();
+ newUpdateTagName(url, ignore, metalake, getOneTag(tags),
newName).validate().handle();
}
break;
@@ -736,7 +724,7 @@ public class GravitinoCommandLine extends
TestableCommandLine {
}
private String getOneTag(String[] tags) {
- if (tags.length > 1) {
+ if (tags == null || tags.length > 1) {
System.err.println(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR);
Main.exit(-1);
}
@@ -767,34 +755,34 @@ public class GravitinoCommandLine extends
TestableCommandLine {
switch (command) {
case CommandActions.DETAILS:
if (line.hasOption(GravitinoOptions.AUDIT)) {
- newRoleAudit(url, ignore, metalake, getOneRole(roles,
CommandActions.DETAILS)).handle();
+ newRoleAudit(url, ignore, metalake,
getOneRole(roles)).validate().handle();
} else {
- newRoleDetails(url, ignore, metalake, getOneRole(roles,
CommandActions.DETAILS)).handle();
+ newRoleDetails(url, ignore, metalake,
getOneRole(roles)).validate().handle();
}
break;
case CommandActions.LIST:
- newListRoles(url, ignore, metalake).handle();
+ newListRoles(url, ignore, metalake).validate().handle();
break;
case CommandActions.CREATE:
- newCreateRole(url, ignore, metalake, roles).handle();
+ newCreateRole(url, ignore, metalake, roles).validate().handle();
break;
case CommandActions.DELETE:
boolean forceDelete = line.hasOption(GravitinoOptions.FORCE);
- newDeleteRole(url, ignore, forceDelete, metalake, roles).handle();
+ newDeleteRole(url, ignore, forceDelete, metalake,
roles).validate().handle();
break;
case CommandActions.GRANT:
- newGrantPrivilegesToRole(
- url, ignore, metalake, getOneRole(roles,
CommandActions.GRANT), name, privileges)
+ newGrantPrivilegesToRole(url, ignore, metalake, getOneRole(roles),
name, privileges)
+ .validate()
.handle();
break;
case CommandActions.REVOKE:
- newRevokePrivilegesFromRole(
- url, ignore, metalake, getOneRole(roles,
CommandActions.REMOVE), name, privileges)
+ newRevokePrivilegesFromRole(url, ignore, metalake, getOneRole(roles),
name, privileges)
+ .validate()
.handle();
break;
@@ -805,9 +793,12 @@ public class GravitinoCommandLine extends
TestableCommandLine {
}
}
- private String getOneRole(String[] roles, String command) {
- Preconditions.checkArgument(
- roles.length == 1, command + " requires only one role, but multiple
are currently passed.");
+ private String getOneRole(String[] roles) {
+ if (roles == null || roles.length != 1) {
+ System.err.println(ErrorMessages.MULTIPLE_ROLE_COMMAND_ERROR);
+ Main.exit(-1);
+ }
+
return roles[0];
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java
index 87ab0da779..dabf34c8b1 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateTag.java
@@ -103,4 +103,10 @@ public class CreateTag extends Command {
System.out.println("Tags " + String.join(",", remaining) + " not
created");
}
}
+
+ @Override
+ public Command validate() {
+ if (tags == null) exitWithError(ErrorMessages.MISSING_TAG);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTag.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTag.java
index 1e05292c82..26919e06ac 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTag.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteTag.java
@@ -116,4 +116,10 @@ public class DeleteTag extends Command {
System.out.println("Tag " + tags[0] + " not deleted.");
}
}
+
+ @Override
+ public Command validate() {
+ if (tags == null) exitWithError(ErrorMessages.MISSING_TAG);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
index 584e073bea..8630282ea6 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/GrantPrivilegesToRole.java
@@ -103,4 +103,10 @@ public class GrantPrivilegesToRole extends MetadataCommand
{
String all = String.join(",", privileges);
System.out.println(role + " granted " + all + " on " + entity.getName());
}
+
+ @Override
+ public Command validate() {
+ if (privileges == null) exitWithError(ErrorMessages.MISSING_PRIVILEGES);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllTags.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllTags.java
index a7aa3748a1..5221100a8e 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllTags.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveAllTags.java
@@ -118,4 +118,10 @@ public class RemoveAllTags extends Command {
System.out.println(entity + " has no tags");
}
}
+
+ @Override
+ public Command validate() {
+ if (name == null || !name.hasName())
exitWithError(ErrorMessages.MISSING_NAME);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
index a62e977a2f..3bfa7cd452 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokePrivilegesFromRole.java
@@ -103,4 +103,10 @@ public class RevokePrivilegesFromRole extends
MetadataCommand {
String all = String.join(",", privileges);
System.out.println(role + " revoked " + all + " on " + entity.getName());
}
+
+ @Override
+ public Command validate() {
+ if (privileges == null) exitWithError(ErrorMessages.MISSING_PRIVILEGES);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTagProperty.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTagProperty.java
index b5b46b59a7..da7a267b8d 100644
---
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTagProperty.java
+++
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTagProperty.java
@@ -74,4 +74,10 @@ public class SetTagProperty extends Command {
System.out.println(tag + " property set.");
}
+
+ @Override
+ public Command validate() {
+ validatePropertyAndValue(property, value);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/TagEntity.java
b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/TagEntity.java
index 7bc8ec3764..4a06918850 100644
--- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/TagEntity.java
+++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/TagEntity.java
@@ -105,4 +105,10 @@ public class TagEntity extends Command {
System.out.println(entity + " now tagged with " + all);
}
+
+ @Override
+ public Command validate() {
+ if (name == null || !name.hasName())
exitWithError(ErrorMessages.MISSING_NAME);
+ return super.validate();
+ }
}
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 8f4a4a9cf0..3503d5eb7b 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
@@ -113,4 +113,10 @@ public class UntagEntity extends Command {
System.out.println(entity + " removed tag " + tags[0].toString() + " now
tagged with " + all);
}
}
+
+ @Override
+ public Command validate() {
+ if (name == null || !name.hasName())
exitWithError(ErrorMessages.MISSING_NAME);
+ return super.validate();
+ }
}
diff --git
a/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
b/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
index 0e671067e3..529979582f 100644
--- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
+++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestRoleCommands.java
@@ -80,6 +80,7 @@ class TestRoleCommands {
doReturn(mockList)
.when(commandLine)
.newListRoles(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo");
+ doReturn(mockList).when(mockList).validate();
commandLine.handleCommandLine();
verify(mockList).handle();
}
@@ -98,12 +99,14 @@ class TestRoleCommands {
doReturn(mockDetails)
.when(commandLine)
.newRoleDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "admin");
+ doReturn(mockDetails).when(mockDetails).validate();
commandLine.handleCommandLine();
verify(mockDetails).handle();
}
@Test
void testRoleDetailsCommandWithMultipleRoles() {
+ Main.useExit = false;
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.ROLE)).thenReturn(true);
@@ -114,7 +117,7 @@ class TestRoleCommands {
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.ROLE,
CommandActions.DETAILS));
- assertThrows(IllegalArgumentException.class,
commandLine::handleCommandLine);
+ assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newRoleDetails(
eq(GravitinoCommandLine.DEFAULT_URL), eq(false),
eq("metalake_demo"), any());
@@ -135,6 +138,7 @@ class TestRoleCommands {
doReturn(mockAudit)
.when(commandLine)
.newRoleAudit(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "group");
+ doReturn(mockAudit).when(mockAudit).validate();
commandLine.handleCommandLine();
verify(mockAudit).handle();
}
@@ -154,6 +158,7 @@ class TestRoleCommands {
.when(commandLine)
.newCreateRole(
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", new
String[] {"admin"});
+ doReturn(mockCreate).when(mockCreate).validate();
commandLine.handleCommandLine();
verify(mockCreate).handle();
}
@@ -178,6 +183,7 @@ class TestRoleCommands {
eq(false),
eq("metalake_demo"),
eq(new String[] {"admin", "engineer", "scientist"}));
+ doReturn(mockCreate).when(mockCreate).validate();
commandLine.handleCommandLine();
verify(mockCreate).handle();
}
@@ -201,6 +207,7 @@ class TestRoleCommands {
false,
"metalake_demo",
new String[] {"admin"});
+ doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
@@ -227,6 +234,7 @@ class TestRoleCommands {
eq(false),
eq("metalake_demo"),
eq(new String[] {"admin", "engineer", "scientist"}));
+ doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
@@ -247,6 +255,7 @@ class TestRoleCommands {
.when(commandLine)
.newDeleteRole(
GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo",
new String[] {"admin"});
+ doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
@@ -276,10 +285,24 @@ class TestRoleCommands {
eq("admin"),
any(),
eq(privileges));
+ doReturn(mockGrant).when(mockGrant).validate();
commandLine.handleCommandLine();
verify(mockGrant).handle();
}
+ @Test
+ void testGrantPrivilegesToRoleWithoutPrivileges() {
+ Main.useExit = false;
+ GrantPrivilegesToRole spyGrantRole =
+ spy(
+ new GrantPrivilegesToRole(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
"admin", null, null));
+ assertThrows(RuntimeException.class, spyGrantRole::validate);
+ verify(spyGrantRole, never()).handle();
+ String errOutput = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(ErrorMessages.MISSING_PRIVILEGES, errOutput);
+ }
+
@Test
void testRevokePrivilegesFromRole() {
RevokePrivilegesFromRole mockRevoke = mock(RevokePrivilegesFromRole.class);
@@ -305,10 +328,24 @@ class TestRoleCommands {
eq("admin"),
any(),
eq(privileges));
+ doReturn(mockRevoke).when(mockRevoke).validate();
commandLine.handleCommandLine();
verify(mockRevoke).handle();
}
+ @Test
+ void testRevokePrivilegesFromRoleWithoutPrivileges() {
+ Main.useExit = false;
+ RevokePrivilegesFromRole spyGrantRole =
+ spy(
+ new RevokePrivilegesFromRole(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
"admin", null, null));
+ assertThrows(RuntimeException.class, spyGrantRole::validate);
+ verify(spyGrantRole, never()).handle();
+ String errOutput = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(ErrorMessages.MISSING_PRIVILEGES, errOutput);
+ }
+
@Test
void testDeleteRoleCommandWithoutRole() {
Main.useExit = false;
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 d3b0c8bfe1..a94ccee7da 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,7 +22,6 @@ 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;
@@ -95,6 +94,7 @@ class TestTagCommands {
doReturn(mockList)
.when(commandLine)
.newListTags(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo");
+ doReturn(mockList).when(mockList).validate();
commandLine.handleCommandLine();
verify(mockList).handle();
}
@@ -113,6 +113,7 @@ class TestTagCommands {
doReturn(mockDetails)
.when(commandLine)
.newTagDetails(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "tagA");
+ doReturn(mockDetails).when(mockDetails).validate();
commandLine.handleCommandLine();
verify(mockDetails).handle();
}
@@ -157,6 +158,7 @@ class TestTagCommands {
"metalake_demo",
new String[] {"tagA"},
"comment");
+ doReturn(mockCreate).when(mockCreate).validate();
commandLine.handleCommandLine();
verify(mockCreate).handle();
}
@@ -164,25 +166,15 @@ class TestTagCommands {
@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 =
+ CreateTag spyCreate =
spy(
- new GravitinoCommandLine(
- mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.CREATE));
+ new CreateTag(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
null, "comment"));
- assertThrows(RuntimeException.class, commandLine::handleCommandLine);
- verify(commandLine, never())
- .newCreateTags(
- eq(GravitinoCommandLine.DEFAULT_URL),
- eq(false),
- eq("metalake_demo"),
- isNull(),
- isNull());
+ assertThrows(RuntimeException.class, spyCreate::validate);
+ verify(spyCreate, never()).handle();
String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
- assertEquals(output, ErrorMessages.MISSING_TAG);
+ assertEquals(ErrorMessages.MISSING_TAG, output);
}
@Test
@@ -202,11 +194,16 @@ class TestTagCommands {
doReturn(mockCreate)
.when(commandLine)
.newCreateTags(
- GravitinoCommandLine.DEFAULT_URL,
- false,
- "metalake_demo",
- new String[] {"tagA", "tagB"},
- "comment");
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ argThat(
+ argument ->
+ argument.length == 2
+ && argument[0].equals("tagA")
+ && argument[1].equals("tagB")),
+ eq("comment"));
+ doReturn(mockCreate).when(mockCreate).validate();
commandLine.handleCommandLine();
verify(mockCreate).handle();
}
@@ -226,6 +223,7 @@ class TestTagCommands {
.when(commandLine)
.newCreateTags(
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", new
String[] {"tagA"}, null);
+ doReturn(mockCreate).when(mockCreate).validate();
commandLine.handleCommandLine();
verify(mockCreate).handle();
}
@@ -245,6 +243,7 @@ class TestTagCommands {
.when(commandLine)
.newDeleteTag(
GravitinoCommandLine.DEFAULT_URL, false, false, "metalake_demo",
new String[] {"tagA"});
+ doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
@@ -269,6 +268,7 @@ class TestTagCommands {
false,
"metalake_demo",
new String[] {"tagA", "tagB"});
+ doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
@@ -289,6 +289,7 @@ class TestTagCommands {
.when(commandLine)
.newDeleteTag(
GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo",
new String[] {"tagA"});
+ doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
@@ -312,64 +313,53 @@ class TestTagCommands {
.when(commandLine)
.newSetTagProperty(
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "tagA",
"property", "value");
+ doReturn(mockSetProperty).when(mockSetProperty).validate();
commandLine.handleCommandLine();
verify(mockSetProperty).handle();
}
@Test
- void testSetTagPropertyCommandWithoutPropertyOption() {
+ void testSetTagPropertyCommandWithoutPropertyAndValue() {
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 =
+ SetTagProperty spySetProperty =
spy(
- new GravitinoCommandLine(
- mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.SET));
+ new SetTagProperty(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
"tagA", null, null));
+ assertThrows(RuntimeException.class, spySetProperty::validate);
+ verify(spySetProperty, never()).handle();
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(output, ErrorMessages.MISSING_PROPERTY_AND_VALUE);
+ }
- assertThrows(RuntimeException.class, commandLine::handleCommandLine);
- verify(commandLine, never())
- .newSetTagProperty(
- eq(GravitinoCommandLine.DEFAULT_URL),
- eq(false),
- eq("metalake_demo"),
- eq("tagA"),
- isNull(),
- eq("value"));
+ @Test
+ void testSetTagPropertyCommandWithoutPropertyOption() {
+ Main.useExit = false;
+ SetTagProperty spySetProperty =
+ spy(
+ new SetTagProperty(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
"tagA", null, "value"));
+ assertThrows(RuntimeException.class, spySetProperty::validate);
+ verify(spySetProperty, never()).handle();
String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
- assertEquals(output, ErrorMessages.INVALID_SET_COMMAND);
+ assertEquals(output, ErrorMessages.MISSING_PROPERTY);
}
@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 =
+ SetTagProperty spySetProperty =
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());
+ new SetTagProperty(
+ GravitinoCommandLine.DEFAULT_URL,
+ false,
+ "metalake_demo",
+ "tagA",
+ "property",
+ null));
+ assertThrows(RuntimeException.class, spySetProperty::validate);
+ verify(spySetProperty, never()).handle();
String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
- assertEquals(output, ErrorMessages.INVALID_SET_COMMAND);
+ assertEquals(output, ErrorMessages.MISSING_VALUE);
}
@Test
@@ -418,6 +408,7 @@ class TestTagCommands {
.when(commandLine)
.newRemoveTagProperty(
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "tagA",
"property");
+ doReturn(mockRemoveProperty).when(mockRemoveProperty).validate();
commandLine.handleCommandLine();
verify(mockRemoveProperty).handle();
}
@@ -463,6 +454,7 @@ class TestTagCommands {
doReturn(mockListProperties)
.when(commandLine)
.newListTagProperties(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "tagA");
+ doReturn(mockListProperties).when(mockListProperties).validate();
commandLine.handleCommandLine();
verify(mockListProperties).handle();
}
@@ -488,6 +480,7 @@ class TestTagCommands {
eq("metalake_demo"),
any(FullName.class),
eq(true));
+ doReturn(mockRemoveAllTags).when(mockRemoveAllTags).validate();
commandLine.handleCommandLine();
verify(mockRemoveAllTags).handle();
}
@@ -509,6 +502,7 @@ class TestTagCommands {
.when(commandLine)
.newUpdateTagComment(
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "tagA",
"new comment");
+ doReturn(mockUpdateComment).when(mockUpdateComment).validate();
commandLine.handleCommandLine();
verify(mockUpdateComment).handle();
}
@@ -556,6 +550,7 @@ class TestTagCommands {
doReturn(mockUpdateName)
.when(commandLine)
.newUpdateTagName(GravitinoCommandLine.DEFAULT_URL, false,
"metalake_demo", "tagA", "tagB");
+ doReturn(mockUpdateName).when(mockUpdateName).validate();
commandLine.handleCommandLine();
verify(mockUpdateName).handle();
}
@@ -602,6 +597,7 @@ class TestTagCommands {
.when(commandLine)
.newListEntityTags(
eq(GravitinoCommandLine.DEFAULT_URL), eq(false),
eq("metalake_demo"), any());
+ doReturn(mockListTags).when(mockListTags).validate();
commandLine.handleCommandLine();
verify(mockListTags).handle();
}
@@ -633,6 +629,7 @@ class TestTagCommands {
return argument != null && argument.length > 0 &&
"tagA".equals(argument[0]);
}
}));
+ doReturn(mockTagEntity).when(mockTagEntity).validate();
commandLine.handleCommandLine();
verify(mockTagEntity).handle();
}
@@ -640,27 +637,19 @@ class TestTagCommands {
@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 =
+ TagEntity spyTagEntity =
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])));
+ new TagEntity(
+ GravitinoCommandLine.DEFAULT_URL,
+ false,
+ "metalake_demo",
+ null,
+ new String[] {"tagA"}));
+
+ assertThrows(RuntimeException.class, spyTagEntity::validate);
+ verify(spyTagEntity, never()).handle();
String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
- assertEquals(output, ErrorMessages.MISSING_NAME);
+ assertEquals(ErrorMessages.MISSING_NAME, output);
}
@Test
@@ -694,6 +683,7 @@ class TestTagCommands {
&& "tagB".equals(argument[1]);
}
}));
+ doReturn(mockTagEntity).when(mockTagEntity).validate();
commandLine.handleCommandLine();
verify(mockTagEntity).handle();
}
@@ -728,6 +718,7 @@ class TestTagCommands {
return argument != null && argument.length > 0 &&
"tagA".equals(argument[0]);
}
}));
+ doReturn(mockUntagEntity).when(mockUntagEntity).validate();
commandLine.handleCommandLine();
verify(mockUntagEntity).handle();
}
@@ -735,32 +726,19 @@ class TestTagCommands {
@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 =
+ UntagEntity spyUntagEntity =
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])));
+ new UntagEntity(
+ GravitinoCommandLine.DEFAULT_URL,
+ false,
+ "metalake_demo",
+ null,
+ new String[] {"tagA"}));
+
+ assertThrows(RuntimeException.class, spyUntagEntity::validate);
+ verify(spyUntagEntity, never()).handle();
String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
- assertEquals(output, ErrorMessages.MISSING_NAME);
+ assertEquals(ErrorMessages.MISSING_NAME, output);
}
@Test
@@ -796,6 +774,7 @@ class TestTagCommands {
&& "tagB".equals(argument[1]);
}
}));
+ doReturn(mockUntagEntity).when(mockUntagEntity).validate();
commandLine.handleCommandLine();
verify(mockUntagEntity).handle();
}
@@ -803,18 +782,59 @@ class TestTagCommands {
@Test
void testDeleteTagCommandWithoutTagOption() {
Main.useExit = false;
+ DeleteTag spyDeleteTag =
+ spy(new DeleteTag(GravitinoCommandLine.DEFAULT_URL, false, false,
"metalake", null));
+
+ assertThrows(RuntimeException.class, spyDeleteTag::validate);
+ verify(spyDeleteTag, never()).handle();
+ String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
+ assertEquals(ErrorMessages.MISSING_TAG, output);
+ }
+
+ @Test
+ void testRemoveAllTagsCommand() {
+ Main.useExit = false;
+ RemoveAllTags mockRemoveAllTags = mock(RemoveAllTags.class);
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(false);
+ when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true);
+
when(mockCommandLine.getOptionValue(GravitinoOptions.NAME)).thenReturn("catalog.schema.table");
+ when(mockCommandLine.hasOption(GravitinoOptions.FORCE)).thenReturn(true);
GravitinoCommandLine commandLine =
spy(
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.TAG,
CommandActions.REMOVE));
- assertThrows(RuntimeException.class, commandLine::handleCommandLine);
- verify(commandLine, never())
- .newDeleteTag(GravitinoCommandLine.DEFAULT_URL, false, false,
"metalake", null);
+ doReturn(mockRemoveAllTags)
+ .when(commandLine)
+ .newRemoveAllTags(
+ eq(GravitinoCommandLine.DEFAULT_URL),
+ eq(false),
+ eq("metalake_demo"),
+ argThat(
+ argument ->
+ argument != null
+ && "catalog".equals(argument.getCatalogName())
+ && "schema".equals(argument.getSchemaName())
+ && "table".equals(argument.getTableName())),
+ eq(true));
+ doReturn(mockRemoveAllTags).when(mockRemoveAllTags).validate();
+ commandLine.handleCommandLine();
+ verify(mockRemoveAllTags).handle();
+ }
+
+ @Test
+ void testRemoveAllTagsCommandWithoutName() {
+ Main.useExit = false;
+ RemoveAllTags spyRemoveAllTags =
+ spy(
+ new RemoveAllTags(
+ GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo",
null, false));
+
+ assertThrows(RuntimeException.class, spyRemoveAllTags::validate);
+ verify(spyRemoveAllTags, never()).handle();
String output = new String(errContent.toByteArray(),
StandardCharsets.UTF_8).trim();
- assertEquals(output, ErrorMessages.MISSING_TAG);
+ assertEquals(ErrorMessages.MISSING_NAME, output);
}
}
