Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#6121] fix(CLI): Refactor the validation logic of topic and fileset #6122

Merged
merged 2 commits into from
Jan 7, 2025
Merged

[#6121] fix(CLI): Refactor the validation logic of topic and fileset #6122

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d2dcf58
[#6121] fix(CLI): Refactor the validation logic of topic and fileset
Abyss-lord Jan 6, 2025
2a9843d
[#6121] fix(CLI): Refactor the validation logic of topic and fileset
Abyss-lord Jan 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 29 additions & 12 deletions clients/cli/src/main/java/org/apache/gravitino/cli/GravitinoCommandLine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1015,7 +1015,7 @@ private void handleTopicCommand() {

if (CommandActions.LIST.equals(command)) {
checkEntities(missingEntities);
newListTopics(url, ignore, metalake, catalog, schema).handle();
newListTopics(url, ignore, metalake, catalog, schema).validate().handle();
return;
}

Expand All @@ -1025,28 +1025,32 @@ private void handleTopicCommand() {

switch (command) {
case CommandActions.DETAILS:
newTopicDetails(url, ignore, metalake, catalog, schema, topic).handle();
newTopicDetails(url, ignore, metalake, catalog, schema, topic).validate().handle();
break;

case CommandActions.CREATE:
{
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
newCreateTopic(url, ignore, metalake, catalog, schema, topic, comment).handle();
newCreateTopic(url, ignore, metalake, catalog, schema, topic, comment)
.validate()
.handle();
break;
}

case CommandActions.DELETE:
{
boolean force = line.hasOption(GravitinoOptions.FORCE);
newDeleteTopic(url, ignore, force, metalake, catalog, schema, topic).handle();
newDeleteTopic(url, ignore, force, metalake, catalog, schema, topic).validate().handle();
break;
}

case CommandActions.UPDATE:
{
if (line.hasOption(GravitinoOptions.COMMENT)) {
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
newUpdateTopicComment(url, ignore, metalake, catalog, schema, topic, comment).handle();
newUpdateTopicComment(url, ignore, metalake, catalog, schema, topic, comment)
.validate()
.handle();
}
break;
}
Expand All @@ -1056,19 +1060,22 @@ private void handleTopicCommand() {
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
String value = line.getOptionValue(GravitinoOptions.VALUE);
newSetTopicProperty(url, ignore, metalake, catalog, schema, topic, property, value)
.validate()
.handle();
break;
}

case CommandActions.REMOVE:
{
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
newRemoveTopicProperty(url, ignore, metalake, catalog, schema, topic, property).handle();
newRemoveTopicProperty(url, ignore, metalake, catalog, schema, topic, property)
.validate()
.handle();
break;
}

case CommandActions.PROPERTIES:
newListTopicProperties(url, ignore, metalake, catalog, schema, topic).handle();
newListTopicProperties(url, ignore, metalake, catalog, schema, topic).validate().handle();
break;

default:
Expand Down Expand Up @@ -1098,7 +1105,7 @@ private void handleFilesetCommand() {
// Handle CommandActions.LIST action separately as it doesn't require the `fileset`
if (CommandActions.LIST.equals(command)) {
checkEntities(missingEntities);
newListFilesets(url, ignore, metalake, catalog, schema).handle();
newListFilesets(url, ignore, metalake, catalog, schema).validate().handle();
return;
}

Expand All @@ -1108,7 +1115,7 @@ private void handleFilesetCommand() {

switch (command) {
case CommandActions.DETAILS:
newFilesetDetails(url, ignore, metalake, catalog, schema, fileset).handle();
newFilesetDetails(url, ignore, metalake, catalog, schema, fileset).validate().handle();
break;

case CommandActions.CREATE:
Expand All @@ -1117,14 +1124,17 @@ private void handleFilesetCommand() {
String[] properties = line.getOptionValues(CommandActions.PROPERTIES);
Map<String, String> propertyMap = new Properties().parse(properties);
newCreateFileset(url, ignore, metalake, catalog, schema, fileset, comment, propertyMap)
.validate()
.handle();
break;
}

case CommandActions.DELETE:
{
boolean force = line.hasOption(GravitinoOptions.FORCE);
newDeleteFileset(url, ignore, force, metalake, catalog, schema, fileset).handle();
newDeleteFileset(url, ignore, force, metalake, catalog, schema, fileset)
.validate()
.handle();
break;
}

Expand All @@ -1133,6 +1143,7 @@ private void handleFilesetCommand() {
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
String value = line.getOptionValue(GravitinoOptions.VALUE);
newSetFilesetProperty(url, ignore, metalake, catalog, schema, fileset, property, value)
.validate()
.handle();
break;
}
Expand All @@ -1141,24 +1152,30 @@ private void handleFilesetCommand() {
{
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
newRemoveFilesetProperty(url, ignore, metalake, catalog, schema, fileset, property)
.validate()
.handle();
break;
}

case CommandActions.PROPERTIES:
newListFilesetProperties(url, ignore, metalake, catalog, schema, fileset).handle();
newListFilesetProperties(url, ignore, metalake, catalog, schema, fileset)
.validate()
.handle();
break;

case CommandActions.UPDATE:
{
if (line.hasOption(GravitinoOptions.COMMENT)) {
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
newUpdateFilesetComment(url, ignore, metalake, catalog, schema, fileset, comment)
.validate()
.handle();
}
if (line.hasOption(GravitinoOptions.RENAME)) {
String newName = line.getOptionValue(GravitinoOptions.RENAME);
newUpdateFilesetName(url, ignore, metalake, catalog, schema, fileset, newName).handle();
newUpdateFilesetName(url, ignore, metalake, catalog, schema, fileset, newName)
.validate()
.handle();
}
break;
}
Expand Down
6 changes: 6 additions & 0 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveFilesetProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,4 +86,10 @@ public void handle() {

System.out.println(property + " property removed.");
}

@Override
public Command validate() {
validateProperty(property);
return super.validate();
}
}
6 changes: 6 additions & 0 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/RemoveTopicProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,4 +87,10 @@ public void handle() {

System.out.println(property + " property removed.");
}

@Override
public Command validate() {
validateProperty(property);
return super.validate();
}
}
6 changes: 6 additions & 0 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetFilesetProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,4 +90,10 @@ public void handle() {

System.out.println(schema + " property set.");
}

@Override
public Command validate() {
validatePropertyAndValue(property, value);
return super.validate();
}
}
6 changes: 6 additions & 0 deletions clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetTopicProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,10 @@ public void handle() {

System.out.println(property + " property set.");
}

@Override
public Command validate() {
validatePropertyAndValue(property, value);
return super.validate();
}
}
93 changes: 93 additions & 0 deletions clients/cli/src/test/java/org/apache/gravitino/cli/TestFilesetCommands.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ void testListFilesetsCommand() {
.when(commandLine)
.newListFilesets(
GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "catalog", "schema");
doReturn(mockList).when(mockList).validate();
commandLine.handleCommandLine();
verify(mockList).handle();
}
Expand All @@ -117,6 +118,7 @@ void testFilesetDetailsCommand() {
"catalog",
"schema",
"fileset");
doReturn(mockDetails).when(mockDetails).validate();
commandLine.handleCommandLine();
verify(mockDetails).handle();
}
Expand Down Expand Up @@ -147,6 +149,7 @@ void testCreateFilesetCommand() {
eq("fileset"),
eq("comment"),
any());
doReturn(mockCreate).when(mockCreate).validate();
commandLine.handleCommandLine();
verify(mockCreate).handle();
}
Expand All @@ -173,6 +176,7 @@ void testDeleteFilesetCommand() {
"catalog",
"schema",
"fileset");
doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
Expand Down Expand Up @@ -200,6 +204,7 @@ void testDeleteFilesetForceCommand() {
"catalog",
"schema",
"fileset");
doReturn(mockDelete).when(mockDelete).validate();
commandLine.handleCommandLine();
verify(mockDelete).handle();
}
Expand Down Expand Up @@ -229,6 +234,7 @@ void testUpdateFilesetCommentCommand() {
"schema",
"fileset",
"new_comment");
doReturn(mockUpdateComment).when(mockUpdateComment).validate();
commandLine.handleCommandLine();
verify(mockUpdateComment).handle();
}
Expand Down Expand Up @@ -258,6 +264,7 @@ void testUpdateFilesetNameCommand() {
"schema",
"fileset",
"new_name");
doReturn(mockUpdateName).when(mockUpdateName).validate();
commandLine.handleCommandLine();
verify(mockUpdateName).handle();
}
Expand All @@ -284,6 +291,7 @@ void testListFilesetPropertiesCommand() {
"catalog",
"schema",
"fileset");
doReturn(mockListProperties).when(mockListProperties).validate();
commandLine.handleCommandLine();
verify(mockListProperties).handle();
}
Expand Down Expand Up @@ -316,10 +324,74 @@ void testSetFilesetPropertyCommand() {
"fileset",
"property",
"value");
doReturn(mockSetProperties).when(mockSetProperties).validate();
commandLine.handleCommandLine();
verify(mockSetProperties).handle();
}

