Skip to content

Commit 14e1583

Browse files
ddsharpeddsharpe
authored
Fix usage for invalid first argument (#312)
* rename InvalidCredential to InvalidCredentialException * add additional unit tests * removed unused OPatch version discovery method * fixed constants names in new unit test * Fix usage statement when first subcommand is unrecognized * replace ImageTool instance with class for initiation of the CLI (static code review) * fix typos
1 parent f1944b5 commit 14e1583

File tree

20 files changed

+300
-120
lines changed

20 files changed

+300
-120
lines changed

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/api/model/CommandResponse.java‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33

44
package com.oracle.weblogic.imagetool.api.model;
55

6+
import com.oracle.weblogic.imagetool.logging.LoggingFacade;
67
import com.oracle.weblogic.imagetool.util.Utils;
78

9+
import static picocli.CommandLine.ExitCode;
10+
811
public class CommandResponse {
912

1013
private final int status;
@@ -13,7 +16,8 @@ public class CommandResponse {
1316

1417
/**
1518
* For use with PicoCLI to return the response to the command line.
16-
* @param status CLI status, 0 if normal.
19+
*
20+
* @param status CLI status, 0 if normal.
1721
* @param message message to the user.
1822
*/
1923
public CommandResponse(int status, String message, Object... messageParams) {
@@ -22,8 +26,31 @@ public CommandResponse(int status, String message, Object... messageParams) {
2226
this.messageParams = messageParams;
2327
}
2428

29+
/**
30+
* Create a new error response (status 1).
31+
*
32+
* @param message error message
33+
* @param messageParams parameters for the error message
34+
* @return a new CommandResponse with status 1
35+
*/
36+
public static CommandResponse error(String message, Object... messageParams) {
37+
return new CommandResponse(ExitCode.SOFTWARE, message, messageParams);
38+
}
39+
40+
/**
41+
* Create a new success response (status 0).
42+
*
43+
* @param message optional message
44+
* @param messageParams parameters for the message
45+
* @return a new CommandResponse with status 1
46+
*/
47+
public static CommandResponse success(String message, Object... messageParams) {
48+
return new CommandResponse(ExitCode.OK, message, messageParams);
49+
}
50+
2551
/**
2652
* Get the status code in this response.
53+
*
2754
* @return the status
2855
*/
2956
public int getStatus() {
@@ -32,10 +59,27 @@ public int getStatus() {
3259

3360
/**
3461
* Get the message in this response.
62+
*
3563
* @return message to the user
3664
*/
3765
public String getMessage() {
3866
return Utils.getMessage(message, messageParams);
3967
}
4068

69+
/**
70+
* Log the response message with the provided logger.
71+
*
72+
* @param logger logger to use.
73+
*/
74+
public void logResponse(LoggingFacade logger) {
75+
if (Utils.isEmptyString(message)) {
76+
return;
77+
}
78+
79+
if (status == ExitCode.OK) {
80+
logger.info(getMessage());
81+
} else if (status == ExitCode.SOFTWARE) {
82+
logger.severe(getMessage());
83+
}
84+
}
4185
}

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/aru/InvalidCredential.java‎ renamed to ‎imagetool/src/main/java/com/oracle/weblogic/imagetool/aru/InvalidCredentialException.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55

66
import com.oracle.weblogic.imagetool.util.Utils;
77

8-
public class InvalidCredential extends AruException {
9-
public InvalidCredential() {
8+
public class InvalidCredentialException extends AruException {
9+
public InvalidCredentialException() {
1010
super(Utils.getMessage("IMG-0022"));
1111
}
1212
}

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/ImageTool.java‎

Lines changed: 45 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
package com.oracle.weblogic.imagetool.cli;
55

66
import java.io.PrintWriter;
7-
import java.util.concurrent.Callable;
87

98
import com.oracle.weblogic.imagetool.api.model.CommandResponse;
109
import com.oracle.weblogic.imagetool.cli.cache.CacheCLI;
@@ -19,6 +18,8 @@
1918
import picocli.CommandLine.HelpCommand;
2019
import picocli.CommandLine.ParseResult;
2120

21+
import static picocli.CommandLine.ExitCode;
22+
2223
@Command(
2324
name = "imagetool",
2425
mixinStandardHelpOptions = true,
@@ -38,57 +39,56 @@
3839
usageHelpWidth = 120,
3940
commandListHeading = "%nCommands:%n%nChoose from:%n"
4041
)
41-
public class ImageTool implements Callable<CommandResponse> {
42+
public class ImageTool {
4243

4344
private static final LoggingFacade logger = LoggingFactory.getLogger(ImageTool.class);
4445

45-
@Override
46-
public CommandResponse call() {
47-
CommandLine.usage(ImageTool.class, System.out);
48-
return new CommandResponse(0, "");
49-
}
50-
5146
/**
52-
* Entry point for Image Tool.
47+
* Entry point for Image Tool command line.
5348
* @param args command line arguments.
5449
*/
5550
public static void main(String[] args) {
56-
CommandResponse response = run(new ImageTool(),
51+
CommandResponse response = run(ImageTool.class,
5752
new PrintWriter(System.out, true),
5853
new PrintWriter(System.err, true),
5954
args);
6055

61-
if (response != null) {
62-
if (response.getStatus() != 0) {
63-
logger.severe(response.getMessage());
64-
} else if (!response.getMessage().isEmpty()) {
65-
logger.info(response.getMessage());
66-
}
67-
System.exit(response.getStatus());
68-
}
69-
System.exit(1);
56+
response.logResponse(logger);
57+
System.exit(response.getStatus());
7058
}
7159

7260
/**
73-
* Used from main entry point, and also entry point for unit tests.
61+
* Executes the provided entryPoint .
62+
*
63+
* @param entryPoint must be an instance or class annotated with picocli.CommandLine.Command
7464
* @param out where to send stdout
7565
* @param err where to send stderr
7666
* @param args the command line arguments (minus the sub commands themselves)
7767
*/
78-
public static CommandResponse run(Callable<CommandResponse> callable, PrintWriter out, PrintWriter err,
79-
String... args) {
80-
81-
CommandLine cmd = new CommandLine(callable)
68+
public static CommandResponse run(Object entryPoint, PrintWriter out, PrintWriter err, String... args) {
69+
CommandLine cmd = new CommandLine(entryPoint)
8270
.setCaseInsensitiveEnumValuesAllowed(true)
8371
.setToggleBooleanFlags(false)
8472
.setUnmatchedArgumentsAllowed(false)
8573
.setColorScheme(CommandLine.Help.defaultColorScheme(CommandLine.Help.Ansi.AUTO))
74+
.setParameterExceptionHandler(new ExceptionHandler())
8675
.setOut(out)
8776
.setErr(err);
8877

89-
cmd.execute(args);
78+
int exit = cmd.execute(args);
79+
CommandResponse response;
80+
if (exit == ExitCode.USAGE) {
81+
response = new CommandResponse(ExitCode.USAGE, null);
82+
} else {
83+
CommandLine commandLine = getSubcommand(cmd);
84+
response = commandLine.getExecutionResult();
85+
if (response == null) {
86+
logger.fine("User requested usage by using help command");
87+
response = CommandResponse.success(null);
88+
}
89+
}
9090
//Get the command line for the sub-command (if exists), and ignore the parents, like "imagetool cache listItems"
91-
return getSubcommand(cmd).getExecutionResult();
91+
return response;
9292
}
9393

9494
/**
@@ -105,4 +105,23 @@ private static CommandLine getSubcommand(CommandLine commandLine) {
105105

106106
return commandLine;
107107
}
108+
109+
static class ExceptionHandler implements CommandLine.IParameterExceptionHandler {
110+
111+
@Override
112+
public int handleParseException(CommandLine.ParameterException ex, String[] args) {
113+
CommandLine cmd = ex.getCommandLine();
114+
PrintWriter writer = cmd.getErr();
115+
116+
writer.println(ex.getMessage());
117+
CommandLine.UnmatchedArgumentException.printSuggestions(ex, writer);
118+
119+
CommandLine.Model.CommandSpec spec = cmd.getCommandSpec();
120+
ex.getCommandLine().usage(writer, cmd.getColorScheme());
121+
122+
return cmd.getExitCodeExceptionMapper() != null
123+
? cmd.getExitCodeExceptionMapper().getExitCode(ex)
124+
: spec.exitCodeOnInvalidInput();
125+
}
126+
}
108127
}

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/AddEntry.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -28,9 +28,9 @@ public CommandResponse call() throws CacheStoreException {
2828
msg = String.format("Added entry %s=%s", key, location);
2929
}
3030
cache().addToCache(key, location);
31-
return new CommandResponse(0, msg);
31+
return CommandResponse.success(msg);
3232
}
33-
return new CommandResponse(1, "IMG-0044");
33+
return CommandResponse.error("IMG-0044");
3434
}
3535

3636
@Option(

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/AddPatchEntry.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,11 @@ public CommandResponse call() throws Exception {
2525
List<String> patches = new ArrayList<>();
2626
patches.add(patchId);
2727
if (!Utils.validatePatchIds(patches, true)) {
28-
return new CommandResponse(1, "Patch ID validation failed");
28+
return CommandResponse.error("Patch ID validation failed");
2929
}
3030
return addToCache(patchId);
3131
} else {
32-
return new CommandResponse(1, "IMG-0076", "--patchId");
32+
return CommandResponse.error("IMG-0076", "--patchId");
3333
}
3434
}
3535

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/CacheAddOperation.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,23 +17,23 @@ public abstract class CacheAddOperation extends CacheOperation {
1717
CommandResponse addToCache(String key) throws CacheStoreException {
1818
// if file is invalid or does not exist, return an error
1919
if (filePath == null || !Files.isRegularFile(filePath)) {
20-
return new CommandResponse(1, "IMG-0049", filePath);
20+
return CommandResponse.error("IMG-0049", filePath);
2121
}
2222

2323
// if the new value is the same as the existing cache value, do nothing
2424
String existingValue = cache().getValueFromCache(key);
2525
if (absolutePath().toString().equals(existingValue)) {
26-
return new CommandResponse(0, "IMG-0075");
26+
return CommandResponse.success("IMG-0075");
2727
}
2828

2929
// if there is already a cache entry and the user did not ask to force it, return an error
3030
if (!force && existingValue != null) {
31-
return new CommandResponse(1, "IMG-0048", key, existingValue);
31+
return CommandResponse.error("IMG-0048", key, existingValue);
3232
}
3333

3434
// input appears valid, add the entry to the cache and exit
3535
cache().addToCache(key, absolutePath().toString());
36-
return new CommandResponse(0, "IMG-0050", key, cache().getValueFromCache(key));
36+
return CommandResponse.success("IMG-0050", key, cache().getValueFromCache(key));
3737
}
3838

3939
Path absolutePath() {

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/CacheCLI.java‎

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -4,11 +4,8 @@
44
package com.oracle.weblogic.imagetool.cli.cache;
55

66
import java.util.List;
7-
import java.util.concurrent.Callable;
87

9-
import com.oracle.weblogic.imagetool.api.model.CommandResponse;
108
import com.oracle.weblogic.imagetool.cli.HelpVersionProvider;
11-
import picocli.CommandLine;
129
import picocli.CommandLine.Command;
1310
import picocli.CommandLine.HelpCommand;
1411
import picocli.CommandLine.Unmatched;
@@ -28,13 +25,7 @@
2825
},
2926
sortOptions = false
3027
)
31-
public class CacheCLI implements Callable<CommandResponse> {
32-
33-
@Override
34-
public CommandResponse call() {
35-
CommandLine.usage(this, System.out);
36-
return new CommandResponse(0, "");
37-
}
28+
public class CacheCLI {
3829

3930
@Unmatched
4031
List<String> unmatchedOptions;

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/DeleteEntry.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,20 @@ public class DeleteEntry extends CacheOperation {
2121
public CommandResponse call() throws Exception {
2222
if (!Utils.isEmptyString(key)) {
2323
if (Constants.CACHE_DIR_KEY.equalsIgnoreCase(key)) {
24-
return new CommandResponse(0, "Cannot delete key: " + key);
24+
return CommandResponse.success("Cannot delete key: " + key);
2525
} else if (Constants.DELETE_ALL_FOR_SURE.equalsIgnoreCase(key)) {
2626
cache().clearCache();
27-
return new CommandResponse(0, "IMG-0046");
27+
return CommandResponse.success("IMG-0046");
2828
} else {
2929
String oldValue = cache().deleteFromCache(key);
3030
if (oldValue != null) {
31-
return new CommandResponse(0, "IMG-0051", key, oldValue);
31+
return CommandResponse.success("IMG-0051", key, oldValue);
3232
} else {
33-
return new CommandResponse(0, "IMG-0052", key);
33+
return CommandResponse.success("IMG-0052", key);
3434
}
3535
}
3636
}
37-
return new CommandResponse(1, "IMG-0045");
37+
return CommandResponse.error("IMG-0045");
3838
}
3939

4040
@Option(

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/cache/ListCacheItems.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ public class ListCacheItems extends CacheOperation {
2525
public CommandResponse call() throws CacheStoreException {
2626
Map<String, String> resultMap = cache().getCacheItems();
2727
if (resultMap == null || resultMap.isEmpty()) {
28-
return new CommandResponse(0, "IMG-0047");
28+
return CommandResponse.success("IMG-0047");
2929
} else {
3030
System.out.println("Cache contents");
3131
String cacheDir = resultMap.getOrDefault(Constants.CACHE_DIR_KEY, null);
@@ -39,7 +39,7 @@ public CommandResponse call() throws CacheStoreException {
3939
.filter(entry -> pattern.matcher(entry.getKey()).matches())
4040
.forEach(System.out::println);
4141

42-
return new CommandResponse(0, "");
42+
return CommandResponse.success(null);
4343
}
4444
}
4545

‎imagetool/src/main/java/com/oracle/weblogic/imagetool/cli/menu/CommonOptions.java‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424
import com.oracle.weblogic.imagetool.aru.AruProduct;
2525
import com.oracle.weblogic.imagetool.aru.AruUtil;
2626
import com.oracle.weblogic.imagetool.aru.InstalledPatch;
27-
import com.oracle.weblogic.imagetool.aru.InvalidCredential;
27+
import com.oracle.weblogic.imagetool.aru.InvalidCredentialException;
2828
import com.oracle.weblogic.imagetool.aru.InvalidPatchNumberException;
2929
import com.oracle.weblogic.imagetool.aru.MultiplePatchVersionsException;
3030
import com.oracle.weblogic.imagetool.builder.BuildCommand;
@@ -163,7 +163,7 @@ void setTempDirectory(String value) {
163163
tempDirectory = value;
164164
}
165165

166-
void init(String buildId) throws InvalidCredential, IOException {
166+
void init(String buildId) throws InvalidCredentialException, IOException {
167167
logger.entering(buildId);
168168
logger.info(HelpVersionProvider.versionString());
169169
dockerfileOptions = new DockerfileOptions(buildId);
@@ -174,7 +174,7 @@ void init(String buildId) throws InvalidCredential, IOException {
174174

175175
// if userid or password is provided, validate the pair of provided values
176176
if ((userId != null || password != null) && !AruUtil.checkCredentials(userId, password)) {
177-
throw new InvalidCredential();
177+
throw new InvalidCredentialException();
178178
}
179179

180180
handleChown();

0 commit comments

Comments
 (0)