Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#6086] fix(CLI): Refactor the validation logic of Metalake #6091

Merged
merged 15 commits into from
Jan 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@ public class ErrorMessages {
public static final String MISSING_ROLE = "Missing --role option.";
public static final String MISSING_TAG = "Missing --tag option.";
public static final String MISSING_URI = "Missing --uri option.";
public static final String MISSING_PROPERTY = "Missing --property option.";
public static final String MISSING_VALUE = "Missing --value option.";
public static final String METALAKE_EXISTS = "Metalake already exists.";
public static final String CATALOG_EXISTS = "Catalog already exists.";
public static final String SCHEMA_EXISTS = "Schema already exists.";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ public String getMetalakeName() {
}

System.err.println(ErrorMessages.MISSING_METALAKE);
Main.exit(-1);

return null;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import org.apache.commons.cli.CommandLine;
import org.apache.commons.cli.HelpFormatter;
import org.apache.commons.cli.Options;
Expand Down Expand Up @@ -167,51 +166,49 @@ private void handleMetalakeCommand() {
String auth = getAuth();
String userName = line.getOptionValue(GravitinoOptions.LOGIN);
FullName name = new FullName(line);
String metalake = name.getMetalakeName();
String outputFormat = line.getOptionValue(GravitinoOptions.OUTPUT);

Command.setAuthenticationMode(auth, userName);

if (CommandActions.LIST.equals(command)) {
newListMetalakes(url, ignore, outputFormat).validate().handle();
return;
}

String metalake = name.getMetalakeName();

switch (command) {
case CommandActions.DETAILS:
if (line.hasOption(GravitinoOptions.AUDIT)) {
newMetalakeAudit(url, ignore, metalake).handle();
newMetalakeAudit(url, ignore, metalake).validate().handle();
} else {
newMetalakeDetails(url, ignore, outputFormat, metalake).handle();
newMetalakeDetails(url, ignore, outputFormat, metalake).validate().handle();
}
break;

case CommandActions.LIST:
newListMetalakes(url, ignore, outputFormat).handle();
break;

case CommandActions.CREATE:
if (Objects.isNull(metalake)) {
System.err.println(CommandEntities.METALAKE + " is not defined");
Main.exit(-1);
}
String comment = line.getOptionValue(GravitinoOptions.COMMENT);
newCreateMetalake(url, ignore, metalake, comment).handle();
newCreateMetalake(url, ignore, metalake, comment).validate().handle();
break;

case CommandActions.DELETE:
boolean force = line.hasOption(GravitinoOptions.FORCE);
newDeleteMetalake(url, ignore, force, metalake).handle();
newDeleteMetalake(url, ignore, force, metalake).validate().handle();
break;

case CommandActions.SET:
String property = line.getOptionValue(GravitinoOptions.PROPERTY);
String value = line.getOptionValue(GravitinoOptions.VALUE);
newSetMetalakeProperty(url, ignore, metalake, property, value).handle();
newSetMetalakeProperty(url, ignore, metalake, property, value).validate().handle();
break;

case CommandActions.REMOVE:
property = line.getOptionValue(GravitinoOptions.PROPERTY);
newRemoveMetalakeProperty(url, ignore, metalake, property).handle();
newRemoveMetalakeProperty(url, ignore, metalake, property).validate().handle();
break;

case CommandActions.PROPERTIES:
newListMetalakeProperties(url, ignore, metalake).handle();
newListMetalakeProperties(url, ignore, metalake).validate().handle();
break;

case CommandActions.UPDATE:
Expand All @@ -221,21 +218,22 @@ private void handleMetalakeCommand() {
}
if (line.hasOption(GravitinoOptions.ENABLE)) {
boolean enableAllCatalogs = line.hasOption(GravitinoOptions.ALL);
newMetalakeEnable(url, ignore, metalake, enableAllCatalogs).handle();
newMetalakeEnable(url, ignore, metalake, enableAllCatalogs).validate().handle();
}
if (line.hasOption(GravitinoOptions.DISABLE)) {
newMetalakeDisable(url, ignore, metalake).handle();
newMetalakeDisable(url, ignore, metalake).validate().handle();
}

if (line.hasOption(GravitinoOptions.COMMENT)) {
comment = line.getOptionValue(GravitinoOptions.COMMENT);
newUpdateMetalakeComment(url, ignore, metalake, comment).handle();
newUpdateMetalakeComment(url, ignore, metalake, comment).validate().handle();
}
if (line.hasOption(GravitinoOptions.RENAME)) {
String newName = line.getOptionValue(GravitinoOptions.RENAME);
force = line.hasOption(GravitinoOptions.FORCE);
newUpdateMetalakeName(url, ignore, force, metalake, newName).handle();
newUpdateMetalakeName(url, ignore, force, metalake, newName).validate().handle();
}

break;

default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

import static org.apache.gravitino.client.GravitinoClientBase.Builder;

import com.google.common.base.Joiner;
import java.io.File;
import org.apache.gravitino.cli.GravitinoConfig;
import org.apache.gravitino.cli.KerberosData;
Expand All @@ -39,14 +40,14 @@
public abstract class Command {
public static final String OUTPUT_FORMAT_TABLE = "table";
public static final String OUTPUT_FORMAT_PLAIN = "plain";
public static final Joiner COMMA_JOINER = Joiner.on(", ").skipNulls();

protected static String authentication = null;
protected static String userName = null;

private static final String SIMPLE_AUTH = "simple";
private static final String OAUTH_AUTH = "oauth";
private static final String KERBEROS_AUTH = "kerberos";

private final String url;
private final boolean ignoreVersions;
private final String outputFormat;
Expand Down Expand Up @@ -99,6 +100,16 @@ public static void setAuthenticationMode(String authentication, String userName)

/** All commands have a handle method to handle and run the required command. */
public abstract void handle();

/**
* verify the arguments. All commands have a verify method to verify the arguments.
*
* @return Returns itself via argument validation, otherwise exits.
*/
public Command validate() {
return this;
}

/**
* Builds a {@link GravitinoClient} instance with the provided server URL and metalake.
*
Expand Down Expand Up @@ -192,4 +203,8 @@ protected <T> void output(T entity) {
throw new IllegalArgumentException("Unsupported output format");
}
}

protected String getMissingEntitiesInfo(String... entities) {
return "Missing required argument(s): " + COMMA_JOINER.join(entities);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,4 +60,10 @@ public void handle() {

System.out.println(property + " property removed.");
}

@Override
public Command validate() {
if (property == null) exitWithError(ErrorMessages.MISSING_PROPERTY);
return this;
}
Abyss-lord marked this conversation as resolved.
Show resolved Hide resolved
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,4 +63,12 @@ public void handle() {

System.out.println(metalake + " property set.");
}

@Override
public Command validate() {
if (property == null && value == null) exitWithError("Missing --property and --value options.");
if (property == null) exitWithError(ErrorMessages.MISSING_PROPERTY);
if (value == null) exitWithError(ErrorMessages.MISSING_VALUE);
return this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ public class TestFulllName {

@BeforeEach
public void setUp() {
Main.useExit = false;
options = new GravitinoOptions().options();
System.setOut(new PrintStream(outContent));
System.setErr(new PrintStream(errContent));
Expand Down Expand Up @@ -82,8 +83,7 @@ public void entityNotFound() throws Exception {
CommandLine commandLine = new DefaultParser().parse(options, args);
FullName fullName = new FullName(commandLine);

String metalakeName = fullName.getMetalakeName();
assertNull(metalakeName);
assertThrows(RuntimeException.class, fullName::getMetalakeName);
}

@Test
Expand Down Expand Up @@ -231,8 +231,7 @@ public void testGetMetalakeWithoutMetalakeOption() throws ParseException {
String[] args = {"table", "list", "-i", "--name", "Hive_catalog.default"};
CommandLine commandLine = new DefaultParser().parse(options, args);
FullName fullName = new FullName(commandLine);
String metalakeName = fullName.getMetalakeName();
assertNull(metalakeName);
assertThrows(RuntimeException.class, fullName::getMetalakeName);
String errOutput = new String(errContent.toByteArray(), StandardCharsets.UTF_8).trim();
assertEquals(errOutput, ErrorMessages.MISSING_METALAKE);
}
Abyss-lord marked this conversation as resolved.
Show resolved Hide resolved
Expand Down
Loading
Loading