Skip to content

Commit

Permalink
[#6136] improvement(CLI): Move check for enable and disable command i…
Browse files Browse the repository at this point in the history
…n Gravitino CLI to command

move check for enable and disable command in ManageMetalake.
  • Loading branch information
Abyss-lord committed Feb 26, 2025
1 parent 93a5ff3 commit 864177f
Show file tree
Hide file tree
Showing 7 changed files with 69 additions and 113 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -159,19 +159,8 @@ private void handlePropertiesCommand() {

/** Handles the "UPDATE" command. */
private void handleUpdateCommand() {
if (line.hasOption(GravitinoOptions.ENABLE) && line.hasOption(GravitinoOptions.DISABLE)) {
System.err.println(ErrorMessages.INVALID_ENABLE_DISABLE);
Main.exit(-1);
}
if (line.hasOption(GravitinoOptions.ENABLE)) {
boolean enableAllCatalogs = line.hasOption(GravitinoOptions.ALL);
gravitinoCommandLine
.newMetalakeEnable(context, metalake, enableAllCatalogs)
.validate()
.handle();
}
if (line.hasOption(GravitinoOptions.DISABLE)) {
gravitinoCommandLine.newMetalakeDisable(context, metalake).validate().handle();
if (line.hasOption(GravitinoOptions.ENABLE) || line.hasOption(GravitinoOptions.DISABLE)) {
gravitinoCommandLine.newManageMetalake(context, metalake).validate().handle();
}

if (line.hasOption(GravitinoOptions.COMMENT)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,9 @@
import org.apache.gravitino.cli.commands.ListTopics;
import org.apache.gravitino.cli.commands.ListUsers;
import org.apache.gravitino.cli.commands.ManageCatalog;
import org.apache.gravitino.cli.commands.ManageMetalake;
import org.apache.gravitino.cli.commands.MetalakeAudit;
import org.apache.gravitino.cli.commands.MetalakeDetails;
import org.apache.gravitino.cli.commands.MetalakeDisable;
import org.apache.gravitino.cli.commands.MetalakeEnable;
import org.apache.gravitino.cli.commands.ModelAudit;
import org.apache.gravitino.cli.commands.ModelDetails;
import org.apache.gravitino.cli.commands.OwnerDetails;
Expand Down Expand Up @@ -838,13 +837,8 @@ protected RevokeAllPrivileges newRevokeAllPrivileges(
return new RevokeAllPrivileges(context, metalake, role, entity);
}

protected MetalakeEnable newMetalakeEnable(
CommandContext context, String metalake, boolean enableAllCatalogs) {
return new MetalakeEnable(context, metalake, enableAllCatalogs);
}

protected MetalakeDisable newMetalakeDisable(CommandContext context, String metalake) {
return new MetalakeDisable(context, metalake);
protected ManageMetalake newManageMetalake(CommandContext context, String metalake) {
return new ManageMetalake(context, metalake);
}

protected ManageCatalog newManageCatalog(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import org.apache.gravitino.cli.CommandContext;
import org.apache.gravitino.cli.ErrorMessages;
import org.apache.gravitino.cli.GravitinoOptions;
import org.apache.gravitino.cli.Main;
import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoClient;
import org.apache.gravitino.exceptions.MetalakeNotInUseException;
Expand Down Expand Up @@ -62,21 +61,17 @@ public void handle() {
}
}

/**
* Validate the command line options.
*
* @return the command instance.
*/
/** {@inheritDoc} */
@Override
public Command validate() {
if (line.hasOption(GravitinoOptions.ENABLE) && line.hasOption(GravitinoOptions.DISABLE)) {
printInformation(ErrorMessages.INVALID_ENABLE_DISABLE);
Main.exit(-1);
exitWithError(ErrorMessages.INVALID_ENABLE_DISABLE);
}

return super.validate();
}

/** Enable a catalog. */
private void enableCatalog() {
try {
if (enableMetalake) {
Expand All @@ -99,6 +94,7 @@ private void enableCatalog() {
printInformation(metalake + "." + catalog + " has been enabled.");
}

/** Disable a catalog. */
private void disableCatalog() {
try {
GravitinoClient client = buildClient(metalake);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,34 +20,56 @@
package org.apache.gravitino.cli.commands;

import java.util.Arrays;
import org.apache.commons.cli.CommandLine;
import org.apache.gravitino.cli.CommandContext;
import org.apache.gravitino.cli.ErrorMessages;
import org.apache.gravitino.cli.GravitinoOptions;
import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;

/** Enable metalake. */
public class MetalakeEnable extends Command {

/** Disable or enable metalake. */
public class ManageMetalake extends Command {
private final String metalake;
private final CommandLine line;
private Boolean enableAllCatalogs;

/**
* Enable a metalake
* Construct a new instance of the {@code ManageMetalake}.
*
* @param context The command context.
* @param metalake The name of the metalake.
* @param enableAllCatalogs Whether to enable all catalogs.
* @param context the command context.
* @param metalake the name of the metalake.
*/
public MetalakeEnable(CommandContext context, String metalake, boolean enableAllCatalogs) {
public ManageMetalake(CommandContext context, String metalake) {
super(context);
this.metalake = metalake;
this.enableAllCatalogs = enableAllCatalogs;
this.line = context.line();

this.enableAllCatalogs = line.hasOption(GravitinoOptions.ALL);
}

/** Enable metalake. */
/** Disable or enable the metalake. */
@Override
public void handle() {
if (line.hasOption(GravitinoOptions.ENABLE)) {
enableMetalake();
} else if (line.hasOption(GravitinoOptions.DISABLE)) {
disableMetalake();
}
}

/** {@inheritDoc} */
@Override
public Command validate() {
if (line.hasOption(GravitinoOptions.ENABLE) && line.hasOption(GravitinoOptions.DISABLE)) {
exitWithError(ErrorMessages.INVALID_ENABLE_DISABLE);
}

return super.validate();
}

/** Enable the metalake. */
private void enableMetalake() {
StringBuilder msgBuilder = new StringBuilder(metalake);
try {
GravitinoAdminClient client = buildAdminClient();
Expand All @@ -68,4 +90,18 @@ public void handle() {

printInformation(msgBuilder.toString());
}

/** Disable the metalake. */
private void disableMetalake() {
try {
GravitinoAdminClient client = buildAdminClient();
client.disableMetalake(metalake);
} catch (NoSuchMetalakeException err) {
exitWithError(ErrorMessages.UNKNOWN_METALAKE);
} catch (Exception exp) {
exitWithError(exp.getMessage());
}

printInformation(metalake + " has been disabled.");
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
import org.apache.gravitino.cli.commands.UpdateCatalogComment;
import org.apache.gravitino.cli.commands.UpdateCatalogName;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -495,5 +496,7 @@ void testCatalogWithDisableAndEnableOptions() {
mockCommandLine, mockOptions, CommandEntities.CATALOG, CommandActions.UPDATE));

assertThrows(RuntimeException.class, commandLine::handleCommandLine);
String errorOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(ErrorMessages.INVALID_ENABLE_DISABLE, errorOutput);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.eq;
import static org.mockito.Mockito.isNull;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.when;
Expand All @@ -41,16 +39,15 @@
import org.apache.gravitino.cli.commands.DeleteMetalake;
import org.apache.gravitino.cli.commands.ListMetalakeProperties;
import org.apache.gravitino.cli.commands.ListMetalakes;
import org.apache.gravitino.cli.commands.ManageMetalake;
import org.apache.gravitino.cli.commands.MetalakeAudit;
import org.apache.gravitino.cli.commands.MetalakeDetails;
import org.apache.gravitino.cli.commands.MetalakeDisable;
import org.apache.gravitino.cli.commands.MetalakeEnable;
import org.apache.gravitino.cli.commands.RemoveMetalakeProperty;
import org.apache.gravitino.cli.commands.SetMetalakeProperty;
import org.apache.gravitino.cli.commands.UpdateMetalakeComment;
import org.apache.gravitino.cli.commands.UpdateMetalakeName;
import org.junit.Assert;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assertions;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -374,7 +371,7 @@ void testUpdateMetalakeNameForceCommand() {

@Test
void testEnableMetalakeCommand() {
MetalakeEnable mockEnable = mock(MetalakeEnable.class);
ManageMetalake mockEnable = mock(ManageMetalake.class);
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.ENABLE)).thenReturn(true);
Expand All @@ -384,15 +381,15 @@ void testEnableMetalakeCommand() {
mockCommandLine, mockOptions, CommandEntities.METALAKE, CommandActions.UPDATE));
doReturn(mockEnable)
.when(commandLine)
.newMetalakeEnable(any(CommandContext.class), eq("metalake_demo"), eq(false));
.newManageMetalake(any(CommandContext.class), eq("metalake_demo"));
doReturn(mockEnable).when(mockEnable).validate();
commandLine.handleCommandLine();
verify(mockEnable).handle();
}

@Test
void testEnableMetalakeCommandWithRecursive() {
MetalakeEnable mockEnable = mock(MetalakeEnable.class);
ManageMetalake mockEnable = mock(ManageMetalake.class);
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.ALL)).thenReturn(true);
Expand All @@ -403,15 +400,15 @@ void testEnableMetalakeCommandWithRecursive() {
mockCommandLine, mockOptions, CommandEntities.METALAKE, CommandActions.UPDATE));
doReturn(mockEnable)
.when(commandLine)
.newMetalakeEnable(any(CommandContext.class), eq("metalake_demo"), eq(true));
.newManageMetalake(any(CommandContext.class), eq("metalake_demo"));
doReturn(mockEnable).when(mockEnable).validate();
commandLine.handleCommandLine();
verify(mockEnable).handle();
}

@Test
void testDisableMetalakeCommand() {
MetalakeDisable mockDisable = mock(MetalakeDisable.class);
ManageMetalake mockDisable = mock(ManageMetalake.class);
when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true);
when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo");
when(mockCommandLine.hasOption(GravitinoOptions.DISABLE)).thenReturn(true);
Expand All @@ -422,7 +419,7 @@ void testDisableMetalakeCommand() {
mockCommandLine, mockOptions, CommandEntities.METALAKE, CommandActions.UPDATE));
doReturn(mockDisable)
.when(commandLine)
.newMetalakeDisable(any(CommandContext.class), eq("metalake_demo"));
.newManageMetalake(any(CommandContext.class), eq("metalake_demo"));
doReturn(mockDisable).when(mockDisable).validate();
commandLine.handleCommandLine();
verify(mockDisable).handle();
Expand All @@ -442,11 +439,8 @@ void testMetalakeWithDisableAndEnableOptions() {
new GravitinoCommandLine(
mockCommandLine, mockOptions, CommandEntities.METALAKE, CommandActions.UPDATE));

Assert.assertThrows(RuntimeException.class, commandLine::handleCommandLine);
verify(commandLine, never())
.newMetalakeEnable(any(CommandContext.class), eq("metalake_demo"), eq(false));
verify(commandLine, never())
.newMetalakeEnable(any(CommandContext.class), eq("metalake_demo"), eq(false));
assertTrue(errContent.toString().contains(ErrorMessages.INVALID_ENABLE_DISABLE));
Assertions.assertThrows(RuntimeException.class, commandLine::handleCommandLine);
String errorOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
Assertions.assertEquals(ErrorMessages.INVALID_ENABLE_DISABLE, errorOutput);
}
}

0 comments on commit 864177f

Please sign in to comment.