Skip to content

Commit

Permalink
Missing required option doesn't throw
Browse files Browse the repository at this point in the history
This is a proposed fix for: airlift#22

Tests still need to be fixed.
  • Loading branch information
jontodd committed Dec 30, 2013
1 parent cd9c8d8 commit 3d1f0c4
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 9 deletions.
22 changes: 22 additions & 0 deletions src/main/java/io/airlift/airline/ParseResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package io.airlift.airline;

import com.google.common.collect.ImmutableList;

import java.util.Collections;
import java.util.List;

public class ParseResult {
private List<ParseException> errors = Collections.emptyList();

public void addError(ParseException error) {
errors.add(error);
}

public boolean hasErrors() {
return errors.size() != 0;
}

public List<ParseException> getErrors() {
return ImmutableList.copyOf(errors);
}
}
37 changes: 28 additions & 9 deletions src/main/java/io/airlift/airline/SingleCommand.java
Original file line number Diff line number Diff line change
Expand Up @@ -57,12 +57,31 @@ public C parse(String... args)
}

public C parse(Iterable<String> args)
{
ParseResult parseResult = new ParseResult();
C command = parse(parseResult, args);

// For backward compatibility we fail fast here. Given that exceptions aren't a great way to handle user errors
// since there can be multiple we may consider deprecating this approach.
if (parseResult.hasErrors()) {
throw parseResult.getErrors().get(0);
}

return command;
}

public C parse(ParseResult parseResult, String... args)
{
return parse(parseResult, ImmutableList.copyOf(args));
}

public C parse(ParseResult parseResult, Iterable<String> args)
{
checkNotNull(args, "args is null");

Parser parser = new Parser();
ParseState state = parser.parseCommand(commandMetadata, args);
validate(state);
validate(state, parseResult);

CommandMetadata command = state.getCommand();

Expand All @@ -75,35 +94,35 @@ public C parse(Iterable<String> args)
ImmutableMap.<Class<?>, Object>of(CommandMetadata.class, commandMetadata));
}

private void validate(ParseState state)
private void validate(ParseState state, ParseResult parseResult)
{
CommandMetadata command = state.getCommand();
if (command == null) {
List<String> unparsedInput = state.getUnparsedInput();
if (unparsedInput.isEmpty()) {
throw new ParseCommandMissingException();
parseResult.addError(new ParseCommandMissingException());
}
else {
throw new ParseCommandUnrecognizedException(unparsedInput);
parseResult.addError(new ParseCommandUnrecognizedException(unparsedInput));
}
}

ArgumentsMetadata arguments = command.getArguments();
if (state.getParsedArguments().isEmpty() && arguments != null && arguments.isRequired()) {
throw new ParseArgumentsMissingException(arguments.getTitle());
parseResult.addError(new ParseArgumentsMissingException(arguments.getTitle()));
}

if (!state.getUnparsedInput().isEmpty()) {
throw new ParseArgumentsUnexpectedException(state.getUnparsedInput());
parseResult.addError(new ParseArgumentsUnexpectedException(state.getUnparsedInput()));
}

if (state.getLocation() == Context.OPTION) {
throw new ParseOptionMissingValueException(state.getCurrentOption().getTitle());
parseResult.addError(new ParseOptionMissingValueException(state.getCurrentOption().getTitle()));
}

for (OptionMetadata option : command.getAllOptions()) {
if (option.isRequired() && !state.getParsedOptions().containsKey(option)) {
throw new ParseOptionMissingException(option.getOptions().iterator().next());
parseResult.addError(new ParseOptionMissingException(option.getOptions().iterator().next()));
}
}
}
Expand Down
30 changes: 30 additions & 0 deletions src/test/java/io/airlift/airline/SingleCommandTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@

import static com.google.common.base.Predicates.compose;
import static com.google.common.base.Predicates.equalTo;
import static com.google.common.base.Predicates.instanceOf;
import static com.google.common.collect.Iterables.find;
import static io.airlift.airline.SingleCommand.singleCommand;
import static org.testng.Assert.assertTrue;
Expand Down Expand Up @@ -171,6 +172,14 @@ public void multipleUnparsedFail()
singleCommand(ArgsMultipleUnparsed.class).parse();
}

@Test
public void multipleUnparsedFailWithResult()
{
ParseResult result = new ParseResult();
singleCommand(ArgsMultipleUnparsed.class).parse(result);
assertParseResultHasErrorOfType(result, IllegalArgumentException.class);
}

public void privateArgs()
{
ArgsPrivate args = singleCommand(ArgsPrivate.class).parse("-verbose", "3");
Expand Down Expand Up @@ -244,12 +253,27 @@ public void requiredMainParameters()
singleCommand(ArgsRequired.class).parse();
}

@Test
public void requiredMainParamertersWithParseResult() {
ParseResult result = new ParseResult();
singleCommand(ArgsRequired.class).parse(result);
assertParseResultHasErrorOfType(result, ParseException.class);
}

@Test(expectedExceptions = ParseException.class, expectedExceptionsMessageRegExp = ".*option.*missing.*")
public void requiredOptions()
{
singleCommand(OptionsRequired.class).parse();
}

@Test
public void requiredOptionsWithParseResult()
{
ParseResult result = new ParseResult();
singleCommand(OptionsRequired.class).parse();
assertParseResultHasErrorOfType(result, ParseException.class);
}

@Test
public void ignoresOptionalOptions()
{
Expand Down Expand Up @@ -388,4 +412,10 @@ public static class CommandTest
public Boolean interactive = false;

}

private void assertParseResultHasErrorOfType(ParseResult result, Class clazz) {
Assert.assertEquals(result.hasErrors(), true);
String errorMessage = String.format("Expected ParseResult to contain at least one instance of: '%s'", clazz);
Assert.assertNotNull(find(result.getErrors(), instanceOf(IllegalArgumentException.class)), errorMessage);
}
}

0 comments on commit 3d1f0c4

Please sign in to comment.