Skip to content

Commit

Permalink
Start of key value implementation
Browse files Browse the repository at this point in the history 
- added method to separate key=value pairs and adding them to the argument list.
- added some initial tests
- added configuration option to enable or disable the key value separation
  • Loading branch information
@emileplas
emileplas committed Oct 5, 2024
1 parent 094ffeb commit 3ae76fe
Show file tree
Hide file tree
Showing 3 changed files with 238 additions and 5 deletions.
59 changes: 56 additions & 3 deletions spring-shell-core/src/main/java/org/springframework/shell/command/parser/Lexer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,58 @@ public DefaultLexer(CommandModel commandModel, ParserConfig config) {
private record ArgumentsSplit(List<String> before, List<String> after) {
}


/**
* Loops over a list of arguments and if any of the arguments would follow the pattern ".*=.*"
* it splits those arguments.
* @param arguments the original arguments list
* @return List<String> containing the arguments without key-value separator
*/
private static List<String> splitKeyAndValueBySeparator(List<String> arguments) {
if(arguments != null){
if(arguments.isEmpty()){
return arguments;
}else{
List<String> result = new ArrayList<>();

for (String argument : arguments) {
if (argument == null) {
result.add(null);
} else if (argument.isEmpty()) {
result.add("");
} else if (argument.matches(".*=.*")) {
String[] parts = argument.split("=", 2);
result.add(parts[0].trim());
result.add(parts[1].trim());
} else {
result.add(argument); // Add non-key-value arguments as-is
}
}
return result;
}
}else{
return null;
}

}

/**
* Returns the argument split with the before and after list. In case isKeyValueSeparatorEnabled is true, it will however first separate the keys from values, adding
* each as an argument to the list so the tokenize method can work with this them as previously implemented.
* @param before before list
* @param after after list
* @param isKeyValueSeparatorEnabled true if key=value is allowed, otherwise not.
* @return ArgumentsSplit
*/
private ArgumentsSplit prepareArgumentSplitWithoutKeyValueSeparator(List<String> before, List<String> after, boolean isKeyValueSeparatorEnabled){
if(isKeyValueSeparatorEnabled){
return new ArgumentsSplit(splitKeyAndValueBySeparator(before), splitKeyAndValueBySeparator(after));
}else{
return new ArgumentsSplit(before, after);
}
}


/**
* Splits arguments from a point first valid command is found, where
* {@code before} is everything before commands and {@code after} what's
Expand All @@ -100,11 +152,12 @@ private ArgumentsSplit splitArguments(List<String> arguments, Map<String, Token>
}
else if (i == 0) {
if (foundSplit) {
return new ArgumentsSplit(Collections.emptyList(), arguments);
return prepareArgumentSplitWithoutKeyValueSeparator(Collections.emptyList(), arguments, config.isEnabled(Feature.ALLOW_KEY_VALUE_SEPARATOR));
}
return new ArgumentsSplit(arguments, Collections.emptyList());
return prepareArgumentSplitWithoutKeyValueSeparator(arguments, Collections.emptyList(), config.isEnabled(Feature.ALLOW_KEY_VALUE_SEPARATOR));
}
return new ArgumentsSplit(arguments.subList(0, i), arguments.subList(i, arguments.size()));

return prepareArgumentSplitWithoutKeyValueSeparator(arguments.subList(0, i), arguments.subList(i, arguments.size()), config.isEnabled(Feature.ALLOW_KEY_VALUE_SEPARATOR));
}

private List<String> extractDirectives(List<String> arguments) {
Expand Down
8 changes: 6 additions & 2 deletions spring-shell-core/src/main/java/org/springframework/shell/command/parser/ParserConfig.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,12 @@ public static enum Feature {
/**
* Defines if options are parsed using case-sensitivity, enabled on default.
*/
CASE_SENSITIVE_OPTIONS(true)
;
CASE_SENSITIVE_OPTIONS(true),


ALLOW_KEY_VALUE_SEPARATOR(true);



private final boolean defaultState;
private final long mask;
Expand Down
176 changes: 176 additions & 0 deletions spring-shell-core/src/test/java/org/springframework/shell/command/parser/ParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,23 @@ void commandArgumentsGetsAddedAfterDoubleDash() {
}
);
}

@Test
void commandArgumentsGetsAddedAfterDoubleDashOptionValueSeparatedByEqualSign() {
register(ROOT3);
ParseResult result = parse("root3", "--arg1=value1", "--", "arg1", "arg2");

assertThat(result.argumentResults()).satisfiesExactly(
ar -> {
assertThat(ar.value()).isEqualTo("arg1");
assertThat(ar.position()).isEqualTo(0);
},
ar -> {
assertThat(ar.value()).isEqualTo("arg2");
assertThat(ar.position()).isEqualTo(1);
}
);
}
}

@Nested
Expand All @@ -95,6 +112,17 @@ void optionValueShouldBeInteger() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeIntegerOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INT);
ParseResult result = parse("root6", "--arg1=1");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo(1);
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeIntegerArray() {
register(ROOT6_OPTION_INTARRAY);
Expand All @@ -106,6 +134,18 @@ void optionValueShouldBeIntegerArray() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeIntegerArrayOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INTARRAY);
//TODO: is this how we want to indicate arrays after the equal sign
ParseResult result = parse("root6", "--arg1=1", "2");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo(new int[] { 1, 2 });
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueFailsFromStringToInteger() {
register(ROOT6_OPTION_INT);
Expand All @@ -121,6 +161,21 @@ void optionValueFailsFromStringToInteger() {
});
}

