From c46bf91daeb2e5f2dd4ffef9ef971846c7902e54 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Mon, 27 Feb 2023 10:12:09 +0000 Subject: [PATCH] Wrong arity for default boolean type - Set boolean type arity zero or more as default. - Fixes #675 --- .../CommandRegistrationFactoryBean.java | 8 +-- .../CommandRegistrationFactoryBeanTests.java | 65 +++++++++++++++++++ .../StandardMethodTargetRegistrar.java | 8 +-- .../StandardMethodTargetRegistrarTests.java | 17 +++++ 4 files changed, 90 insertions(+), 8 deletions(-) diff --git a/spring-shell-core/src/main/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBean.java b/spring-shell-core/src/main/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBean.java index 0c9f92ac..c692a082 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBean.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBean.java @@ -255,10 +255,10 @@ class CommandRegistrationFactoryBean implements FactoryBean } else { if (ClassUtils.isAssignable(boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } else if (ClassUtils.isAssignable(Boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } } @@ -298,10 +298,10 @@ class CommandRegistrationFactoryBean implements FactoryBean optionSpec.required(); optionSpec.position(mp.getParameterIndex()); if (ClassUtils.isAssignable(boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } else if (ClassUtils.isAssignable(Boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } else { optionSpec.arity(OptionArity.EXACTLY_ONE); diff --git a/spring-shell-core/src/test/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBeanTests.java b/spring-shell-core/src/test/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBeanTests.java index 40bb2bf9..4d18823c 100644 --- a/spring-shell-core/src/test/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBeanTests.java +++ b/spring-shell-core/src/test/java/org/springframework/shell/command/annotation/support/CommandRegistrationFactoryBeanTests.java @@ -144,6 +144,71 @@ class CommandRegistrationFactoryBeanTests { } } + + @Test + void setsOptionValuesWithBoolean() { + configCommon(OptionValuesWithBoolean.class, new OptionValuesWithBoolean(), "command1", new Class[] { boolean.class }) + .run((context) -> { + CommandRegistrationFactoryBean fb = context.getBean(FACTORYBEANREF, + CommandRegistrationFactoryBean.class); + assertThat(fb).isNotNull(); + CommandRegistration registration = fb.getObject(); + assertThat(registration).isNotNull(); + assertThat(registration.getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(registration.getOptions().get(0).getArityMax()).isEqualTo(1); + }); + configCommon(OptionValuesWithBoolean.class, new OptionValuesWithBoolean(), "command2", new Class[] { Boolean.class }) + .run((context) -> { + CommandRegistrationFactoryBean fb = context.getBean(FACTORYBEANREF, + CommandRegistrationFactoryBean.class); + assertThat(fb).isNotNull(); + CommandRegistration registration = fb.getObject(); + assertThat(registration).isNotNull(); + assertThat(registration.getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(registration.getOptions().get(0).getArityMax()).isEqualTo(1); + }); + configCommon(OptionValuesWithBoolean.class, new OptionValuesWithBoolean(), "command3", new Class[] { Boolean.class }) + .run((context) -> { + CommandRegistrationFactoryBean fb = context.getBean(FACTORYBEANREF, + CommandRegistrationFactoryBean.class); + assertThat(fb).isNotNull(); + CommandRegistration registration = fb.getObject(); + assertThat(registration).isNotNull(); + assertThat(registration.getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(registration.getOptions().get(0).getArityMax()).isEqualTo(1); + }); + configCommon(OptionValuesWithBoolean.class, new OptionValuesWithBoolean(), "command4", new Class[] { Boolean.class }) + .run((context) -> { + CommandRegistrationFactoryBean fb = context.getBean(FACTORYBEANREF, + CommandRegistrationFactoryBean.class); + assertThat(fb).isNotNull(); + CommandRegistration registration = fb.getObject(); + assertThat(registration).isNotNull(); + assertThat(registration.getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(registration.getOptions().get(0).getArityMax()).isEqualTo(1); + }); + } + + @Command + private static class OptionValuesWithBoolean { + + @Command + void command1(@Option(defaultValue = "false") boolean arg) { + } + + @Command + void command2(@Option(defaultValue = "false") Boolean arg) { + } + + @Command + void command3(@Option Boolean arg) { + } + + @Command + void command4(Boolean arg) { + } + } + private ApplicationContextRunner configCommon(Class type, T bean) { return configCommon(type, bean, "command", new Class[0]); } diff --git a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java index a55144ee..60f2c6f5 100644 --- a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java +++ b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java @@ -157,10 +157,10 @@ public class StandardMethodTargetRegistrar implements MethodTargetRegistrar { } else { if (ClassUtils.isAssignable(boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } else if (ClassUtils.isAssignable(Boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } } if (!ObjectUtils.nullSafeEquals(so.defaultValue(), ShellOption.NONE) @@ -198,10 +198,10 @@ public class StandardMethodTargetRegistrar implements MethodTargetRegistrar { .required() .position(mp.getParameterIndex()); if (ClassUtils.isAssignable(boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } else if (ClassUtils.isAssignable(Boolean.class, parameterType)) { - optionSpec.arity(OptionArity.ZERO); + optionSpec.arity(OptionArity.ZERO_OR_ONE); } else { optionSpec.arity(OptionArity.EXACTLY_ONE); diff --git a/spring-shell-standard/src/test/java/org/springframework/shell/standard/StandardMethodTargetRegistrarTests.java b/spring-shell-standard/src/test/java/org/springframework/shell/standard/StandardMethodTargetRegistrarTests.java index fe7885e6..5c86113a 100644 --- a/spring-shell-standard/src/test/java/org/springframework/shell/standard/StandardMethodTargetRegistrarTests.java +++ b/spring-shell-standard/src/test/java/org/springframework/shell/standard/StandardMethodTargetRegistrarTests.java @@ -417,16 +417,29 @@ public class StandardMethodTargetRegistrarTests { assertThat(catalog.getRegistrations().get("foo1").getOptions()).hasSize(1); assertThat(catalog.getRegistrations().get("foo1").getOptions().get(0).getDefaultValue()).isEqualTo("false"); assertThat(catalog.getRegistrations().get("foo1").getOptions().get(0).isRequired()).isFalse(); + assertThat(catalog.getRegistrations().get("foo1").getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(catalog.getRegistrations().get("foo1").getOptions().get(0).getArityMax()).isEqualTo(1); assertThat(catalog.getRegistrations().get("foo2")).isNotNull(); assertThat(catalog.getRegistrations().get("foo2").getOptions()).hasSize(1); assertThat(catalog.getRegistrations().get("foo2").getOptions().get(0).getDefaultValue()).isEqualTo("true"); assertThat(catalog.getRegistrations().get("foo2").getOptions().get(0).isRequired()).isFalse(); + assertThat(catalog.getRegistrations().get("foo2").getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(catalog.getRegistrations().get("foo2").getOptions().get(0).getArityMax()).isEqualTo(1); assertThat(catalog.getRegistrations().get("foo3")).isNotNull(); assertThat(catalog.getRegistrations().get("foo3").getOptions()).hasSize(1); assertThat(catalog.getRegistrations().get("foo3").getOptions().get(0).isRequired()).isFalse(); assertThat(catalog.getRegistrations().get("foo3").getOptions().get(0).getDefaultValue()).isEqualTo("false"); + assertThat(catalog.getRegistrations().get("foo3").getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(catalog.getRegistrations().get("foo3").getOptions().get(0).getArityMax()).isEqualTo(1); + + assertThat(catalog.getRegistrations().get("foo4")).isNotNull(); + assertThat(catalog.getRegistrations().get("foo4").getOptions()).hasSize(1); + assertThat(catalog.getRegistrations().get("foo4").getOptions().get(0).isRequired()).isTrue(); + assertThat(catalog.getRegistrations().get("foo4").getOptions().get(0).getDefaultValue()).isNull(); + assertThat(catalog.getRegistrations().get("foo4").getOptions().get(0).getArityMin()).isEqualTo(0); + assertThat(catalog.getRegistrations().get("foo4").getOptions().get(0).getArityMax()).isEqualTo(1); } @ShellComponent @@ -443,6 +456,10 @@ public class StandardMethodTargetRegistrarTests { @ShellMethod(value = "foo3") public void foo3(@ShellOption boolean arg1) { } + + @ShellMethod(value = "foo4") + public void foo4(boolean arg1) { + } } @Test