@Test
void testSetFilesetPropertyCommandWithoutPropertyAndValue() {
Main.useExit = false;
SetFilesetProperty spySetProperty =
spy(
new SetFilesetProperty(
GravitinoCommandLine.DEFAULT_URL,
false,
"metalake_demo",
"catalog",
"schema",
"fileset",
null,
null));

assertThrows(RuntimeException.class, spySetProperty::validate);
verify(spySetProperty, never()).handle();
String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(ErrorMessages.MISSING_PROPERTY_AND_VALUE, errOutput);
}

@Test
void testSetFilesetPropertyCommandWithoutProperty() {
Main.useExit = false;
SetFilesetProperty spySetProperty =
spy(
new SetFilesetProperty(
GravitinoCommandLine.DEFAULT_URL,
false,
"metalake_demo",
"catalog",
"schema",
"fileset",
null,
"value"));

assertThrows(RuntimeException.class, spySetProperty::validate);
verify(spySetProperty, never()).handle();
String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(ErrorMessages.MISSING_PROPERTY, errOutput);
}

@Test
void testSetFilesetPropertyCommandWithoutValue() {
Main.useExit = false;
SetFilesetProperty spySetProperty =
spy(
new SetFilesetProperty(
GravitinoCommandLine.DEFAULT_URL,
false,
"metalake_demo",
"catalog",
"schema",
"fileset",
"property",
null));

assertThrows(RuntimeException.class, spySetProperty::validate);
verify(spySetProperty, never()).handle();
String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(ErrorMessages.MISSING_VALUE, errOutput);
}

@Test
void testRemoveFilesetPropertyCommand() {
RemoveFilesetProperty mockSetProperties = mock(RemoveFilesetProperty.class);
Expand All @@ -345,10 +417,31 @@ void testRemoveFilesetPropertyCommand() {
"schema",
"fileset",
"property");
doReturn(mockSetProperties).when(mockSetProperties).validate();
commandLine.handleCommandLine();
verify(mockSetProperties).handle();
}

@Test
void testRemoveFilesetPropertyCommandWithoutProperty() {
Main.useExit = false;
RemoveFilesetProperty spyRemoveProperty =
spy(
new RemoveFilesetProperty(
GravitinoCommandLine.DEFAULT_URL,
false,
"metalake_demo",
"catalog",
"schema",
"fileset",
null));

assertThrows(RuntimeException.class, spyRemoveProperty::validate);
verify(spyRemoveProperty, never()).handle();
String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(ErrorMessages.MISSING_PROPERTY, errOutput);
}

@Test
@SuppressWarnings("DefaultCharset")
void testListFilesetCommandWithoutCatalog() {
Expand Down
Loading
Loading