@Test
void optionValueFailsFromStringToIntegerOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INT);
ParseResult result = parse("root6", "--arg1=x");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo("x");
assertThat(result.messageResults()).isNotEmpty();
assertThat(result.messageResults()).satisfiesExactly(ms -> {
// "2002E:(pos 0): Illegal option value 'x', reason 'Failed to convert from type [java.lang.String] to type [int] for value 'x''"
assertThat(ms.getMessage()).contains("Failed to convert");
});
}

@Test
void optionValueShouldBeNegativeInteger() {
register(ROOT6_OPTION_INT);
Expand All @@ -132,6 +187,17 @@ void optionValueShouldBeNegativeInteger() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void optionValueShouldBeNegativeIntegerOptionValueSeparatedByEqualSign() {
register(ROOT6_OPTION_INT);
ParseResult result = parse("root6", "--arg1=-1");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults().get(0).value()).isEqualTo(-1);
assertThat(result.messageResults()).isEmpty();
}

}

@Nested
Expand Down Expand Up @@ -312,6 +378,26 @@ void shouldFindTwoLongOptionArgument() {
);
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindTwoLongOptionArgumentOptionValueSeparatedByEqualSign() {
register(ROOT3_OPTION_ARG1_ARG2);
ParseResult result = parse("root3", "--arg1=value1", "--arg2=value2");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg1" });
assertThat(r.value()).isEqualTo("value1");
},
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg2" });
assertThat(r.value()).isEqualTo("value2");
}
);
assertThat(result.messageResults()).isEmpty();
}
}

@Nested
Expand Down Expand Up @@ -349,6 +435,22 @@ void shouldFindShortOptionWithArg() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionWithOptionValueSeparatedByEqualSign() {
register(ROOT3_SHORT_OPTION_A);
ParseResult result = parse("root3", "-a=aaa");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
assertThat(r.value()).isEqualTo("aaa");
}
);
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptions() {
register(ROOT3_SHORT_OPTION_A_B);
Expand All @@ -369,6 +471,26 @@ void shouldFindShortOptions() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionsWithOptionValueSeparatedByEqualSign() {
register(ROOT3_SHORT_OPTION_A_B);
ParseResult result = parse("root3", "-a=aaa", "-b=bbb");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'a' });
assertThat(r.value()).isEqualTo("aaa");
},
r -> {
assertThat(r.option().getShortNames()).isEqualTo(new Character[] { 'b' });
assertThat(r.value()).isEqualTo("bbb");
}
);
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionsRequired() {
register(ROOT3_SHORT_OPTION_A_B_REQUIRED);
Expand All @@ -379,6 +501,16 @@ void shouldFindShortOptionsRequired() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindShortOptionsRequiredWithOptionValueSeparatedByEqualSign() {
register(ROOT3_SHORT_OPTION_A_B_REQUIRED);
ParseResult result = parse("root3", "-a=aaa", "-b=bbb");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.messageResults()).isEmpty();
}

}

@Nested
Expand Down Expand Up @@ -439,6 +571,25 @@ void shouldFindLongOptionRegLowerCommandUpper() {
assertThat(result.messageResults()).isEmpty();
}

@Test
void shouldFindLongOptionRegLowerCommandUpperWithOptionValueSeparatedByEqualSign() {
register(ROOT3);
ParserConfig config = new ParserConfig()
.disable(Feature.CASE_SENSITIVE_COMMANDS)
.disable(Feature.CASE_SENSITIVE_OPTIONS)
;
ParseResult result = parse(config, "root3", "--Arg1=value1");
assertThat(result).isNotNull();
assertThat(result.commandRegistration()).isNotNull();
assertThat(result.optionResults()).isNotEmpty();
assertThat(result.optionResults()).satisfiesExactly(
r -> {
assertThat(r.value()).isEqualTo("value1");
}
);
assertThat(result.messageResults()).isEmpty();
}

}

@Nested
Expand Down Expand Up @@ -611,6 +762,31 @@ void positionOverridesDefaultsKeepsDefaultWhenOption() {
);
}

@Test
void positionOverridesDefaultsKeepsDefaultWhenOptionWithOptionValueSeparatedByEqualSign() {
register(ROOT7_POSITIONAL_TWO_ARG_STRING_DEFAULT);
ParseResult result = parse("root7", "--arg1=a", "b");
assertThat(result.messageResults()).isEmpty();
assertThat(result.argumentResults()).satisfiesExactly(
r -> {
assertThat(r.value()).isEqualTo("b");
assertThat(r.position()).isEqualTo(0);
}
);
assertThat(result.optionResults()).isNotNull().satisfiesExactly(
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg1" });
assertThat(r.value()).isEqualTo("a");
},
r -> {
assertThat(r.option().getLongNames()).isEqualTo(new String[] { "arg2" });
assertThat(r.value()).isEqualTo("b");
}
);
}



@Test
void positionWithLastHavingNoDefault() {
register(ROOT7_POSITIONAL_TWO_ARG_STRING_DEFAULT_ONE_NODEFAULT);
Expand Down

0 comments on commit 3ae76fe

Please sign in to comment.