diff --git a/spring-shell-core/src/main/java/org/springframework/shell/ConfigurableCommandRegistry.java b/spring-shell-core/src/main/java/org/springframework/shell/ConfigurableCommandRegistry.java new file mode 100644 index 00000000..6ec2677a --- /dev/null +++ b/spring-shell-core/src/main/java/org/springframework/shell/ConfigurableCommandRegistry.java @@ -0,0 +1,47 @@ +/* + * Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.shell; + +import java.util.HashMap; +import java.util.Map; +import java.util.TreeMap; + +/** + * A {@link CommandRegistry} that supports registration of new commands. + * + *

Makes sure that no two commands are registered with the same name.

+ * + * @author Eric Bottard + */ +public class ConfigurableCommandRegistry implements CommandRegistry { + + private Map commands = new HashMap<>(); + + @Override + public Map listCommands() { + return new TreeMap<>(commands); + } + + public void register(String name, MethodTarget target) { + MethodTarget previous = commands.get(name); + if (previous != null) { + throw new IllegalArgumentException( + String.format("Illegal registration for command '%s': Attempt to register both '%s' and '%s'", name, target, previous)); + } + commands.put(name, target); + } +} diff --git a/spring-shell-core/src/main/java/org/springframework/shell/MethodTargetResolver.java b/spring-shell-core/src/main/java/org/springframework/shell/MethodTargetRegistrar.java similarity index 75% rename from spring-shell-core/src/main/java/org/springframework/shell/MethodTargetResolver.java rename to spring-shell-core/src/main/java/org/springframework/shell/MethodTargetRegistrar.java index 3b76bd02..4e04aa7e 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/MethodTargetResolver.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/MethodTargetRegistrar.java @@ -16,19 +16,17 @@ package org.springframework.shell; -import java.util.Map; - /** - * Strategy interface for discovering commands. + * Strategy interface for registering commands. * * @author Eric Bottard * @author Camilo Gonzalez */ -public interface MethodTargetResolver { +public interface MethodTargetRegistrar { /** - * Return a mapping from {@literal } to actual behavior. + * Register mappings from {@literal } to actual behavior. */ - public Map resolve(); + void register(ConfigurableCommandRegistry registry); } diff --git a/spring-shell-core/src/main/java/org/springframework/shell/Shell.java b/spring-shell-core/src/main/java/org/springframework/shell/Shell.java index 3bb15b9b..f09cd81f 100644 --- a/spring-shell-core/src/main/java/org/springframework/shell/Shell.java +++ b/spring-shell-core/src/main/java/org/springframework/shell/Shell.java @@ -33,8 +33,6 @@ import javax.validation.Validation; import javax.validation.executable.ExecutableValidator; import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.boot.ApplicationArguments; -import org.springframework.boot.ApplicationRunner; import org.springframework.context.ApplicationContext; import org.springframework.core.MethodParameter; import org.springframework.util.ReflectionUtils; @@ -80,9 +78,11 @@ public class Shell implements CommandRegistry { @PostConstruct public void gatherMethodTargets() throws Exception { - for (MethodTargetResolver resolver : applicationContext.getBeansOfType(MethodTargetResolver.class).values()) { - methodTargets.putAll(resolver.resolve()); + ConfigurableCommandRegistry registry = new ConfigurableCommandRegistry(); + for (MethodTargetRegistrar resolver : applicationContext.getBeansOfType(MethodTargetRegistrar.class).values()) { + resolver.register(registry); } + methodTargets = registry.listCommands(); } /** diff --git a/spring-shell-core/src/test/java/org/springframework/shell/ConfigurableCommandRegistryTest.java b/spring-shell-core/src/test/java/org/springframework/shell/ConfigurableCommandRegistryTest.java new file mode 100644 index 00000000..2732fb66 --- /dev/null +++ b/spring-shell-core/src/test/java/org/springframework/shell/ConfigurableCommandRegistryTest.java @@ -0,0 +1,59 @@ +/* + * Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.shell; + +import static org.hamcrest.collection.IsMapContaining.hasEntry; +import static org.hamcrest.collection.IsMapContaining.hasKey; +import static org.junit.Assert.*; + +import org.hamcrest.collection.IsMapContaining; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; + +/** + * Unit tests for {@link ConfigurableCommandRegistry}. + * + * @author Eric Bottard + */ +public class ConfigurableCommandRegistryTest { + + @Rule + public ExpectedException thrown= ExpectedException.none(); + + @Test + public void testRegistration() { + ConfigurableCommandRegistry registry = new ConfigurableCommandRegistry(); + registry.register("foo", MethodTarget.of("toString", this, "some command")); + + assertThat(registry.listCommands(), hasKey("foo")); + } + + @Test + public void testDoubleRegistration() { + ConfigurableCommandRegistry registry = new ConfigurableCommandRegistry(); + registry.register("foo", MethodTarget.of("toString", this, "some command")); + + thrown.expect(IllegalArgumentException.class); + thrown.expectMessage("foo"); + thrown.expectMessage("toString"); + thrown.expectMessage("hashCode"); + + registry.register("foo", MethodTarget.of("hashCode", this, "some command")); + } + +} diff --git a/spring-shell-samples/src/main/java/org/springframework/shell/samples/standard/Commands.java b/spring-shell-samples/src/main/java/org/springframework/shell/samples/standard/Commands.java index 1f72a649..91ef737f 100644 --- a/spring-shell-samples/src/main/java/org/springframework/shell/samples/standard/Commands.java +++ b/spring-shell-samples/src/main/java/org/springframework/shell/samples/standard/Commands.java @@ -35,7 +35,7 @@ import org.springframework.stereotype.Component; * * @author Eric Bottard */ -@ShellComponent("") +@ShellComponent() public class Commands { @ShellMethod(help = "a command whose name looks the same as another one", value = "help me out") diff --git a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java index df108497..3f8b017f 100644 --- a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java +++ b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyAdapterAutoConfiguration.java @@ -35,8 +35,8 @@ import org.springframework.shell.converters.SimpleFileConverter; public class LegacyAdapterAutoConfiguration { @Bean - public LegacyMethodTargetResolver legacyMethodTargetResolver() { - return new LegacyMethodTargetResolver(); + public LegacyMethodTargetRegistrar legacyMethodTargetResolver() { + return new LegacyMethodTargetRegistrar(); } @Bean diff --git a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyMethodTargetResolver.java b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyMethodTargetRegistrar.java similarity index 74% rename from spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyMethodTargetResolver.java rename to spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyMethodTargetRegistrar.java index 8fc90edc..9e53a0d7 100644 --- a/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyMethodTargetResolver.java +++ b/spring-shell-shell1-adapter/src/main/java/org/springframework/shell/legacy/LegacyMethodTargetRegistrar.java @@ -20,18 +20,20 @@ import static org.springframework.util.StringUtils.collectionToDelimitedString; import java.util.HashMap; import java.util.Map; +import java.util.TreeMap; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; +import org.springframework.shell.ConfigurableCommandRegistry; import org.springframework.shell.core.CommandMarker; import org.springframework.shell.core.annotation.CliCommand; import org.springframework.shell.MethodTarget; -import org.springframework.shell.MethodTargetResolver; +import org.springframework.shell.MethodTargetRegistrar; import org.springframework.stereotype.Component; import org.springframework.util.ReflectionUtils; /** - * A {@link MethodTargetResolver} that discovers methods annotated with {@link CliCommand} on beans + * A {@link MethodTargetRegistrar} that discovers methods annotated with {@link CliCommand} on beans * implementing the {@link CommandMarker} marker interface. * * @author Eric Bottard @@ -39,31 +41,33 @@ import org.springframework.util.ReflectionUtils; * @author Camilo Gonzalez */ @Component -public class LegacyMethodTargetResolver implements MethodTargetResolver { +public class LegacyMethodTargetRegistrar implements MethodTargetRegistrar { @Autowired private ApplicationContext applicationContext; + + private Map commands = new TreeMap<>(); @Override - public Map resolve() { - Map methodTargets = new HashMap<>(); + public void register(ConfigurableCommandRegistry registry) { Map beans = applicationContext.getBeansOfType(CommandMarker.class); for (Object bean : beans.values()) { Class clazz = bean.getClass(); ReflectionUtils.doWithMethods(clazz, method -> { CliCommand cliCommand = method.getAnnotation(CliCommand.class); for (String key : cliCommand.value()) { - methodTargets.put(key, new MethodTarget(method, bean, cliCommand.help())); + MethodTarget target = new MethodTarget(method, bean, cliCommand.help()); + registry.register(key, target); + commands.put(key, target); } }, method -> method.getAnnotation(CliCommand.class) != null); } - return methodTargets; } @Override public String toString() { return getClass().getSimpleName() + " contributing " - + collectionToDelimitedString(resolve().keySet(), ", ", "[", "]"); + + collectionToDelimitedString(commands.keySet(), ", ", "[", "]"); } } diff --git a/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyMethodTargetResolverTest.java b/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyMethodTargetRegistrarTest.java similarity index 72% rename from spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyMethodTargetResolverTest.java rename to spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyMethodTargetRegistrarTest.java index c454aaae..ca88eb96 100644 --- a/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyMethodTargetResolverTest.java +++ b/spring-shell-shell1-adapter/src/test/java/org/springframework/shell/legacy/LegacyMethodTargetRegistrarTest.java @@ -27,27 +27,32 @@ import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; +import org.springframework.shell.ConfigurableCommandRegistry; import org.springframework.shell.MethodTarget; -import org.springframework.shell.MethodTargetResolver; +import org.springframework.shell.MethodTargetRegistrar; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** - * Created by ericbottard on 09/12/15. + * Unit tests for {@link LegacyMethodTargetRegistrar}. + * + * @author Eric Bottard */ @RunWith(SpringJUnit4ClassRunner.class) -@ContextConfiguration(classes = LegacyMethodTargetResolverTest.Config.class) -public class LegacyMethodTargetResolverTest { +@ContextConfiguration(classes = LegacyMethodTargetRegistrarTest.Config.class) +public class LegacyMethodTargetRegistrarTest { @Autowired private LegacyCommands legacyCommands; @Autowired - private MethodTargetResolver resolver; + private MethodTargetRegistrar resolver; @Test public void findsMethodsAnnotatedWithCliCommand() throws Exception { - Map targets = resolver.resolve(); + ConfigurableCommandRegistry registry = new ConfigurableCommandRegistry(); + resolver.register(registry); + Map targets = registry.listCommands(); assertThat(targets).contains(entry( "register module", @@ -64,8 +69,8 @@ public class LegacyMethodTargetResolverTest { } @Bean - public MethodTargetResolver methodTargetResolver() { - return new LegacyMethodTargetResolver(); + public MethodTargetRegistrar methodTargetResolver() { + return new LegacyMethodTargetRegistrar(); } } diff --git a/spring-shell-standard-commands/src/test/java/org/springframework/shell/standard/commands/StandardCommandsAutoConfigurationTest.java b/spring-shell-standard-commands/src/test/java/org/springframework/shell/standard/commands/StandardCommandsAutoConfigurationTest.java new file mode 100644 index 00000000..b1ca3025 --- /dev/null +++ b/spring-shell-standard-commands/src/test/java/org/springframework/shell/standard/commands/StandardCommandsAutoConfigurationTest.java @@ -0,0 +1,26 @@ +/* + * Copyright 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. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.shell.standard.commands; + +/** + * Tests for {@link StandardCommandsAutoConfiguration}. + * + * @author Eric Bottard + */ +public class StandardCommandsAutoConfigurationTest { + +} diff --git a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardAPIAutoConfiguration.java b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardAPIAutoConfiguration.java index 3d5d8f69..1e839f3a 100644 --- a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardAPIAutoConfiguration.java +++ b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardAPIAutoConfiguration.java @@ -20,7 +20,7 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.core.convert.ConversionService; import org.springframework.shell.CommandRegistry; -import org.springframework.shell.MethodTargetResolver; +import org.springframework.shell.MethodTargetRegistrar; import org.springframework.shell.ParameterResolver; /** @@ -42,8 +42,8 @@ public class StandardAPIAutoConfiguration { } @Bean - public MethodTargetResolver standardMethodTargetResolver() { - return new StandardMethodTargetResolver(); + public MethodTargetRegistrar standardMethodTargetResolver() { + return new StandardMethodTargetRegistrar(); } @Bean diff --git a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetResolver.java b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java similarity index 75% rename from spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetResolver.java rename to spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java index 74ef49cb..874598a2 100644 --- a/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetResolver.java +++ b/spring-shell-standard/src/main/java/org/springframework/shell/standard/StandardMethodTargetRegistrar.java @@ -23,27 +23,28 @@ import java.util.Map; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.context.ApplicationContext; +import org.springframework.shell.ConfigurableCommandRegistry; import org.springframework.shell.MethodTarget; -import org.springframework.shell.MethodTargetResolver; -import org.springframework.stereotype.Component; +import org.springframework.shell.MethodTargetRegistrar; import org.springframework.util.ReflectionUtils; /** - * The standard implementation of {@link MethodTargetResolver} for new shell applications, + * The standard implementation of {@link MethodTargetRegistrar} for new shell applications, * resolves methods annotated with {@link ShellMethod} on {@link ShellComponent} beans. * * @author Eric Bottard * @author Florent Biville * @author Camilo Gonzalez */ -public class StandardMethodTargetResolver implements MethodTargetResolver { +public class StandardMethodTargetRegistrar implements MethodTargetRegistrar { @Autowired private ApplicationContext applicationContext; + + private Map commands = new HashMap<>(); @Override - public Map resolve() { - Map methodTargets = new HashMap<>(); + public void register(ConfigurableCommandRegistry registry) { Map commandBeans = applicationContext.getBeansWithAnnotation(ShellComponent.class); for (Object bean : commandBeans.values()) { Class clazz = bean.getClass(); @@ -54,16 +55,17 @@ public class StandardMethodTargetResolver implements MethodTargetResolver { keys = new String[] {method.getName()}; } for (String key : keys) { - methodTargets.put(key, new MethodTarget(method, bean, shellMapping.help())); + MethodTarget target = new MethodTarget(method, bean, shellMapping.help()); + registry.register(key, target); + commands.put(key, target); } }, method -> method.getAnnotation(ShellMethod.class) != null); } - return methodTargets; } @Override public String toString() { return getClass().getSimpleName() + " contributing " - + collectionToDelimitedString(resolve().keySet(), ", ", "[", "]"); + + collectionToDelimitedString(commands.keySet(), ", ", "[", "]"); } }