From 93a5ff39ddb46602353e6b0edbfde7b6a2be153e Mon Sep 17 00:00:00 2001 From: pancx Date: Wed, 26 Feb 2025 14:01:29 +0800 Subject: [PATCH 1/4] [#6136] improvement(CLI): Move check for enable and disable command in Gravitino CLI to command move check for enable and disable command in ManageCatalog. --- .../gravitino/cli/CatalogCommandHandler.java | 16 +---- .../apache/gravitino/cli/CommandContext.java | 8 +++ .../gravitino/cli/TestableCommandLine.java | 12 +--- .../cli/commands/CatalogDisable.java | 62 ------------------ ...{CatalogEnable.java => ManageCatalog.java} | 64 +++++++++++++++---- .../gravitino/cli/TestCatalogCommands.java | 25 ++------ 6 files changed, 74 insertions(+), 113 deletions(-) delete mode 100644 clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDisable.java rename clients/cli/src/main/java/org/apache/gravitino/cli/commands/{CatalogEnable.java => ManageCatalog.java} (57%) diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java index 4e5a50fd379..f8749720cd9 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/CatalogCommandHandler.java @@ -175,19 +175,9 @@ 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 enableMetalake = line.hasOption(GravitinoOptions.ALL); - gravitinoCommandLine - .newCatalogEnable(context, metalake, catalog, enableMetalake) - .validate() - .handle(); - } - if (line.hasOption(GravitinoOptions.DISABLE)) { - gravitinoCommandLine.newCatalogDisable(context, metalake, catalog).validate().handle(); + + if (line.hasOption(GravitinoOptions.ENABLE) || line.hasOption(GravitinoOptions.DISABLE)) { + gravitinoCommandLine.newManageCatalog(context, metalake, catalog).validate().handle(); } if (line.hasOption(GravitinoOptions.COMMENT)) { diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java b/clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java index 6f41e3697eb..1f92dd62c91 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/CommandContext.java @@ -115,6 +115,14 @@ public String auth() { return auth; } + /** + * Returns the command line. + * + * @return The command line. + */ + public CommandLine line() { + return line; + } /** * Retrieves the Gravitino URL from the command line options or the GRAVITINO_URL environment * variable or the Gravitino config file. 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 346e2691f42..54067b483d0 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 @@ -26,8 +26,6 @@ import org.apache.gravitino.cli.commands.AddRoleToUser; import org.apache.gravitino.cli.commands.CatalogAudit; import org.apache.gravitino.cli.commands.CatalogDetails; -import org.apache.gravitino.cli.commands.CatalogDisable; -import org.apache.gravitino.cli.commands.CatalogEnable; import org.apache.gravitino.cli.commands.ClientVersion; import org.apache.gravitino.cli.commands.ColumnAudit; import org.apache.gravitino.cli.commands.CreateCatalog; @@ -78,6 +76,7 @@ import org.apache.gravitino.cli.commands.ListTopicProperties; 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.MetalakeAudit; import org.apache.gravitino.cli.commands.MetalakeDetails; import org.apache.gravitino.cli.commands.MetalakeDisable; @@ -848,14 +847,9 @@ protected MetalakeDisable newMetalakeDisable(CommandContext context, String meta return new MetalakeDisable(context, metalake); } - protected CatalogEnable newCatalogEnable( - CommandContext context, String metalake, String catalog, boolean enableMetalake) { - return new CatalogEnable(context, metalake, catalog, enableMetalake); - } - - protected CatalogDisable newCatalogDisable( + protected ManageCatalog newManageCatalog( CommandContext context, String metalake, String catalog) { - return new CatalogDisable(context, metalake, catalog); + return new ManageCatalog(context, metalake, catalog); } protected ListModel newListModel( diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDisable.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDisable.java deleted file mode 100644 index 7a9954b1ee0..00000000000 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogDisable.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -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.NoSuchCatalogException; -import org.apache.gravitino.exceptions.NoSuchMetalakeException; - -/** Disable catalog. */ -public class CatalogDisable extends Command { - - private final String metalake; - private final String catalog; - - /** - * Disable catalog - * - * @param context The command context. - * @param metalake The name of the metalake. - * @param catalog The name of the catalog. - */ - public CatalogDisable(CommandContext context, String metalake, String catalog) { - super(context); - this.metalake = metalake; - this.catalog = catalog; - } - - /** Disable catalog. */ - @Override - public void handle() { - try { - GravitinoClient client = buildClient(metalake); - client.disableCatalog(catalog); - } catch (NoSuchMetalakeException noSuchMetalakeException) { - exitWithError(ErrorMessages.UNKNOWN_METALAKE); - } catch (NoSuchCatalogException noSuchCatalogException) { - exitWithError(ErrorMessages.UNKNOWN_CATALOG); - } catch (Exception exp) { - exitWithError(exp.getMessage()); - } - - printInformation(metalake + "." + catalog + " has been disabled."); - } -} diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogEnable.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java similarity index 57% rename from clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogEnable.java rename to clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java index 8c5ca513540..3ef1b2ed5ea 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/CatalogEnable.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java @@ -16,41 +16,68 @@ * specific language governing permissions and limitations * under the License. */ + package org.apache.gravitino.cli.commands; +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.cli.Main; import org.apache.gravitino.client.GravitinoAdminClient; import org.apache.gravitino.client.GravitinoClient; import org.apache.gravitino.exceptions.MetalakeNotInUseException; import org.apache.gravitino.exceptions.NoSuchCatalogException; import org.apache.gravitino.exceptions.NoSuchMetalakeException; -/** Enable catalog. */ -public class CatalogEnable extends Command { +/** Disable or enable a catalog. */ +public class ManageCatalog extends Command { private final String metalake; private final String catalog; + private final CommandLine line; private final boolean enableMetalake; /** - * Enable catalog + * Constrcut a new instance of the {@code ManageCatalog}. * - * @param context The command context. - * @param metalake The name of the metalake. - * @param catalog The name of the catalog. - * @param enableMetalake Whether to enable it's metalake + * @param context the command context. + * @param metalake the metalake name. + * @param catalog the catalog name. */ - public CatalogEnable( - CommandContext context, String metalake, String catalog, boolean enableMetalake) { + public ManageCatalog(CommandContext context, String metalake, String catalog) { super(context); this.metalake = metalake; this.catalog = catalog; - this.enableMetalake = enableMetalake; + this.line = context.line(); + this.enableMetalake = line.hasOption(GravitinoOptions.ALL); } - /** Enable catalog. */ + /** Disable or enable a catalog. */ @Override public void handle() { + if (line.hasOption(GravitinoOptions.ENABLE)) { + enableCatalog(); + } else if (line.hasOption(GravitinoOptions.DISABLE)) { + disableCatalog(); + } + } + + /** + * Validate the command line options. + * + * @return the command instance. + */ + @Override + public Command validate() { + if (line.hasOption(GravitinoOptions.ENABLE) && line.hasOption(GravitinoOptions.DISABLE)) { + printInformation(ErrorMessages.INVALID_ENABLE_DISABLE); + Main.exit(-1); + } + + return super.validate(); + } + + private void enableCatalog() { try { if (enableMetalake) { GravitinoAdminClient adminClient = buildAdminClient(); @@ -71,4 +98,19 @@ public void handle() { printInformation(metalake + "." + catalog + " has been enabled."); } + + private void disableCatalog() { + try { + GravitinoClient client = buildClient(metalake); + client.disableCatalog(catalog); + } catch (NoSuchMetalakeException noSuchMetalakeException) { + exitWithError(ErrorMessages.UNKNOWN_METALAKE); + } catch (NoSuchCatalogException noSuchCatalogException) { + exitWithError(ErrorMessages.UNKNOWN_CATALOG); + } catch (Exception exp) { + exitWithError(exp.getMessage()); + } + + printInformation(metalake + "." + catalog + " has been disabled."); + } } diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java index 26639368a6a..f42059c3d43 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java @@ -21,9 +21,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThrows; -import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.anyBoolean; import static org.mockito.ArgumentMatchers.argThat; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; @@ -41,12 +39,11 @@ import org.apache.commons.cli.Options; import org.apache.gravitino.cli.commands.CatalogAudit; import org.apache.gravitino.cli.commands.CatalogDetails; -import org.apache.gravitino.cli.commands.CatalogDisable; -import org.apache.gravitino.cli.commands.CatalogEnable; import org.apache.gravitino.cli.commands.CreateCatalog; import org.apache.gravitino.cli.commands.DeleteCatalog; import org.apache.gravitino.cli.commands.ListCatalogProperties; import org.apache.gravitino.cli.commands.ListCatalogs; +import org.apache.gravitino.cli.commands.ManageCatalog; import org.apache.gravitino.cli.commands.RemoveCatalogProperty; import org.apache.gravitino.cli.commands.SetCatalogProperty; import org.apache.gravitino.cli.commands.UpdateCatalogComment; @@ -419,7 +416,7 @@ void testCatalogDetailsCommandWithoutCatalog() { @Test void testEnableCatalogCommand() { - CatalogEnable mockEnable = mock(CatalogEnable.class); + ManageCatalog mockEnable = mock(ManageCatalog.class); when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true); @@ -432,8 +429,7 @@ void testEnableCatalogCommand() { mockCommandLine, mockOptions, CommandEntities.CATALOG, CommandActions.UPDATE)); doReturn(mockEnable) .when(commandLine) - .newCatalogEnable( - any(CommandContext.class), eq("metalake_demo"), eq("catalog"), anyBoolean()); + .newManageCatalog(any(CommandContext.class), eq("metalake_demo"), eq("catalog")); doReturn(mockEnable).when(mockEnable).validate(); commandLine.handleCommandLine(); verify(mockEnable).handle(); @@ -441,7 +437,7 @@ void testEnableCatalogCommand() { @Test void testEnableCatalogCommandWithRecursive() { - CatalogEnable mockEnable = mock(CatalogEnable.class); + ManageCatalog mockEnable = mock(ManageCatalog.class); when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true); @@ -455,8 +451,7 @@ void testEnableCatalogCommandWithRecursive() { mockCommandLine, mockOptions, CommandEntities.CATALOG, CommandActions.UPDATE)); doReturn(mockEnable) .when(commandLine) - .newCatalogEnable( - any(CommandContext.class), eq("metalake_demo"), eq("catalog"), anyBoolean()); + .newManageCatalog(any(CommandContext.class), eq("metalake_demo"), eq("catalog")); doReturn(mockEnable).when(mockEnable).validate(); commandLine.handleCommandLine(); verify(mockEnable).handle(); @@ -464,7 +459,7 @@ void testEnableCatalogCommandWithRecursive() { @Test void testDisableCatalogCommand() { - CatalogDisable mockDisable = mock(CatalogDisable.class); + ManageCatalog mockDisable = mock(ManageCatalog.class); when(mockCommandLine.hasOption(GravitinoOptions.METALAKE)).thenReturn(true); when(mockCommandLine.getOptionValue(GravitinoOptions.METALAKE)).thenReturn("metalake_demo"); when(mockCommandLine.hasOption(GravitinoOptions.NAME)).thenReturn(true); @@ -477,7 +472,7 @@ void testDisableCatalogCommand() { mockCommandLine, mockOptions, CommandEntities.CATALOG, CommandActions.UPDATE)); doReturn(mockDisable) .when(commandLine) - .newCatalogDisable(any(CommandContext.class), eq("metalake_demo"), eq("catalog")); + .newManageCatalog(any(CommandContext.class), eq("metalake_demo"), eq("catalog")); doReturn(mockDisable).when(mockDisable).validate(); commandLine.handleCommandLine(); verify(mockDisable).handle(); @@ -500,11 +495,5 @@ void testCatalogWithDisableAndEnableOptions() { mockCommandLine, mockOptions, CommandEntities.CATALOG, CommandActions.UPDATE)); assertThrows(RuntimeException.class, commandLine::handleCommandLine); - verify(commandLine, never()) - .newCatalogEnable( - any(CommandContext.class), eq("metalake_demo"), eq("catalog"), anyBoolean()); - verify(commandLine, never()) - .newCatalogDisable(any(CommandContext.class), eq("metalake_demo"), eq("catalog")); - assertTrue(errContent.toString().contains(ErrorMessages.INVALID_ENABLE_DISABLE)); } } From 864177f15584ae5afc1c4ce789399bca29ec54c7 Mon Sep 17 00:00:00 2001 From: pancx Date: Wed, 26 Feb 2025 14:22:02 +0800 Subject: [PATCH 2/4] [#6136] improvement(CLI): Move check for enable and disable command in Gravitino CLI to command move check for enable and disable command in ManageMetalake. --- .../gravitino/cli/MetalakeCommandHandler.java | 15 +---- .../gravitino/cli/TestableCommandLine.java | 12 +--- .../gravitino/cli/commands/ManageCatalog.java | 12 ++-- ...etalakeEnable.java => ManageMetalake.java} | 56 +++++++++++++++---- .../cli/commands/MetalakeDisable.java | 56 ------------------- .../gravitino/cli/TestCatalogCommands.java | 3 + .../gravitino/cli/TestMetalakeCommands.java | 28 ++++------ 7 files changed, 69 insertions(+), 113 deletions(-) rename clients/cli/src/main/java/org/apache/gravitino/cli/commands/{MetalakeEnable.java => ManageMetalake.java} (58%) delete mode 100644 clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDisable.java diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java index 218f6e14588..c76f875279b 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/MetalakeCommandHandler.java @@ -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)) { 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 54067b483d0..dcb8e1bc24b 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 @@ -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; @@ -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( diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java index 3ef1b2ed5ea..2e563513916 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageCatalog.java @@ -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; @@ -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) { @@ -99,6 +94,7 @@ private void enableCatalog() { printInformation(metalake + "." + catalog + " has been enabled."); } + /** Disable a catalog. */ private void disableCatalog() { try { GravitinoClient client = buildClient(metalake); diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeEnable.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageMetalake.java similarity index 58% rename from clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeEnable.java rename to clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageMetalake.java index f402c376081..35651838b52 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeEnable.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/ManageMetalake.java @@ -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(); @@ -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."); + } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDisable.java b/clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDisable.java deleted file mode 100644 index ad05d293433..00000000000 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/commands/MetalakeDisable.java +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.gravitino.cli.commands; - -import org.apache.gravitino.cli.CommandContext; -import org.apache.gravitino.cli.ErrorMessages; -import org.apache.gravitino.client.GravitinoAdminClient; -import org.apache.gravitino.exceptions.NoSuchMetalakeException; - -/** Disable metalake. */ -public class MetalakeDisable extends Command { - private String metalake; - - /** - * Disable metalake - * - * @param context The command context. - * @param metalake The name of the metalake. - */ - public MetalakeDisable(CommandContext context, String metalake) { - super(context); - this.metalake = metalake; - } - - /** Disable metalake. */ - @Override - public void handle() { - 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."); - } -} diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java index f42059c3d43..553820db1b7 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestCatalogCommands.java @@ -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; @@ -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); } } diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java index e01b652c644..4707d24387e 100644 --- a/clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestMetalakeCommands.java @@ -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; @@ -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; @@ -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); @@ -384,7 +381,7 @@ 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(); @@ -392,7 +389,7 @@ void testEnableMetalakeCommand() { @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); @@ -403,7 +400,7 @@ 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(); @@ -411,7 +408,7 @@ void testEnableMetalakeCommandWithRecursive() { @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); @@ -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(); @@ -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); } } From 8fa11c00db2bac35891152f76ef1e655ef02b5e2 Mon Sep 17 00:00:00 2001 From: pancx Date: Wed, 26 Feb 2025 16:37:45 +0800 Subject: [PATCH 3/4] [#6136] improvement(CLI): Move check for enable and disable command in Gravitino CLI to command fix some bugs. --- .../src/main/java/org/apache/gravitino/cli/ErrorMessages.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java index 554f3a8503c..c1a36398a36 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java @@ -39,7 +39,7 @@ public class ErrorMessages { public static final String ENTITY_IN_USE = " in use, please disable it first."; public static final String INVALID_ENABLE_DISABLE = - "Unable to us --enable and --disable at the same time"; + "The --enable and the --disable options are mutual exclusive."; public static final String INVALID_OWNER_COMMAND = "Unsupported combination of options either use --user or --group."; public static final String INVALID_REMOVE_COMMAND = From d0e2c593176fc25295b51f22e0887b3d82963243 Mon Sep 17 00:00:00 2001 From: pancx Date: Wed, 26 Feb 2025 17:05:03 +0800 Subject: [PATCH 4/4] [#6136] improvement(CLI): Move check for enable and disable command in Gravitino CLI to command fix some bugs. --- .../src/main/java/org/apache/gravitino/cli/ErrorMessages.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java index c1a36398a36..0ba98c0c77f 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/ErrorMessages.java @@ -39,7 +39,7 @@ public class ErrorMessages { public static final String ENTITY_IN_USE = " in use, please disable it first."; public static final String INVALID_ENABLE_DISABLE = - "The --enable and the --disable options are mutual exclusive."; + "Unable to use --enable and --disable at the same time."; public static final String INVALID_OWNER_COMMAND = "Unsupported combination of options either use --user or --group."; public static final String INVALID_REMOVE_COMMAND =