diff --git a/spring-shell2-core/src/main/java/org/springframework/shell2/commands/Help.java b/spring-shell2-core/src/main/java/org/springframework/shell2/commands/Help.java index d7a92f0a..341d9275 100644 --- a/spring-shell2-core/src/main/java/org/springframework/shell2/commands/Help.java +++ b/spring-shell2-core/src/main/java/org/springframework/shell2/commands/Help.java @@ -107,12 +107,14 @@ public class Help { if (description.defaultValue().isPresent()) { result.append("["); } - if (!description.mandatoryKey()) { - result.append("["); - } - result.append(description.keys().iterator().next(), AttributedStyle.BOLD); - if (!description.mandatoryKey()) { - result.append("]"); + if(!description.keys().isEmpty()) { + if (!description.mandatoryKey()) { + result.append("["); + } + result.append(description.keys().iterator().next(), AttributedStyle.BOLD); + if (!description.mandatoryKey()) { + result.append("]"); + } if (!description.formal().isEmpty()) { result.append(" "); } @@ -132,7 +134,9 @@ public class Help { for (ParameterDescription description : parameterDescriptions) { result.append("\t").append(description.keys().stream().collect(Collectors.joining(" or ")), AttributedStyle.BOLD); if (description.formal().length() > 0) { - result.append(" "); + if (!description.keys().isEmpty()) { + result.append(" "); + } appendUnderlinedFormal(result, description); result.append("\n\t"); } diff --git a/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java b/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java index a9aea569..61b98291 100644 --- a/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java +++ b/spring-shell2-core/src/main/java/org/springframework/shell2/standard/StandardParameterResolver.java @@ -100,7 +100,7 @@ public class StandardParameterResolver implements ParameterResolver { @Override public boolean supports(MethodParameter parameter) { - return true; + return parameter.getMethodAnnotation(ShellMethod.class) != null; } @Override diff --git a/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/legacy/LegacyCommands.java b/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/legacy/LegacyCommands.java index 1713af0e..32e721cc 100644 --- a/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/legacy/LegacyCommands.java +++ b/spring-shell2-samples/src/main/java/org/springframework/shell2/samples/legacy/LegacyCommands.java @@ -43,7 +43,7 @@ public class LegacyCommands implements CommandMarker { key = {"type"}, help = "the type for the registered module") ArtifactType type, - @CliOption(mandatory = true, + @CliOption(mandatory = false, key = {"coordinates", "coords"}, help = "coordinates to the module archive") String coordinates, @@ -56,8 +56,28 @@ public class LegacyCommands implements CommandMarker { } @CliCommand(value = "sum", help = "adds two numbers") - public int sum(@CliOption(key = "v1", unspecifiedDefaultValue = "38") int a, @CliOption(key = "v2", specifiedDefaultValue = "42") int b) { + public int sum( + @CliOption(key = "v1", unspecifiedDefaultValue = "38") int a, + @CliOption(key = "v2", specifiedDefaultValue = "42") int b) { return a + b; } + @CliCommand(value = "sum2", help = "adds two numbers") + public int sum2( + @CliOption(key = "v1", unspecifiedDefaultValue = "38") int a, + @CliOption(key = "v2", specifiedDefaultValue = "42", mandatory = true) int b, + @CliOption(key = "v3", mandatory = false) int c) { + return a + b + c; + } + + @CliCommand(value = "legacy-echo", help = "Echoes a message") + public String legacyEcho(@CliOption(key = "", mandatory = true) String message) { + return message; + } + + @CliCommand(value = "optional-echo", help = "Echoes an optional message") + public String optionalEcho(@CliOption(key = "", mandatory = false) String message) { + return message; + } + } diff --git a/spring-shell2-shell1-adapter/src/main/java/org/springframework/shell2/legacy/LegacyParameterResolver.java b/spring-shell2-shell1-adapter/src/main/java/org/springframework/shell2/legacy/LegacyParameterResolver.java index 34bc960b..dde1977d 100644 --- a/spring-shell2-shell1-adapter/src/main/java/org/springframework/shell2/legacy/LegacyParameterResolver.java +++ b/spring-shell2-shell1-adapter/src/main/java/org/springframework/shell2/legacy/LegacyParameterResolver.java @@ -1,5 +1,5 @@ /* - * Copyright 2015 the original author or authors. + * Copyright 2015-2017 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.shell2.legacy; +import java.lang.reflect.Parameter; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -41,10 +42,18 @@ import org.springframework.util.Assert; * Resolves parameters by looking at the {@link CliOption} annotation and acting accordingly. * * @author Eric Bottard + * @author Camilo Gonzalez */ @Component public class LegacyParameterResolver implements ParameterResolver { + private static final String CLI_OPTION_NULL = "__NULL__"; + + /** + * Prefix used by Spring Shell 1 for the argument keys (e.g. command --key value). + */ + private static final String CLI_PREFIX = "--"; + @Autowired(required = false) private Collection> converters = new ArrayList<>(); @@ -82,7 +91,58 @@ public class LegacyParameterResolver implements ParameterResolver { @Override public ParameterDescription describe(MethodParameter parameter) { - throw new UnsupportedOperationException(); + Parameter jlrParameter = parameter.getMethod().getParameters()[parameter.getParameterIndex()]; + CliOption option = jlrParameter.getAnnotation(CliOption.class); + ParameterDescription result = ParameterDescription.outOf(parameter); + result.help(option.help()); + List keys = Arrays.asList(option.key()); + result.keys(keys.stream() + .filter(key -> !key.isEmpty()) + .map(key -> CLI_PREFIX + key) + .collect(Collectors.toList())); + Optional defaultValue = defaultValueFor(option, result.keys()); + if (defaultValue.isPresent()) { + result.defaultValue(defaultValue.get()); + } + boolean containsEmptyKey = keys.contains(""); + result.mandatoryKey(!containsEmptyKey); + return result; + } + + private Optional defaultValueFor(CliOption option, List keys) { + // CliOption annotations have two default values, one for when the key is specified without a value, + // and one when the key isn't specified (e.g. "command --key" vs "command") + + final boolean unspecifiedDefaultDeclared = !CLI_OPTION_NULL.equals(option.unspecifiedDefaultValue()); + final boolean specifiedDefaultDeclared = !CLI_OPTION_NULL.equals(option.specifiedDefaultValue()); + + if (!unspecifiedDefaultDeclared && !specifiedDefaultDeclared) { + if (option.mandatory()) { + return Optional.empty(); + } else { + // according to CliOption, is no default is declared, then null will be presented to non-primitive + // arguments + return Optional.of("null"); + } + } + + final StringBuilder defaultValue = new StringBuilder(); + + if (unspecifiedDefaultDeclared) { + defaultValue.append(option.unspecifiedDefaultValue()); + } + + if (specifiedDefaultDeclared) { + if (unspecifiedDefaultDeclared) { + defaultValue.append(", or "); + } + + defaultValue.append(option.specifiedDefaultValue()); + defaultValue.append(" if used as "); + defaultValue.append(keys.stream().collect(Collectors.joining(" or "))); + } + + return Optional.of(defaultValue.toString()); } @Override @@ -137,5 +197,5 @@ public class LegacyParameterResolver implements ParameterResolver { private Supplier noConverterFound(String key, String value, Class parameterType) { return () -> new IllegalStateException("No converter found for --" + key + " from '" + value + "' to type " + parameterType); } - + } diff --git a/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyCommands.java b/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyCommands.java index aeb4f7c1..46362daf 100644 --- a/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyCommands.java +++ b/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyCommands.java @@ -30,6 +30,8 @@ public class LegacyCommands implements CommandMarker { public static final Method REGISTER_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "register", String.class, ArtifactType.class, String.class, boolean.class); public static final Method SUM_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "sum", int.class, int.class); + public static final Method LEGACY_ECHO_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "legacyEcho", String.class); + public static final Method SOME_METHOD = ReflectionUtils.findMethod(LegacyCommands.class, "someMethod", String.class, boolean.class); @CliCommand(value = "register module", help = "Register a new module") public String register( @@ -57,5 +59,17 @@ public class LegacyCommands implements CommandMarker { public int sum(@CliOption(key = "v1", unspecifiedDefaultValue = "38") int a, @CliOption(key = "v2", specifiedDefaultValue = "42") int b) { return a + b; } + + @CliCommand(value = "legacy-echo", help = "Echoes a message") + public String legacyEcho(@CliOption(key = "", mandatory = true) String message) { + return message; + } + @CliCommand(value = "someMethod", help = "Method used for testing purposes") + public String someMethod( + @CliOption(key = "key", mandatory = false, help = "The optional parameter") String parameter, + @CliOption(key = "option", help = "an option", specifiedDefaultValue = "true", unspecifiedDefaultValue = "false", mandatory = true) boolean option) { + return parameter + ", " + option; + } + } diff --git a/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyParameterResolverTest.java b/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyParameterResolverTest.java index 95f912b5..c90fefba 100644 --- a/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyParameterResolverTest.java +++ b/spring-shell2-shell1-adapter/src/test/java/org/springframework/shell2/legacy/LegacyParameterResolverTest.java @@ -33,6 +33,8 @@ import org.springframework.shell.converters.BooleanConverter; import org.springframework.shell.converters.EnumConverter; import org.springframework.shell.converters.StringConverter; import org.springframework.shell.core.Converter; +import org.springframework.shell.core.annotation.CliOption; +import org.springframework.shell2.ParameterDescription; import org.springframework.shell2.ParameterResolver; import org.springframework.shell2.Utils; import org.springframework.test.context.ContextConfiguration; @@ -153,6 +155,96 @@ public class LegacyParameterResolverTest { thrown.expectMessage("No converter found for --v2 from '42' to type int"); resolve(methodParameter, "--v1 1 --v2"); } + + @Test + public void testDescribeBothDefaultsNotDeclared() { + MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.REGISTER_METHOD, 1); + + ParameterDescription description = parameterResolver.describe(methodParameter); + + assertThat(description.keys()).containsExactly("--type"); + assertThat(description.formal()).isEqualTo(Utils.unCamelify(ArtifactType.class.getSimpleName())); + assertThat(description.defaultValue().isPresent()).isFalse(); + assertThat(description.mandatoryKey()).isTrue(); + + String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help(); + assertThat(description.help()).isEqualTo(expectedHelp); + } + + @Test + public void testDescribeBothDefaultsDeclared() { + MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 1); + + ParameterDescription description = parameterResolver.describe(methodParameter); + + assertThat(description.keys()).containsExactly("--option"); + assertThat(description.formal()).isEqualTo(boolean.class.getName()); + assertThat(description.defaultValue().get()).isEqualTo("false, or true if used as --option"); + assertThat(description.mandatoryKey()).isTrue(); + + String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help(); + assertThat(description.help()).isEqualTo(expectedHelp); + } + + @Test + public void testDescribeOnlySpecifiedDefaultDeclared() { + MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 1); + + ParameterDescription description = parameterResolver.describe(methodParameter); + + assertThat(description.keys()).containsExactly("--v2"); + assertThat(description.formal()).isEqualTo(int.class.getName()); + assertThat(description.defaultValue().get()).isEqualTo("42 if used as --v2"); + assertThat(description.mandatoryKey()).isTrue(); + + String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help(); + assertThat(description.help()).isEqualTo(expectedHelp); + } + + @Test + public void testDescribeOnlyUnspecifiedDefaultDeclared() { + MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SUM_METHOD, 0); + + ParameterDescription description = parameterResolver.describe(methodParameter); + + assertThat(description.keys()).containsExactly("--v1"); + assertThat(description.formal()).isEqualTo(int.class.getName()); + assertThat(description.defaultValue().get()).isEqualTo("38"); + assertThat(description.mandatoryKey()).isTrue(); + + String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help(); + assertThat(description.help()).isEqualTo(expectedHelp); + } + + @Test + public void testDescribeDefaultKey() { + MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.LEGACY_ECHO_METHOD, 0); + + ParameterDescription description = parameterResolver.describe(methodParameter); + + assertThat(description.keys()).isEmpty(); + assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName())); + assertThat(description.defaultValue().isPresent()).isFalse(); + assertThat(description.mandatoryKey()).isFalse(); + + String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help(); + assertThat(description.help()).isEqualTo(expectedHelp); + } + + @Test + public void testDescribeNonMandatoryNoDefaults() { + MethodParameter methodParameter = Utils.createMethodParameter(LegacyCommands.SOME_METHOD, 0); + + ParameterDescription description = parameterResolver.describe(methodParameter); + + assertThat(description.keys()).containsExactly("--key"); + assertThat(description.formal()).isEqualTo(Utils.unCamelify(String.class.getSimpleName())); + assertThat(description.defaultValue().get()).isEqualTo("null"); + assertThat(description.mandatoryKey()).isTrue(); + + String expectedHelp = methodParameter.getParameterAnnotation(CliOption.class).help(); + assertThat(description.help()).isEqualTo(expectedHelp); + } private Object resolve(MethodParameter methodParameter, String command) { return parameterResolver.resolve(methodParameter, asList(command.split(" ")));