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 156898a136 [#6106] fix(CLI): Refactor the validation logic of user and group (#6113) 156898a136 is described below commit 156898a13625c1d0bff9c0214cb8eb5cf200ae7b Author: Lord of Abyss <103809695+abyss-l...@users.noreply.github.com> AuthorDate: Mon Jan 6 16:50:57 2025 +0800 [#6106] fix(CLI): Refactor the validation logic of user and group (#6113) ### What changes were proposed in this pull request? Refactor the validation logic of user and group. ### Why are the changes needed? Fix: #6106 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local test. --- .../apache/gravitino/cli/GravitinoCommandLine.java | 28 +++++++++++----------- .../apache/gravitino/cli/TestGroupCommands.java | 10 ++++++++ .../org/apache/gravitino/cli/TestUserCommands.java | 10 ++++++++ 3 files changed, 34 insertions(+), 14 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 f6b3520b86..cd1ce5f6c0 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 @@ -517,29 +517,29 @@ public class GravitinoCommandLine extends TestableCommandLine { switch (command) { case CommandActions.DETAILS: if (line.hasOption(GravitinoOptions.AUDIT)) { - newUserAudit(url, ignore, metalake, user).handle(); + newUserAudit(url, ignore, metalake, user).validate().handle(); } else { - newUserDetails(url, ignore, metalake, user).handle(); + newUserDetails(url, ignore, metalake, user).validate().handle(); } break; case CommandActions.LIST: - newListUsers(url, ignore, metalake).handle(); + newListUsers(url, ignore, metalake).validate().handle(); break; case CommandActions.CREATE: - newCreateUser(url, ignore, metalake, user).handle(); + newCreateUser(url, ignore, metalake, user).validate().handle(); break; case CommandActions.DELETE: boolean force = line.hasOption(GravitinoOptions.FORCE); - newDeleteUser(url, ignore, force, metalake, user).handle(); + newDeleteUser(url, ignore, force, metalake, user).validate().handle(); break; case CommandActions.REVOKE: String[] revokeRoles = line.getOptionValues(GravitinoOptions.ROLE); for (String role : revokeRoles) { - newRemoveRoleFromUser(url, ignore, metalake, user, role).handle(); + newRemoveRoleFromUser(url, ignore, metalake, user, role).validate().handle(); } System.out.printf("Remove roles %s from user %s%n", COMMA_JOINER.join(revokeRoles), user); break; @@ -547,7 +547,7 @@ public class GravitinoCommandLine extends TestableCommandLine { case CommandActions.GRANT: String[] grantRoles = line.getOptionValues(GravitinoOptions.ROLE); for (String role : grantRoles) { - newAddRoleToUser(url, ignore, metalake, user, role).handle(); + newAddRoleToUser(url, ignore, metalake, user, role).validate().handle(); } System.out.printf("Grant roles %s to user %s%n", COMMA_JOINER.join(grantRoles), user); break; @@ -578,29 +578,29 @@ public class GravitinoCommandLine extends TestableCommandLine { switch (command) { case CommandActions.DETAILS: if (line.hasOption(GravitinoOptions.AUDIT)) { - newGroupAudit(url, ignore, metalake, group).handle(); + newGroupAudit(url, ignore, metalake, group).validate().handle(); } else { - newGroupDetails(url, ignore, metalake, group).handle(); + newGroupDetails(url, ignore, metalake, group).validate().handle(); } break; case CommandActions.LIST: - newListGroups(url, ignore, metalake).handle(); + newListGroups(url, ignore, metalake).validate().handle(); break; case CommandActions.CREATE: - newCreateGroup(url, ignore, metalake, group).handle(); + newCreateGroup(url, ignore, metalake, group).validate().handle(); break; case CommandActions.DELETE: boolean force = line.hasOption(GravitinoOptions.FORCE); - newDeleteGroup(url, ignore, force, metalake, group).handle(); + newDeleteGroup(url, ignore, force, metalake, group).validate().handle(); break; case CommandActions.REVOKE: String[] revokeRoles = line.getOptionValues(GravitinoOptions.ROLE); for (String role : revokeRoles) { - newRemoveRoleFromGroup(url, ignore, metalake, group, role).handle(); + newRemoveRoleFromGroup(url, ignore, metalake, group, role).validate().handle(); } System.out.printf("Remove roles %s from group %s%n", COMMA_JOINER.join(revokeRoles), group); break; @@ -608,7 +608,7 @@ public class GravitinoCommandLine extends TestableCommandLine { case CommandActions.GRANT: String[] grantRoles = line.getOptionValues(GravitinoOptions.ROLE); for (String role : grantRoles) { - newAddRoleToGroup(url, ignore, metalake, group, role).handle(); + newAddRoleToGroup(url, ignore, metalake, group, role).validate().handle(); } System.out.printf("Grant roles %s to group %s%n", COMMA_JOINER.join(grantRoles), group); break; diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java index 98e3ea910f..ce7a895682 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestGroupCommands.java @@ -83,6 +83,7 @@ class TestGroupCommands { doReturn(mockList) .when(commandLine) .newListGroups(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo"); + doReturn(mockList).when(mockList).validate(); commandLine.handleCommandLine(); verify(mockList).handle(); } @@ -101,6 +102,7 @@ class TestGroupCommands { doReturn(mockDetails) .when(commandLine) .newGroupDetails(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA"); + doReturn(mockDetails).when(mockDetails).validate(); commandLine.handleCommandLine(); verify(mockDetails).handle(); } @@ -120,6 +122,7 @@ class TestGroupCommands { doReturn(mockAudit) .when(commandLine) .newGroupAudit(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "group"); + doReturn(mockAudit).when(mockAudit).validate(); commandLine.handleCommandLine(); verify(mockAudit).handle(); } @@ -138,6 +141,7 @@ class TestGroupCommands { doReturn(mockCreate) .when(commandLine) .newCreateGroup(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA"); + doReturn(mockCreate).when(mockCreate).validate(); commandLine.handleCommandLine(); verify(mockCreate).handle(); } @@ -156,6 +160,7 @@ class TestGroupCommands { doReturn(mockDelete) .when(commandLine) .newDeleteGroup(GravitinoCommandLine.DEFAULT_URL, false, false, "metalake_demo", "groupA"); + doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); } @@ -175,6 +180,7 @@ class TestGroupCommands { doReturn(mockDelete) .when(commandLine) .newDeleteGroup(GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo", "groupA"); + doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); } @@ -246,6 +252,8 @@ class TestGroupCommands { .newRemoveRoleFromGroup( GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA", "role1"); + doReturn(mockRemoveFirstRole).when(mockRemoveFirstRole).validate(); + doReturn(mockRemoveSecondRole).when(mockRemoveSecondRole).validate(); commandLine.handleCommandLine(); verify(mockRemoveFirstRole).handle(); @@ -279,6 +287,8 @@ class TestGroupCommands { .newAddRoleToGroup( GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "groupA", "role1"); + doReturn(mockAddFirstRole).when(mockAddFirstRole).validate(); + doReturn(mockAddSecondRole).when(mockAddSecondRole).validate(); commandLine.handleCommandLine(); verify(mockAddSecondRole).handle(); diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java index e8630ce975..c7612f6c87 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestUserCommands.java @@ -83,6 +83,7 @@ class TestUserCommands { doReturn(mockList) .when(commandLine) .newListUsers(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo"); + doReturn(mockList).when(mockList).validate(); commandLine.handleCommandLine(); verify(mockList).handle(); } @@ -101,6 +102,7 @@ class TestUserCommands { doReturn(mockDetails) .when(commandLine) .newUserDetails(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user"); + doReturn(mockDetails).when(mockDetails).validate(); commandLine.handleCommandLine(); verify(mockDetails).handle(); } @@ -120,6 +122,7 @@ class TestUserCommands { doReturn(mockAudit) .when(commandLine) .newUserAudit(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "admin"); + doReturn(mockAudit).when(mockAudit).validate(); commandLine.handleCommandLine(); verify(mockAudit).handle(); } @@ -138,6 +141,7 @@ class TestUserCommands { doReturn(mockCreate) .when(commandLine) .newCreateUser(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user"); + doReturn(mockCreate).when(mockCreate).validate(); commandLine.handleCommandLine(); verify(mockCreate).handle(); } @@ -156,6 +160,7 @@ class TestUserCommands { doReturn(mockDelete) .when(commandLine) .newDeleteUser(GravitinoCommandLine.DEFAULT_URL, false, false, "metalake_demo", "user"); + doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); } @@ -175,6 +180,7 @@ class TestUserCommands { doReturn(mockDelete) .when(commandLine) .newDeleteUser(GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo", "user"); + doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); } @@ -247,6 +253,8 @@ class TestUserCommands { .newRemoveRoleFromUser( GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", "role1"); + doReturn(mockRemoveFirstRole).when(mockRemoveFirstRole).validate(); + doReturn(mockRemoveSecondRole).when(mockRemoveSecondRole).validate(); commandLine.handleCommandLine(); verify(mockRemoveSecondRole).handle(); @@ -281,6 +289,8 @@ class TestUserCommands { .newAddRoleToUser( GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "user", "role1"); + doReturn(mockAddFirstRole).when(mockAddFirstRole).validate(); + doReturn(mockAddSecondRole).when(mockAddSecondRole).validate(); commandLine.handleCommandLine(); verify(mockAddFirstRole).handle();