From ea64c72a916b93b1280ac2db44f43f849d7f4d85 Mon Sep 17 00:00:00 2001 From: Lord of Abyss <103809695+Abyss-lord@users.noreply.github.com> Date: Wed, 12 Feb 2025 07:37:12 +0800 Subject: [PATCH] [#6422] improve(CLI): Add roles and owner command context CLI (#6438) ### What changes were proposed in this pull request? Add roles and owner command context CLI ### Why are the changes needed? Fix: #6422 ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? local test. --- .../gravitino/cli/GravitinoCommandLine.java | 4 +- .../gravitino/cli/OwnerCommandHandler.java | 20 ++--- .../gravitino/cli/RoleCommandHandler.java | 33 ++++---- .../gravitino/cli/TestableCommandLine.java | 52 +++++------- .../gravitino/cli/commands/CreateRole.java | 10 +-- .../gravitino/cli/commands/DeleteRole.java | 12 ++- .../cli/commands/GrantPrivilegesToRole.java | 30 +++---- .../gravitino/cli/commands/ListRoles.java | 17 ++-- .../gravitino/cli/commands/OwnerDetails.java | 13 ++- .../cli/commands/RevokeAllPrivileges.java | 23 +++-- .../commands/RevokePrivilegesFromRole.java | 18 ++-- .../gravitino/cli/commands/RoleAudit.java | 19 ++--- .../gravitino/cli/commands/RoleDetails.java | 14 ++-- .../gravitino/cli/commands/SetOwner.java | 11 ++- .../gravitino/cli/TestOwnerCommands.java | 51 ++++++------ .../gravitino/cli/TestRoleCommands.java | 83 ++++++++----------- 16 files changed, 174 insertions(+), 236 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 e61051e5382..3a7c656691a 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 @@ -114,7 +114,7 @@ private void executeCommand() { if (CommandActions.HELP.equals(command)) { handleHelpCommand(); } else if (line.hasOption(GravitinoOptions.OWNER)) { - new OwnerCommandHandler(this, line, command, ignore, entity).handle(); + new OwnerCommandHandler(this, line, command, context, entity).handle(); } else if (entity.equals(CommandEntities.COLUMN)) { new ColumnCommandHandler(this, line, command, context).handle(); } else if (entity.equals(CommandEntities.TABLE)) { @@ -136,7 +136,7 @@ private void executeCommand() { } else if (entity.equals(CommandEntities.TAG)) { new TagCommandHandler(this, line, command, context).handle(); } else if (entity.equals(CommandEntities.ROLE)) { - new RoleCommandHandler(this, line, command, ignore).handle(); + new RoleCommandHandler(this, line, command, context).handle(); } else if (entity.equals(CommandEntities.MODEL)) { new ModelCommandHandler(this, line, command, context).handle(); } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/OwnerCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/OwnerCommandHandler.java index 7e41fb478ae..28fb7fb3803 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/OwnerCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/OwnerCommandHandler.java @@ -27,8 +27,7 @@ public class OwnerCommandHandler extends CommandHandler { private final GravitinoCommandLine gravitinoCommandLine; private final CommandLine line; private final String command; - private final boolean ignore; - private final String url; + private final CommandContext context; private final FullName name; private final String metalake; private final String entityName; @@ -42,21 +41,21 @@ public class OwnerCommandHandler extends CommandHandler { * @param gravitinoCommandLine The Gravitino command line instance. * @param line The command line arguments. * @param command The command to execute. - * @param ignore Ignore server version mismatch. + * @param context The command context. * @param entity The entity to execute the command on. */ public OwnerCommandHandler( GravitinoCommandLine gravitinoCommandLine, CommandLine line, String command, - boolean ignore, + CommandContext context, String entity) { this.gravitinoCommandLine = gravitinoCommandLine; this.line = line; this.command = command; - this.ignore = ignore; + this.context = context; - this.url = getUrl(line); + this.context.setUrl(getUrl(line)); this.owner = line.getOptionValue(GravitinoOptions.USER); this.group = line.getOptionValue(GravitinoOptions.GROUP); this.name = new FullName(line); @@ -102,22 +101,19 @@ private boolean executeCommand() { /** Handles the "DETAILS" command. */ private void handleDetailsCommand() { - gravitinoCommandLine - .newOwnerDetails(url, ignore, metalake, entityName, entity) - .validate() - .handle(); + gravitinoCommandLine.newOwnerDetails(context, metalake, entityName, entity).validate().handle(); } /** Handles the "SET" command. */ private void handleSetCommand() { if (owner != null && group == null) { gravitinoCommandLine - .newSetOwner(url, ignore, metalake, entityName, entity, owner, false) + .newSetOwner(context, metalake, entityName, entity, owner, false) .validate() .handle(); } else if (owner == null && group != null) { gravitinoCommandLine - .newSetOwner(url, ignore, metalake, entityName, entity, group, true) + .newSetOwner(context, metalake, entityName, entity, group, true) .validate() .handle(); } else { diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java index 327ef21fdd8..ef21ec5c419 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/RoleCommandHandler.java @@ -28,19 +28,21 @@ public class RoleCommandHandler extends CommandHandler { private final GravitinoCommandLine gravitinoCommandLine; private final CommandLine line; private final String command; - private final boolean ignore; - private final String url; + private final CommandContext context; private String metalake; private String[] roles; private String[] privileges; public RoleCommandHandler( - GravitinoCommandLine gravitinoCommandLine, CommandLine line, String command, boolean ignore) { + GravitinoCommandLine gravitinoCommandLine, + CommandLine line, + String command, + CommandContext context) { this.gravitinoCommandLine = gravitinoCommandLine; this.line = line; this.command = command; - this.ignore = ignore; - this.url = getUrl(line); + this.context = context; + this.context.setUrl(getUrl(line)); } /** Handles the command execution logic based on the provided command. */ @@ -106,32 +108,27 @@ private boolean executeCommand() { private void handleDetailsCommand() { if (line.hasOption(GravitinoOptions.AUDIT)) { - gravitinoCommandLine.newRoleAudit(url, ignore, metalake, getOneRole()).validate().handle(); + gravitinoCommandLine.newRoleAudit(context, metalake, getOneRole()).validate().handle(); } else { - gravitinoCommandLine.newRoleDetails(url, ignore, metalake, getOneRole()).validate().handle(); + gravitinoCommandLine.newRoleDetails(context, metalake, getOneRole()).validate().handle(); } } private void handleListCommand() { - gravitinoCommandLine.newListRoles(url, ignore, metalake).validate().handle(); + gravitinoCommandLine.newListRoles(context, metalake).validate().handle(); } private void handleCreateCommand() { - gravitinoCommandLine.newCreateRole(url, ignore, metalake, roles).validate().handle(); + gravitinoCommandLine.newCreateRole(context, metalake, roles).validate().handle(); } private void handleDeleteCommand() { - boolean forceDelete = line.hasOption(GravitinoOptions.FORCE); - gravitinoCommandLine - .newDeleteRole(url, ignore, forceDelete, metalake, roles) - .validate() - .handle(); + gravitinoCommandLine.newDeleteRole(context, metalake, roles).validate().handle(); } private void handleGrantCommand() { gravitinoCommandLine - .newGrantPrivilegesToRole( - url, ignore, metalake, getOneRole(), new FullName(line), privileges) + .newGrantPrivilegesToRole(context, metalake, getOneRole(), new FullName(line), privileges) .validate() .handle(); } @@ -140,13 +137,13 @@ private void handleRevokeCommand() { boolean removeAll = line.hasOption(GravitinoOptions.ALL); if (removeAll) { gravitinoCommandLine - .newRevokeAllPrivileges(url, ignore, metalake, getOneRole(), new FullName(line)) + .newRevokeAllPrivileges(context, metalake, getOneRole(), new FullName(line)) .validate() .handle(); } else { gravitinoCommandLine .newRevokePrivilegesFromRole( - url, ignore, metalake, getOneRole(), new FullName(line), privileges) + context, metalake, getOneRole(), new FullName(line), privileges) .validate() .handle(); } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java index a4f47239c71..22425b621b6 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/TestableCommandLine.java @@ -451,25 +451,24 @@ protected AddRoleToGroup newAddRoleToGroup( return new AddRoleToGroup(context, metalake, group, role); } - protected RoleDetails newRoleDetails(String url, boolean ignore, String metalake, String role) { - return new RoleDetails(url, ignore, metalake, role); + protected RoleDetails newRoleDetails(CommandContext context, String metalake, String role) { + return new RoleDetails(context, metalake, role); } - protected ListRoles newListRoles(String url, boolean ignore, String metalake) { - return new ListRoles(url, ignore, metalake); + protected ListRoles newListRoles(CommandContext context, String metalake) { + return new ListRoles(context, metalake); } - protected RoleAudit newRoleAudit(String url, boolean ignore, String metalake, String role) { - return new RoleAudit(url, ignore, metalake, role); + protected RoleAudit newRoleAudit(CommandContext context, String metalake, String role) { + return new RoleAudit(context, metalake, role); } - protected CreateRole newCreateRole(String url, boolean ignore, String metalake, String[] roles) { - return new CreateRole(url, ignore, metalake, roles); + protected CreateRole newCreateRole(CommandContext context, String metalake, String[] roles) { + return new CreateRole(context, metalake, roles); } - protected DeleteRole newDeleteRole( - String url, boolean ignore, boolean force, String metalake, String[] roles) { - return new DeleteRole(url, ignore, force, metalake, roles); + protected DeleteRole newDeleteRole(CommandContext context, String metalake, String[] roles) { + return new DeleteRole(context, metalake, roles); } protected TagDetails newTagDetails(CommandContext context, String metalake, String tag) { @@ -549,19 +548,18 @@ protected ListColumns newListColumns( } protected SetOwner newSetOwner( - String url, - boolean ignore, + CommandContext context, String metalake, String entity, String entityType, String owner, boolean isGroup) { - return new SetOwner(url, ignore, metalake, entity, entityType, owner, isGroup); + return new SetOwner(context, metalake, entity, entityType, owner, isGroup); } protected OwnerDetails newOwnerDetails( - String url, boolean ignore, String metalake, String entity, String entityType) { - return new OwnerDetails(url, ignore, metalake, entity, entityType); + CommandContext context, String metalake, String entity, String entityType) { + return new OwnerDetails(context, metalake, entity, entityType); } protected ListTopics newListTopics( @@ -827,28 +825,18 @@ protected CreateTable newCreateTable( } protected GrantPrivilegesToRole newGrantPrivilegesToRole( - String url, - boolean ignore, - String metalake, - String role, - FullName entity, - String[] privileges) { - return new GrantPrivilegesToRole(url, ignore, metalake, role, entity, privileges); + CommandContext context, String metalake, String role, FullName entity, String[] privileges) { + return new GrantPrivilegesToRole(context, metalake, role, entity, privileges); } protected RevokePrivilegesFromRole newRevokePrivilegesFromRole( - String url, - boolean ignore, - String metalake, - String role, - FullName entity, - String[] privileges) { - return new RevokePrivilegesFromRole(url, ignore, metalake, role, entity, privileges); + CommandContext context, String metalake, String role, FullName entity, String[] privileges) { + return new RevokePrivilegesFromRole(context, metalake, role, entity, privileges); } protected RevokeAllPrivileges newRevokeAllPrivileges( - String url, boolean ignore, String metalake, String role, FullName entity) { - return new RevokeAllPrivileges(url, ignore, metalake, role, entity); + CommandContext context, String metalake, String role, FullName entity) { + return new RevokeAllPrivileges(context, metalake, role, entity); } protected MetalakeEnable newMetalakeEnable( diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java index e821013471f..aeecaaf0a3f 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CreateRole.java @@ -21,6 +21,7 @@ import com.google.common.base.Joiner; import java.util.Collections; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -33,13 +34,12 @@ public class CreateRole extends Command { /** * Create a new role. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param roles The array of roles. */ - public CreateRole(String url, boolean ignoreVersions, String metalake, String[] roles) { - super(url, ignoreVersions); + public CreateRole(CommandContext context, String metalake, String[] roles) { + super(context); this.metalake = metalake; this.roles = roles; } @@ -60,6 +60,6 @@ public void handle() { exitWithError(exp.getMessage()); } - System.out.println(Joiner.on(", ").join(roles) + " created"); + printInformation(Joiner.on(", ").join(roles) + " created"); } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java index 4e0bb508d19..f596ed09631 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/DeleteRole.java @@ -23,6 +23,7 @@ import com.google.common.collect.Lists; import java.util.List; import org.apache.gravitino.cli.AreYouSure; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -37,17 +38,14 @@ public class DeleteRole extends Command { /** * Delete a role. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. - * @param force Force operation. + * @param context The command context. * @param metalake The name of the metalake. * @param roles The name of the role. */ - public DeleteRole( - String url, boolean ignoreVersions, boolean force, String metalake, String[] roles) { - super(url, ignoreVersions); + public DeleteRole(CommandContext context, String metalake, String[] roles) { + super(context); this.metalake = metalake; - this.force = force; + this.force = context.force(); this.roles = roles; } 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 b21a74ff48c..4bdcc618b4c 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 @@ -23,6 +23,7 @@ import java.util.Set; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.authorization.Privilege; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.cli.FullName; import org.apache.gravitino.cli.Privileges; @@ -43,21 +44,15 @@ public class GrantPrivilegesToRole extends MetadataCommand { /** * Grants one or more privileges. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param role The name of the role. * @param entity The name of the entity. * @param privileges The list of privileges. */ public GrantPrivilegesToRole( - String url, - boolean ignoreVersions, - String metalake, - String role, - FullName entity, - String[] privileges) { - super(url, ignoreVersions); + CommandContext context, String metalake, String role, FullName entity, String[] privileges) { + super(context); this.metalake = metalake; this.entity = entity; this.role = role; @@ -73,8 +68,7 @@ public void handle() { for (String privilege : privileges) { if (!Privileges.isValid(privilege)) { - System.err.println(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege); - return; + exitWithError(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege); } PrivilegeDTO privilegeDTO = PrivilegeDTO.builder() @@ -87,21 +81,17 @@ public void handle() { MetadataObject metadataObject = constructMetadataObject(entity, client); client.grantPrivilegesToRole(role, metadataObject, privilegesSet); } catch (NoSuchMetalakeException err) { - System.err.println(ErrorMessages.UNKNOWN_METALAKE); - return; + exitWithError(ErrorMessages.UNKNOWN_METALAKE); } catch (NoSuchRoleException err) { - System.err.println(ErrorMessages.UNKNOWN_ROLE); - return; + exitWithError(ErrorMessages.UNKNOWN_ROLE); } catch (NoSuchMetadataObjectException err) { - System.err.println(ErrorMessages.UNKNOWN_USER); - return; + exitWithError(ErrorMessages.UNKNOWN_USER); } catch (Exception exp) { - System.err.println(exp.getMessage()); - return; + exitWithError(exp.getMessage()); } String all = String.join(",", privileges); - System.out.println(role + " granted " + all + " on " + entity.getName()); + printInformation(role + " granted " + all + " on " + entity.getName()); } @Override diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListRoles.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListRoles.java index 760fe21e3bc..d859c377c7f 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListRoles.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ListRoles.java @@ -19,6 +19,7 @@ package org.apache.gravitino.cli.commands; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -31,12 +32,11 @@ public class ListRoles extends Command { /** * Lists all groups in a metalake. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. */ - public ListRoles(String url, boolean ignoreVersions, String metalake) { - super(url, ignoreVersions); + public ListRoles(CommandContext context, String metalake) { + super(context); this.metalake = metalake; } @@ -52,9 +52,10 @@ public void handle() { } catch (Exception exp) { exitWithError(exp.getMessage()); } - - String all = roles.length == 0 ? "No roles exist." : String.join(",", roles); - - System.out.println(all); + if (roles.length == 0) { + printInformation("No roles exist."); + } else { + printResults(String.join(",", roles)); + } } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/OwnerDetails.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/OwnerDetails.java index a815d6ba14d..2c8d17a9f77 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/OwnerDetails.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/OwnerDetails.java @@ -23,6 +23,7 @@ import org.apache.gravitino.MetadataObject; import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.CommandEntities; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; @@ -38,15 +39,13 @@ public class OwnerDetails extends Command { /** * Displays the owner of an entity. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param entity The name of the entity. * @param entityType The type entity. */ - public OwnerDetails( - String url, boolean ignoreVersions, String metalake, String entity, String entityType) { - super(url, ignoreVersions); + public OwnerDetails(CommandContext context, String metalake, String entity, String entityType) { + super(context); this.metalake = metalake; this.entity = entity; @@ -83,9 +82,9 @@ public void handle() { } if (owner.isPresent()) { - System.out.println(owner.get().name()); + printResults(owner.get().name()); } else { - System.out.println("No owner"); + printInformation("No owner"); } } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java index 3167c4b6c37..2b059b9afff 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RevokeAllPrivileges.java @@ -30,6 +30,7 @@ import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.Role; import org.apache.gravitino.authorization.SecurableObject; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.cli.FullName; import org.apache.gravitino.client.GravitinoClient; @@ -46,15 +47,14 @@ public class RevokeAllPrivileges extends MetadataCommand { /** * Revokes all privileges from a role to an entity or all entities. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param role The name of the role. * @param entity The name of the entity. */ public RevokeAllPrivileges( - String url, boolean ignoreVersions, String metalake, String role, FullName entity) { - super(url, ignoreVersions); + CommandContext context, String metalake, String role, FullName entity) { + super(context); this.metalake = metalake; this.role = role; this.entity = entity; @@ -86,8 +86,11 @@ public void handle() { exitWithError(e.getMessage()); } - if (revokedPrivileges.isEmpty()) outputRevokedPrivileges("No privileges revoked."); - outputRevokedPrivileges(revokedPrivileges); + if (revokedPrivileges.isEmpty()) { + printInformation("No privileges revoked."); + } else { + outputRevokedPrivileges(revokedPrivileges); + } } private List getMatchedObjects(GravitinoClient client) { @@ -106,16 +109,12 @@ private void outputRevokedPrivileges(Map> revokedPrivileg revokedInfoList.add(entry.getKey() + ": " + COMMA_JOINER.join(revokedPrivilegesList)); } - System.out.println("Revoked privileges:"); + printInformation("Revoked privileges:"); for (String info : revokedInfoList) { - System.out.println(info); + printResults(info); } } - private void outputRevokedPrivileges(String message) { - System.out.println(message); - } - /** * verify the arguments. * 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 5907c494c05..c1e80708b32 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 @@ -23,6 +23,7 @@ import java.util.Set; import org.apache.gravitino.MetadataObject; import org.apache.gravitino.authorization.Privilege; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.cli.FullName; import org.apache.gravitino.cli.Privileges; @@ -43,21 +44,15 @@ public class RevokePrivilegesFromRole extends MetadataCommand { /** * Revokes one or more privileges. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param role The name of the role. * @param entity The name of the entity. * @param privileges The list of privileges. */ public RevokePrivilegesFromRole( - String url, - boolean ignoreVersions, - String metalake, - String role, - FullName entity, - String[] privileges) { - super(url, ignoreVersions); + CommandContext context, String metalake, String role, FullName entity, String[] privileges) { + super(context); this.metalake = metalake; this.entity = entity; this.role = role; @@ -73,8 +68,7 @@ public void handle() { for (String privilege : privileges) { if (!Privileges.isValid(privilege)) { - System.err.println(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege); - return; + exitWithError(ErrorMessages.UNKNOWN_PRIVILEGE + " " + privilege); } PrivilegeDTO privilegeDTO = PrivilegeDTO.builder() @@ -97,7 +91,7 @@ public void handle() { } String all = String.join(",", privileges); - System.out.println(role + " revoked " + all + " on " + entity.getName()); + printInformation(role + " revoked " + all + " on " + entity.getName()); } @Override diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleAudit.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleAudit.java index e497ca836f5..f512a3089ba 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleAudit.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleAudit.java @@ -20,6 +20,7 @@ package org.apache.gravitino.cli.commands; import org.apache.gravitino.authorization.Role; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -33,13 +34,12 @@ public class RoleAudit extends AuditCommand { /** * Displays the audit information of a role. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param role The name of the role. */ - public RoleAudit(String url, boolean ignoreVersions, String metalake, String role) { - super(url, ignoreVersions); + public RoleAudit(CommandContext context, String metalake, String role) { + super(context); this.metalake = metalake; this.role = role; } @@ -47,19 +47,16 @@ public RoleAudit(String url, boolean ignoreVersions, String metalake, String rol /** Displays the audit information of a specified role. */ @Override public void handle() { - Role result; + Role result = null; try (GravitinoClient client = buildClient(metalake)) { result = client.getRole(this.role); } catch (NoSuchMetalakeException err) { - System.err.println(ErrorMessages.UNKNOWN_METALAKE); - return; + exitWithError(ErrorMessages.UNKNOWN_METALAKE); } catch (NoSuchRoleException err) { - System.err.println(ErrorMessages.UNKNOWN_ROLE); - return; + exitWithError(ErrorMessages.UNKNOWN_ROLE); } catch (Exception exp) { - System.err.println(exp.getMessage()); - return; + exitWithError(exp.getMessage()); } if (result != null) { diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleDetails.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleDetails.java index 57e314ac56e..94af016df37 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleDetails.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/RoleDetails.java @@ -22,6 +22,7 @@ import java.util.List; import org.apache.gravitino.authorization.Privilege; import org.apache.gravitino.authorization.SecurableObject; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; import org.apache.gravitino.exceptions.NoSuchMetalakeException; @@ -35,13 +36,12 @@ public class RoleDetails extends Command { /** * Displays the securable objects in a role. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param role The name of the role. */ - public RoleDetails(String url, boolean ignoreVersions, String metalake, String role) { - super(url, ignoreVersions); + public RoleDetails(CommandContext context, String metalake, String role) { + super(context); this.metalake = metalake; this.role = role; } @@ -63,11 +63,11 @@ public void handle() { } for (SecurableObject object : objects) { - System.out.print(object.name() + "," + object.type() + ","); + printInformation(object.name() + "," + object.type() + ","); for (Privilege privilege : object.privileges()) { - System.out.print(privilege.simpleString() + " "); + printInformation(privilege.simpleString() + " "); } } - System.out.println(""); + printInformation(""); } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetOwner.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetOwner.java index 45de461043a..a13f278b359 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetOwner.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/SetOwner.java @@ -22,6 +22,7 @@ import org.apache.gravitino.MetadataObject; import org.apache.gravitino.MetadataObjects; import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.cli.CommandContext; import org.apache.gravitino.cli.CommandEntities; import org.apache.gravitino.cli.ErrorMessages; import org.apache.gravitino.client.GravitinoClient; @@ -39,8 +40,7 @@ public class SetOwner extends Command { /** * Sets the owner of an entity. * - * @param url The URL of the Gravitino server. - * @param ignoreVersions If true don't check the client/server versions match. + * @param context The command context. * @param metalake The name of the metalake. * @param entity The name of the entity. * @param entityType The type entity. @@ -48,14 +48,13 @@ public class SetOwner extends Command { * @param isGroup True if the owner is a group, false if it is not. */ public SetOwner( - String url, - boolean ignoreVersions, + CommandContext context, String metalake, String entity, String entityType, String owner, boolean isGroup) { - super(url, ignoreVersions); + super(context); this.metalake = metalake; this.entity = entity; this.owner = owner; @@ -97,6 +96,6 @@ public void handle() { exitWithError(exp.getMessage()); } - System.out.println("Set owner to " + owner); + printInformation("Set owner to " + owner); } } diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestOwnerCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestOwnerCommands.java index f3928fba28f..9cd28389840 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestOwnerCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestOwnerCommands.java @@ -21,6 +21,10 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; @@ -80,13 +84,12 @@ void testSetOwnerUserCommand() { doReturn(mockSetOwner) .when(commandLine) .newSetOwner( - GravitinoCommandLine.DEFAULT_URL, - false, - "metalake_demo", - "postgres", - "catalog", - "admin", - false); + any(CommandContext.class), + eq("metalake_demo"), + eq("postgres"), + eq("catalog"), + eq("admin"), + anyBoolean()); doReturn(mockSetOwner).when(mockSetOwner).validate(); commandLine.handleCommandLine(); verify(mockSetOwner).handle(); @@ -110,13 +113,12 @@ void testSetOwnerGroupCommand() { doReturn(mockSetOwner) .when(commandLine) .newSetOwner( - GravitinoCommandLine.DEFAULT_URL, - false, - "metalake_demo", - "postgres", - "catalog", - "ITdept", - true); + any(CommandContext.class), + eq("metalake_demo"), + eq("postgres"), + eq("catalog"), + eq("ITdept"), + anyBoolean()); doReturn(mockSetOwner).when(mockSetOwner).validate(); commandLine.handleCommandLine(); verify(mockSetOwner).handle(); @@ -137,7 +139,7 @@ void testOwnerDetailsCommand() { doReturn(mockOwnerDetails) .when(commandLine) .newOwnerDetails( - GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "postgres", "catalog"); + any(CommandContext.class), eq("metalake_demo"), eq("postgres"), eq("catalog")); doReturn(mockOwnerDetails).when(mockOwnerDetails).validate(); commandLine.handleCommandLine(); verify(mockOwnerDetails).handle(); @@ -158,11 +160,7 @@ void testOwnerDetailsCommandWithoutName() { assertThrows(RuntimeException.class, commandLine::handleCommandLine); verify(commandLine, never()) .newOwnerDetails( - GravitinoCommandLine.DEFAULT_URL, - false, - "metalake_demo", - null, - CommandEntities.CATALOG); + any(CommandContext.class), eq("metalake_demo"), isNull(), eq(CommandEntities.CATALOG)); String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); assertEquals(ErrorMessages.MISSING_NAME, errOutput); @@ -186,13 +184,12 @@ void testSetOwnerUserCommandWithoutUserAndGroup() { assertThrows(RuntimeException.class, commandLine::handleCommandLine); verify(commandLine, never()) .newSetOwner( - GravitinoCommandLine.DEFAULT_URL, - false, - "metalake_demo", - "postgres", - CommandEntities.CATALOG, - null, - false); + any(CommandContext.class), + eq("metalaek_demo"), + eq("postgres"), + eq(CommandEntities.CATALOG), + any(), + anyBoolean()); String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); assertEquals(ErrorMessages.INVALID_SET_COMMAND, errOutput); } 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 be463019fd3..845c7bea7fb 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 @@ -21,6 +21,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; +import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.Mockito.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.eq; @@ -84,7 +85,7 @@ void testListRolesCommand() { mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.LIST)); doReturn(mockList) .when(commandLine) - .newListRoles(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo"); + .newListRoles(any(CommandContext.class), eq("metalake_demo")); doReturn(mockList).when(mockList).validate(); commandLine.handleCommandLine(); verify(mockList).handle(); @@ -103,7 +104,7 @@ void testRoleDetailsCommand() { mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.DETAILS)); doReturn(mockDetails) .when(commandLine) - .newRoleDetails(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "admin"); + .newRoleDetails(any(CommandContext.class), eq("metalake_demo"), eq("admin")); doReturn(mockDetails).when(mockDetails).validate(); commandLine.handleCommandLine(); @@ -125,8 +126,7 @@ void testRoleDetailsCommandWithMultipleRoles() { assertThrows(RuntimeException.class, commandLine::handleCommandLine); verify(commandLine, never()) - .newRoleDetails( - eq(GravitinoCommandLine.DEFAULT_URL), eq(false), eq("metalake_demo"), any()); + .newRoleDetails(any(CommandContext.class), eq("metalake_demo"), any()); } @Test @@ -143,7 +143,7 @@ void testRoleAuditCommand() { mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.DETAILS)); doReturn(mockAudit) .when(commandLine) - .newRoleAudit(GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "group"); + .newRoleAudit(any(CommandContext.class), eq("metalake_demo"), eq("group")); doReturn(mockAudit).when(mockAudit).validate(); commandLine.handleCommandLine(); verify(mockAudit).handle(); @@ -163,7 +163,9 @@ void testCreateRoleCommand() { doReturn(mockCreate) .when(commandLine) .newCreateRole( - GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", new String[] {"admin"}); + any(CommandContext.class), + eq("metalake_demo"), + argThat(strings -> strings.length == 1 && "admin".equals(strings[0]))); doReturn(mockCreate).when(mockCreate).validate(); commandLine.handleCommandLine(); verify(mockCreate).handle(); @@ -185,10 +187,9 @@ void testCreateRolesCommand() { doReturn(mockCreate) .when(commandLine) .newCreateRole( - GravitinoCommandLine.DEFAULT_URL, - false, - "metalake_demo", - new String[] {"admin", "engineer", "scientist"}); + any(CommandContext.class), + eq("metalake_demo"), + argThat(strings -> strings.length == 3)); doReturn(mockCreate).when(mockCreate).validate(); commandLine.handleCommandLine(); verify(mockCreate).handle(); @@ -208,11 +209,9 @@ void testDeleteRoleCommand() { doReturn(mockDelete) .when(commandLine) .newDeleteRole( - GravitinoCommandLine.DEFAULT_URL, - false, - false, - "metalake_demo", - new String[] {"admin"}); + any(CommandContext.class), + eq("metalake_demo"), + argThat(strings -> strings.length == 1 && "admin".equals(strings[0]))); doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); @@ -235,11 +234,9 @@ void testDeleteRolesCommand() { doReturn(mockDelete) .when(commandLine) .newDeleteRole( - GravitinoCommandLine.DEFAULT_URL, - false, - false, - "metalake_demo", - new String[] {"admin", "engineer", "scientist"}); + any(CommandContext.class), + eq("metalake_demo"), + argThat(strings -> strings.length == 3)); doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); @@ -260,7 +257,9 @@ void testDeleteRoleForceCommand() { doReturn(mockDelete) .when(commandLine) .newDeleteRole( - GravitinoCommandLine.DEFAULT_URL, false, true, "metalake_demo", new String[] {"admin"}); + any(CommandContext.class), + eq("metalake_demo"), + argThat(strings -> strings.length == 1 && "admin".equals(strings[0]))); doReturn(mockDelete).when(mockDelete).validate(); commandLine.handleCommandLine(); verify(mockDelete).handle(); @@ -285,12 +284,7 @@ void testGrantPrivilegesToRole() { doReturn(mockGrant) .when(commandLine) .newGrantPrivilegesToRole( - eq(GravitinoCommandLine.DEFAULT_URL), - eq(false), - eq("metalake_demo"), - eq("admin"), - any(), - eq(privileges)); + any(CommandContext.class), eq("metalake_demo"), eq("admin"), any(), eq(privileges)); doReturn(mockGrant).when(mockGrant).validate(); commandLine.handleCommandLine(); verify(mockGrant).handle(); @@ -299,10 +293,10 @@ void testGrantPrivilegesToRole() { @Test void testGrantPrivilegesToRoleWithoutPrivileges() { Main.useExit = false; + CommandContext mockContext = mock(CommandContext.class); + when(mockContext.url()).thenReturn(GravitinoCommandLine.DEFAULT_URL); GrantPrivilegesToRole spyGrantRole = - spy( - new GrantPrivilegesToRole( - GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "admin", null, null)); + spy(new GrantPrivilegesToRole(mockContext, "metalake_demo", "admin", null, null)); assertThrows(RuntimeException.class, spyGrantRole::validate); verify(spyGrantRole, never()).handle(); String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); @@ -328,12 +322,7 @@ void testRevokePrivilegesFromRole() { doReturn(mockRevoke) .when(commandLine) .newRevokePrivilegesFromRole( - eq(GravitinoCommandLine.DEFAULT_URL), - eq(false), - eq("metalake_demo"), - eq("admin"), - any(), - eq(privileges)); + any(CommandContext.class), eq("metalake_demo"), eq("admin"), any(), eq(privileges)); doReturn(mockRevoke).when(mockRevoke).validate(); commandLine.handleCommandLine(); verify(mockRevoke).handle(); @@ -355,12 +344,7 @@ void testRevokeAllPrivilegesFromRole() { mockCommandLine, mockOptions, CommandEntities.ROLE, CommandActions.REVOKE)); doReturn(mockRevoke) .when(commandLine) - .newRevokeAllPrivileges( - eq(GravitinoCommandLine.DEFAULT_URL), - eq(false), - eq("metalake_demo"), - eq("admin"), - any()); + .newRevokeAllPrivileges(any(CommandContext.class), eq("metalake_demo"), eq("admin"), any()); doReturn(mockRevoke).when(mockRevoke).validate(); commandLine.handleCommandLine(); verify(mockRevoke).handle(); @@ -369,14 +353,14 @@ void testRevokeAllPrivilegesFromRole() { @Test void testRevokeAllPrivilegesFromRoleWithoutName() throws ParseException { Main.useExit = false; + CommandContext mockContext = mock(CommandContext.class); + when(mockContext.url()).thenReturn(GravitinoCommandLine.DEFAULT_URL); String[] args = {"role", "revoke", "-m", "metalake_demo", "--role", "admin", "--all"}; CommandLine commandLine = new DefaultParser().parse(options, args); FullName fullName = new FullName(commandLine); RevokeAllPrivileges spyRevokeAllPrivileges = - spy( - new RevokeAllPrivileges( - GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "admin", fullName)); + spy(new RevokeAllPrivileges(mockContext, "metalake_demo", "admin", fullName)); assertThrows(RuntimeException.class, spyRevokeAllPrivileges::validate); verify(spyRevokeAllPrivileges, never()).handle(); @@ -387,10 +371,10 @@ void testRevokeAllPrivilegesFromRoleWithoutName() throws ParseException { @Test void testRevokePrivilegesFromRoleWithoutPrivileges() { Main.useExit = false; + CommandContext mockContext = mock(CommandContext.class); + when(mockContext.url()).thenReturn(GravitinoCommandLine.DEFAULT_URL); RevokePrivilegesFromRole spyGrantRole = - spy( - new RevokePrivilegesFromRole( - GravitinoCommandLine.DEFAULT_URL, false, "metalake_demo", "admin", null, null)); + spy(new RevokePrivilegesFromRole(mockContext, "metalake_demo", "admin", null, null)); assertThrows(RuntimeException.class, spyGrantRole::validate); verify(spyGrantRole, never()).handle(); String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); @@ -410,8 +394,7 @@ void testDeleteRoleCommandWithoutRole() { assertThrows(RuntimeException.class, commandLine::handleCommandLine); verify(commandLine, never()) - .newDeleteRole( - eq(GravitinoCommandLine.DEFAULT_URL), eq(false), eq(false), eq("metalake_demo"), any()); + .newDeleteRole(any(CommandContext.class), eq("metalake_demo"), any()); String output = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim(); assertEquals(output, ErrorMessages.MISSING_ROLE); }