From 3a48abad1baafee28531519b6d2ff5c4d92c7c2e Mon Sep 17 00:00:00 2001 From: Lord of Abyss <103809695+Abyss-lord@users.noreply.github.com> Date: Wed, 15 Jan 2025 06:15:43 +0800 Subject: [PATCH] [#6220] improve(CLI): Clean up GravitinoCommandLine class now it been refactored (#6227) ### What changes were proposed in this pull request? Clean up GravitinoCommandLine class now it been refactored ### Why are the changes needed? Fix: #6220 ### Does this PR introduce _any_ user-facing change? No ### How was this patch tested? local ut test. --- .../gravitino/cli/ColumnCommandHandler.java | 4 +- .../gravitino/cli/FilesetCommandHandler.java | 4 +- .../gravitino/cli/GravitinoCommandLine.java | 98 +------------------ .../gravitino/cli/SimpleCommandHandler.java | 53 ++++++++++ .../gravitino/cli/TestSimpleCommands.java | 75 ++++++++++++++ 5 files changed, 134 insertions(+), 100 deletions(-) create mode 100644 clients/cli/src/main/java/org/apache/gravitino/cli/SimpleCommandHandler.java create mode 100644 clients/cli/src/test/java/org/apache/gravitino/cli/TestSimpleCommands.java diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/ColumnCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/ColumnCommandHandler.java index 96f056c1a3c..c0775dae966 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/ColumnCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/ColumnCommandHandler.java @@ -53,7 +53,7 @@ public ColumnCommandHandler( this.command = command; this.ignore = ignore; - this.url = gravitinoCommandLine.getUrl(); + this.url = getUrl(line); this.name = new FullName(line); this.metalake = name.getMetalakeName(); this.catalog = name.getCatalogName(); @@ -65,7 +65,7 @@ public ColumnCommandHandler( @Override protected void handle() { String userName = line.getOptionValue(GravitinoOptions.LOGIN); - Command.setAuthenticationMode(gravitinoCommandLine.getAuth(), userName); + Command.setAuthenticationMode(getAuth(line), userName); List missingEntities = Lists.newArrayList(); if (catalog == null) missingEntities.add(CommandEntities.CATALOG); diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/FilesetCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/FilesetCommandHandler.java index 33fc1fe9ee7..dce797294d8 100644 --- a/clients/cli/src/main/java/org/apache/gravitino/cli/FilesetCommandHandler.java +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/FilesetCommandHandler.java @@ -54,7 +54,7 @@ public FilesetCommandHandler( this.command = command; this.ignore = ignore; - this.url = gravitinoCommandLine.getUrl(); + this.url = getUrl(line); this.name = new FullName(line); this.metalake = name.getMetalakeName(); this.catalog = name.getCatalogName(); @@ -65,7 +65,7 @@ public FilesetCommandHandler( @Override protected void handle() { String userName = line.getOptionValue(GravitinoOptions.LOGIN); - Command.setAuthenticationMode(gravitinoCommandLine.getAuth(), userName); + Command.setAuthenticationMode(getAuth(line), userName); List missingEntities = Lists.newArrayList(); if (catalog == null) missingEntities.add(CommandEntities.CATALOG); 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 11737206067..d7e257a8a81 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 @@ -19,13 +19,11 @@ package org.apache.gravitino.cli; -import com.google.common.base.Joiner; import java.io.BufferedReader; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.nio.charset.StandardCharsets; -import java.util.List; import org.apache.commons.cli.CommandLine; import org.apache.commons.cli.HelpFormatter; import org.apache.commons.cli.Options; @@ -37,18 +35,13 @@ public class GravitinoCommandLine extends TestableCommandLine { private final Options options; private final String entity; private final String command; - private String urlEnv; - private boolean urlSet = false; private boolean ignore = false; private String ignoreEnv; private boolean ignoreSet = false; - private String authEnv; - private boolean authSet = false; public static final String CMD = "gcli"; // recommended name public static final String DEFAULT_URL = "http://localhost:8090"; // This joiner is used to join multiple outputs to be displayed, e.g. roles or groups - private static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls(); /** * Gravitino Command line. @@ -97,14 +90,8 @@ public void handleSimpleLine() { /* Display command usage. */ if (line.hasOption(GravitinoOptions.HELP)) { displayHelp(options); - } - /* Display Gravitino client version. */ - else if (line.hasOption(GravitinoOptions.VERSION)) { - newClientVersion(getUrl(), ignore).handle(); - } - /* Display Gravitino server version. */ - else if (line.hasOption(GravitinoOptions.SERVER)) { - newServerVersion(getUrl(), ignore).handle(); + } else { + new SimpleCommandHandler(this, line, ignore).handle(); } } @@ -168,85 +155,4 @@ private void handleHelpCommand() { Main.exit(-1); } } - - /** - * Retrieves the Gravitinno URL from the command line options or the GRAVITINO_URL environment - * variable or the Gravitio config file. - * - * @return The Gravitinno URL, or null if not found. - */ - public String getUrl() { - GravitinoConfig config = new GravitinoConfig(null); - - // If specified on the command line use that - if (line.hasOption(GravitinoOptions.URL)) { - return line.getOptionValue(GravitinoOptions.URL); - } - - // Cache the Gravitino URL environment variable - if (urlEnv == null && !urlSet) { - urlEnv = System.getenv("GRAVITINO_URL"); - urlSet = true; - } - - // If set return the Gravitino URL environment variable - if (urlEnv != null) { - return urlEnv; - } - - // Check if the Gravitino URL is specified in the configuration file - if (config.fileExists()) { - config.read(); - String configURL = config.getGravitinoURL(); - if (configURL != null) { - return configURL; - } - } - - // Return the default localhost URL - return DEFAULT_URL; - } - - /** - * Retrieves the Gravitinno authentication from the command line options or the GRAVITINO_AUTH - * environment variable or the Gravitio config file. - * - * @return The Gravitinno authentication, or null if not found. - */ - public String getAuth() { - // If specified on the command line use that - if (line.hasOption(GravitinoOptions.SIMPLE)) { - return GravitinoOptions.SIMPLE; - } - - // Cache the Gravitino authentication type environment variable - if (authEnv == null && !authSet) { - authEnv = System.getenv("GRAVITINO_AUTH"); - authSet = true; - } - - // If set return the Gravitino authentication type environment variable - if (authEnv != null) { - return authEnv; - } - - // Check if the authentication type is specified in the configuration file - GravitinoConfig config = new GravitinoConfig(null); - if (config.fileExists()) { - config.read(); - String configAuthType = config.getGravitinoAuthType(); - if (configAuthType != null) { - return configAuthType; - } - } - - return null; - } - - private void checkEntities(List entities) { - if (!entities.isEmpty()) { - System.err.println(ErrorMessages.MISSING_ENTITIES + COMMA_JOINER.join(entities)); - Main.exit(-1); - } - } } diff --git a/clients/cli/src/main/java/org/apache/gravitino/cli/SimpleCommandHandler.java b/clients/cli/src/main/java/org/apache/gravitino/cli/SimpleCommandHandler.java new file mode 100644 index 00000000000..48aca9f9569 --- /dev/null +++ b/clients/cli/src/main/java/org/apache/gravitino/cli/SimpleCommandHandler.java @@ -0,0 +1,53 @@ +/* + * 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; + +import org.apache.commons.cli.CommandLine; + +/** Handles the command execution for simple command based on the command line options. */ +public class SimpleCommandHandler extends CommandHandler { + private final GravitinoCommandLine gravitinoCommandLine; + private final CommandLine line; + private final boolean ignore; + + /** + * Constructs a {@link SimpleCommandHandler} instance. + * + * @param gravitinoCommandLine The Gravitino command line instance. + * @param line The command line arguments. + * @param ignore Ignore server version mismatch. + */ + public SimpleCommandHandler( + GravitinoCommandLine gravitinoCommandLine, CommandLine line, boolean ignore) { + this.gravitinoCommandLine = gravitinoCommandLine; + this.line = line; + this.ignore = ignore; + } + + /** Handles the command execution logic based on the provided command. */ + @Override + protected void handle() { + if (line.hasOption(GravitinoOptions.VERSION)) { + gravitinoCommandLine.newClientVersion(getUrl(line), ignore).validate().handle(); + } else if (line.hasOption(GravitinoOptions.SERVER)) { + gravitinoCommandLine.newServerVersion(getUrl(line), ignore).validate().handle(); + } + } +} diff --git a/clients/cli/src/test/java/org/apache/gravitino/cli/TestSimpleCommands.java b/clients/cli/src/test/java/org/apache/gravitino/cli/TestSimpleCommands.java new file mode 100644 index 00000000000..044e06c58f7 --- /dev/null +++ b/clients/cli/src/test/java/org/apache/gravitino/cli/TestSimpleCommands.java @@ -0,0 +1,75 @@ +/* + * 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; + +import static org.mockito.Mockito.doReturn; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import org.apache.commons.cli.CommandLine; +import org.apache.commons.cli.Options; +import org.apache.gravitino.cli.commands.ClientVersion; +import org.apache.gravitino.cli.commands.ServerVersion; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +public class TestSimpleCommands { + + private CommandLine mockCommandLine; + private Options mockOptions; + + @BeforeEach + void setUp() { + mockCommandLine = mock(CommandLine.class); + mockOptions = mock(Options.class); + } + + @Test + void testServerVersion() { + ServerVersion mockServerVersion = mock(ServerVersion.class); + when(mockCommandLine.hasOption(GravitinoOptions.SERVER)).thenReturn(true); + GravitinoCommandLine commandLine = + spy(new GravitinoCommandLine(mockCommandLine, mockOptions, null, null)); + + doReturn(mockServerVersion) + .when(commandLine) + .newServerVersion(GravitinoCommandLine.DEFAULT_URL, false); + doReturn(mockServerVersion).when(mockServerVersion).validate(); + commandLine.handleSimpleLine(); + verify(mockServerVersion).handle(); + } + + @Test + void testClientVersion() { + ClientVersion mockClientVersion = mock(ClientVersion.class); + when(mockCommandLine.hasOption(GravitinoOptions.VERSION)).thenReturn(true); + GravitinoCommandLine commandLine = + spy(new GravitinoCommandLine(mockCommandLine, mockOptions, null, null)); + + doReturn(mockClientVersion) + .when(commandLine) + .newClientVersion(GravitinoCommandLine.DEFAULT_URL, false); + doReturn(mockClientVersion).when(mockClientVersion).validate(); + commandLine.handleSimpleLine(); + verify(mockClientVersion).handle(); + } +}