Abyss-lord opened a new issue, #5861:
URL: https://github.com/apache/gravitino/issues/5861
### What would you like to be improved?
Current Issues
1. The `executeCommand` method contains a large number of if-else
structures, which leads to poor readability and scalability.
2. The specific entity execution commands (e.g., `handleXXXCommand`) suffer
from the same problem.
3. The parameter validation logic should placed in the `handleXXXCommand`
layer.
### How should we improve?
# Resolve the issue of excessive if-else statements in the executeCommand
method.
Start by refactoring the `handleRoleCommand` method to improve the CLI code.
To resolve the issue of excessive if-else in the `executeCommand` method, refer
to this pull request #5688 . We can create a Map to store the execution method
corresponding to each entity. e.g.
```java
private final Map<String, Consumer<Void>> entityMap = Maps.newHashMap();
private void initializeEntityMap() {
entityMap.put(CommandEntities.COLUMN, v -> handleColumnCommand());
entityMap.put(CommandEntities.TABLE, v -> handleTableCommand());
entityMap.put(CommandEntities.SCHEMA, v -> handleSchemaCommand());
entityMap.put(CommandEntities.CATALOG, v -> handleCatalogCommand());
entityMap.put(CommandEntities.METALAKE, v -> handleMetalakeCommand());
entityMap.put(CommandEntities.TOPIC, v -> handleTopicCommand());
entityMap.put(CommandEntities.FILESET, v -> handleFilesetCommand());
entityMap.put(CommandEntities.USER, v -> handleUserCommand());
entityMap.put(CommandEntities.GROUP, v -> handleGroupCommand());
entityMap.put(CommandEntities.TAG, v -> handleTagCommand());
entityMap.put(CommandEntities.ROLE, v -> handleRoleCommand());
}
```
# Resolve the issue of excessive if-else statements in parameter validation
and specific command execution.
Similarly, construct a Map to store the execution method corresponding to
each Command, for example:
```java
private final Map<String, Consumer<Role>> roleCommandMap = Maps.newHashMap();
private void initializeRoleCommandMap() {
roleCommandMap.put(CommandActions.DETAILS, v ->
handleRoleDetailCommand());
roleCommandMap.put(CommandActions.LIST, v -> handleListRoleCommand());
roleCommandMap.put(CommandActions.CREATE, v ->
handleRoleCreateCommand());
roleCommandMap.put(CommandActions.DELETE, v ->
handleRoleDeleteCommand());
}
```
In the original processing logic, the steps can be simplified as follows:
1. Retrieve the necessary arguments.
2. Determine the method to be executed.
3. Pass the retrieved arguments to the corresponding method.
```java
protected void handleRoleCommand() {
String url = getUrl();
String auth = getAuth();
String userName = line.getOptionValue(GravitinoOptions.LOGIN);
FullName name = new FullName(line);
String metalake = name.getMetalakeName();
String role = line.getOptionValue(GravitinoOptions.ROLE);
Command.setAuthenticationMode(auth, userName);
if (CommandActions.DETAILS.equals(command)) {
newRoleDetails(url, ignore, metalake, role).handle();
} else if (CommandActions.LIST.equals(command)) {
newListRoles(url, ignore, metalake).handle();
} else if (CommandActions.CREATE.equals(command)) {
newCreateRole(url, ignore, metalake, role).handle();
} else if (CommandActions.DELETE.equals(command)) {
boolean force = line.hasOption(GravitinoOptions.FORCE);
newDeleteRole(url, ignore, force, metalake, role).handle();
}
}
```
One entity processing method will only call one command handling method.
Therefore, is it possible to create a data class `Role` with two purposes:
1. Validate whether the parameters are complete based on the command to be
executed.
2. Provide necessary prompt messages to inform the user of any missing
parameters.
and in GravitinoCommandLine, use a variable to store the Role
instanceļ¼current design as follows:
**BaseEntity**
The base class for all entities, designed to store common methods.
**Role**
Used for performing checks related to the Role entity.
**Key attributes**
1. `actionCheckMap`: A dictionary that stores command validation methods.
2. `entityArgMap`: Stores field names and values, for example, `"METALAKE":
metalake value`.
**key methods**
`public boolean checkArguments(String action):` Checks whether the necessary
arguments for the corresponding operation are defined.
Based on the design above, the related operations for the Role entity can be
simplified to the following code:
```java
/**
* Create a role
*/
protected void handleRoleCreateCommand() {
if (!roleDataObject.checkArguments(CommandActions.CREATE)) {
return;
}
newCreateRole(
roleDataObject.getUrl(), ignore, roleDataObject.getMetalake(),
roleDataObject.getRole())
.handle();
}
/**
* Delete a role
*/
protected void handleRoleDeleteCommand() {
if (!roleDataObject.checkArguments(CommandActions.DELETE)) {
return;
}
boolean force = line.hasOption(GravitinoOptions.FORCE);
newDeleteRole(
roleDataObject.getUrl(),
ignore,
force,
roleDataObject.getMetalake(),
roleDataObject.getRole())
.handle();
}
```
If a new command, such as `remove`, is supported for the role in the future,
the expansion steps would be as follows:
1. Override the `checkRemoveArguments` method from `BaseEntity` class to
define the remove check logic
2. Add a `handleRoleRemoveCommand` method in `GravitinoCommandLine`.
3. Add `handleRoleRemoveCommand` to the roleCommandMap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@gravitino.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
