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 4da9443752 [#6102] fix(CLI): Fix Exception thrown when trying to set multiple tags properties in Gravitino CLI (#6111) 4da9443752 is described below commit 4da94437525f89f26798043130e3cb8630c227b9 Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com> AuthorDate: Mon Jan 6 16:42:05 2025 +0800 [#6102] fix(CLI): Fix Exception thrown when trying to set multiple tags properties in Gravitino CLI (#6111) ### What changes were proposed in this pull request? Fix Exception thrown when trying to set multiple tags properties in Gravitino CLI. It should give some user-friendly information. ### Why are the changes needed? Fix: #6102 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? ut + local test ```bash gcli tag set --metalake demo_metalake --tag tagA tagB tagC --property test --value value # This command only supports one --tag option. gcli tag details --metalake demo_metalake --tag tagA tagB tagC # This command only supports one --tag option. gcli tag remove --metalake demo_metalake --tag tagA tagB tagC --property test # This command only supports one --tag option. gcli tag update --metalake demo_metalake --tag tagA tagB tagC --comment "new comment" # This command only supports one --tag option. gcli tag update --metalake demo_metalake --tag tagA tagB tagC --rename "new name" # This command only supports one --tag option. ``` --- .../apache/gravitino/cli/GravitinoCommandLine.java | 5 +- .../org/apache/gravitino/cli/TestTagCommands.java | 116 ++++++++++++++++++++- 2 files changed, 116 insertions(+), 5 deletions(-) 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 b3917c4f06..f6b3520b86 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 @@ -724,7 +724,10 @@ public class GravitinoCommandLine extends TestableCommandLine { } private String getOneTag(String[] tags) { - Preconditions.checkArgument(tags.length <= 1, ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR); + if (tags.length > 1) { + System.err.println(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR); + Main.exit(-1); + } return tags[0]; } 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 3279c23d14..d3b0c8bfe1 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 @@ -117,6 +117,25 @@ class TestTagCommands { verify(mockDetails).handle(); } + @Test + void testTagDetailsCommandWithMultipleTag() { + Main.useExit = false; + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)) + .thenReturn(new String[] {"tagA", "tagB"}); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.DETAILS)); + + assertThrows(RuntimeException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newTagDetails(eq(GravitinoCommandLine.DEFAULT_URL), eq(false), eq("metalake_demo"), any()); + String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); + assertEquals(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR, output); + } + @Test void testCreateTagCommand() { CreateTag mockCreate = mock(CreateTag.class); @@ -355,6 +374,7 @@ class TestTagCommands { @Test void testSetMultipleTagPropertyCommandError() { + 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); @@ -368,10 +388,17 @@ class TestTagCommands { spy( new GravitinoCommandLine( mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.SET)); - Assertions.assertThrows( - IllegalArgumentException.class, - () -> commandLine.handleCommandLine(), - ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR); + Assertions.assertThrows(RuntimeException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newSetTagProperty( + eq(GravitinoCommandLine.DEFAULT_URL), + eq(false), + eq("metalake_demo"), + any(), + eq("property"), + eq("value")); + String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); + assertEquals(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR, output); } @Test @@ -395,6 +422,33 @@ class TestTagCommands { verify(mockRemoveProperty).handle(); } + @Test + void testRemoveTagPropertyCommandWithMultipleTags() { + 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", "tagB"}); + when(mockCommandLine.hasOption(GravitinoOptions.PROPERTY)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.PROPERTY)).thenReturn("property"); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.REMOVE)); + + assertThrows(RuntimeException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newRemoveTagProperty( + eq(GravitinoCommandLine.DEFAULT_URL), + eq(false), + eq("metalake_demo"), + any(), + eq("property")); + String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); + assertEquals(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR, output); + } + @Test void testListTagPropertiesCommand() { ListTagProperties mockListProperties = mock(ListTagProperties.class); @@ -459,6 +513,33 @@ class TestTagCommands { verify(mockUpdateComment).handle(); } + @Test + void testUpdateTagCommentCommandWithMultipleTags() { + Main.useExit = false; + when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); + when(mockCommandLine.hasOption(GravitinoOptions.COMMENT)).thenReturn(true); + when(mockCommandLine.hasOption(GravitinoOptions.TAG)).thenReturn(true); + when(mockCommandLine.getOptionValues(GravitinoOptions.TAG)) + .thenReturn(new String[] {"tagA", "tagB"}); + when(mockCommandLine.getOptionValue(GravitinoOptions.COMMENT)).thenReturn("new comment"); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.UPDATE)); + + assertThrows(RuntimeException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newUpdateTagComment( + eq(GravitinoCommandLine.DEFAULT_URL), + eq(false), + eq("metalake_demo"), + any(), + eq("new comment")); + String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); + assertEquals(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR, output); + } + @Test void testUpdateTagNameCommand() { UpdateTagName mockUpdateName = mock(UpdateTagName.class); @@ -479,6 +560,33 @@ class TestTagCommands { verify(mockUpdateName).handle(); } + @Test + void testUpdateTagNameCommandWithMultipleTags() { + 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", "tagB"}); + when(mockCommandLine.hasOption(GravitinoOptions.RENAME)).thenReturn(true); + when(mockCommandLine.getOptionValue(GravitinoOptions.RENAME)).thenReturn("tagC"); + GravitinoCommandLine commandLine = + spy( + new GravitinoCommandLine( + mockCommandLine, mockOptions, CommandEntities.TAG, CommandActions.UPDATE)); + + assertThrows(RuntimeException.class, commandLine::handleCommandLine); + verify(commandLine, never()) + .newUpdateTagName( + eq(GravitinoCommandLine.DEFAULT_URL), + eq(false), + eq("metalake_demo"), + any(), + eq("tagC")); + String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); + assertEquals(ErrorMessages.MULTIPLE_TAG_COMMAND_ERROR, output); + } + @Test void testListEntityTagsCommand() { ListEntityTags mockListTags = mock(ListEntityTags.class);