spring-cloudGH-1083: Disallow reuse bean name for bindings

Resolves spring-cloud#1083

By default Spring Framework allows beans overriding via the same name.
The binding target definitions (`@Input` and `@Output`) populate beans as well
and when we use the same name for target we end up with unexpected behavior
but without errors.
Since it isn't so obvious via Spring Framework bean definition DSLs
(XML or Java & Annotations) how to override beans with the same name,
that is absolutely easy to use the same value for `@Input` and `@Output`
definitions even in different binding interfaces.
That's hard to analyze fro the target application since mostly
`@Input` and `@Output` produce `MessageChannel` beans.

* Fail fast with the `BeanDefinitionStoreException` when we meet existing
bean definition for the same name
* Add JavaDocs to the `@Input` and `@Output` to explain that their `value`
is a bean name, as well as destination by default

Since `@EnableBinding` is `@Inherited`, the inheritor picks up it from the
super class during configuration class parsing.
The parsing process logic is such that after the root class we go to parse its
super classes, and therefore come back to the `@EnableBinding` again.
In this case we process all the `@Import`s one more time and collect them to
the root `configurationClass`.
Essentially we get a duplication for the `ImportBeanDefinitionRegistrar`s
such as `BindingBeansRegistrar`.
The last one parsed `@EnableBinding` and registers appropriate beans for the
`@Input` and `@Output`, as well as for the binding interface - `BindableProxyFactory`.
But since we have it twice in the `configurationClass` we end up with
`BeanDefinitionStoreException` mentioned before.
That's how Spring Framework works with inheritance for configuration classes
and that's may be why it allows to override beans by default

* Skip parsing `@EnableBinding` one more time if the bean definition for
binding interface is already present in the `registry`
* Fix `AggregateWithMainTest` do not process `@ComponentScan` what causes
picking up the configuration classes for children contexts in the aggregation
* Fix `testBindableProxyFactoryCaching()` do not register `Source` and `Processor`
in the same application context because both of them cause registration for the
`Source.OUTPUT` bean

spring-cloud-stream-test-support/src/test/java/org/springframework/cloud/stream/test/aggregate/main/AggregateWithMainTest.java

@@ -21,7 +21,8 @@
 import org.junit.Test;
 
 import org.springframework.beans.factory.annotation.Autowired;
-import org.springframework.boot.autoconfigure.SpringBootApplication;
+import org.springframework.boot.SpringBootConfiguration;
+import org.springframework.boot.autoconfigure.EnableAutoConfiguration;
 import org.springframework.cloud.stream.aggregate.AggregateApplication;
 import org.springframework.cloud.stream.aggregate.AggregateApplicationBuilder;
 import org.springframework.cloud.stream.annotation.EnableBinding;
@@ -37,6 +38,7 @@
 
 /**
  * @author Marius Bogoevici
+ * @author Artem Bilan
  */
 public class AggregateWithMainTest {
 
@@ -59,13 +61,15 @@ public void testAggregateApplication() throws InterruptedException {
 		context.close();
 	}
 
-	@SpringBootApplication
+	@SpringBootConfiguration
+	@EnableAutoConfiguration
 	public static class MainConfiguration {
+
 	}
 
 	@Configuration
 	@EnableBinding(Processor.class)
-	public static class UppercaseProcessor {
+	static class UppercaseProcessor {
 
 		@Autowired
 		Processor processor;
@@ -74,15 +78,18 @@ public static class UppercaseProcessor {
 		public String transform(String in) {
 			return in.toUpperCase();
 		}
+
 	}
 
 	@Configuration
 	@EnableBinding(Processor.class)
-	public static class SuffixProcessor {
+	static class SuffixProcessor {
 
 		@Transformer(inputChannel = Processor.INPUT, outputChannel = Processor.OUTPUT)
 		public String transform(String in) {
 			return in + " WORLD!";
 		}
+
 	}
+
 }

spring-cloud-stream/src/main/java/org/springframework/cloud/stream/annotation/Input.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.
@@ -40,6 +40,12 @@
 @Documented
 public @interface Input {
 
+	/**
+	 * Specify the binding target name;
+	 * used as a bean name for binding target
+	 * and as a destination name by default.
+	 * @return the binding target name
+	 */
 	String value() default "";
 
 }

spring-cloud-stream/src/main/java/org/springframework/cloud/stream/annotation/Output.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.
@@ -30,6 +30,7 @@
  *
  * @author Dave Syer
  * @author Marius Bogoevici
+ * @author Artem Bilan
  */
 
 @Qualifier
@@ -40,6 +41,12 @@
 @Documented
 public @interface Output {
 
+	/**
+	 * Specify the binding target name;
+	 * used as a bean name for binding target
+	 * and as a destination name by default.
+	 * @return the binding target name
+	 */
 	String value() default "";
 
 }

spring-cloud-stream/src/main/java/org/springframework/cloud/stream/binding/BindingBeanDefinitionRegistryUtils.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.
@@ -20,6 +20,7 @@
 import java.lang.reflect.Method;
 import java.util.Map;
 
+import org.springframework.beans.factory.BeanDefinitionStoreException;
 import org.springframework.beans.factory.support.AutowireCandidateQualifier;
 import org.springframework.beans.factory.support.BeanDefinitionRegistry;
 import org.springframework.beans.factory.support.RootBeanDefinition;
@@ -36,6 +37,7 @@
  *
  * @author Marius Bogoevici
  * @author Dave Syer
+ * @author Artem Bilan
  */
 public abstract class BindingBeanDefinitionRegistryUtils {
 
@@ -57,6 +59,11 @@ private static void registerBindingTargetBeanDefinition(Class<? extends Annotati
 			String qualifierValue, String name, String bindingTargetInterfaceBeanName,
 			String bindingTargetInterfaceMethodName, BeanDefinitionRegistry registry) {
 
+		if (registry.containsBeanDefinition(name)) {
+			throw new BeanDefinitionStoreException(bindingTargetInterfaceBeanName, name,
+					"bean definition with this name already exists - " + registry.getBeanDefinition(name));
+		}
+
 		RootBeanDefinition rootBeanDefinition = new RootBeanDefinition();
 		rootBeanDefinition.setFactoryBeanName(bindingTargetInterfaceBeanName);
 		rootBeanDefinition.setUniqueFactoryMethodName(bindingTargetInterfaceMethodName);

spring-cloud-stream/src/main/java/org/springframework/cloud/stream/config/BindingBeansRegistrar.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.
@@ -29,6 +29,7 @@
 /**
  * @author Marius Bogoevici
  * @author Dave Syer
+ * @author Artem Bilan
  */
 
 public class BindingBeansRegistrar implements ImportBeanDefinitionRegistrar {
@@ -40,11 +41,13 @@ public void registerBeanDefinitions(AnnotationMetadata metadata,
 				ClassUtils.resolveClassName(metadata.getClassName(), null),
 				EnableBinding.class);
 		for (Class<?> type : collectClasses(attrs, metadata.getClassName())) {
-			BindingBeanDefinitionRegistryUtils.registerBindingTargetBeanDefinitions(type,
-					type.getName(), registry);
-			BindingBeanDefinitionRegistryUtils.registerBindingTargetsQualifiedBeanDefinitions(
-					ClassUtils.resolveClassName(metadata.getClassName(), null), type,
-					registry);
+			if (!registry.containsBeanDefinition(type.getName())) {
+				BindingBeanDefinitionRegistryUtils.registerBindingTargetBeanDefinitions(type,
+						type.getName(), registry);
+				BindingBeanDefinitionRegistryUtils.registerBindingTargetsQualifiedBeanDefinitions(
+						ClassUtils.resolveClassName(metadata.getClassName(), null), type,
+						registry);
+			}
 		}
 	}
 

spring-cloud-stream/src/test/java/org/springframework/cloud/stream/aggregation/AggregationTest.java

@@ -32,8 +32,8 @@
 import org.springframework.cloud.stream.aggregate.AggregateApplicationBuilder;
 import org.springframework.cloud.stream.aggregate.AggregateApplicationBuilder.SourceConfigurer;
 import org.springframework.cloud.stream.aggregate.SharedBindingTargetRegistry;
-import org.springframework.cloud.stream.aggregate.SharedChannelRegistry;
 import org.springframework.cloud.stream.annotation.EnableBinding;
+import org.springframework.cloud.stream.annotation.Output;
 import org.springframework.cloud.stream.binding.BindableProxyFactory;
 import org.springframework.cloud.stream.binding.BindingTargetFactory;
 import org.springframework.cloud.stream.messaging.Processor;
@@ -51,6 +51,7 @@
 /**
  * @author Marius Bogoevici
  * @author Ilayaperumal Gopinathan
+ * @author Artem Bilan
  */
 public class AggregationTest {
 
@@ -84,13 +85,14 @@ public void aggregation() {
 	}
 
 	@Test
+	@SuppressWarnings("deprecation")
 	public void testModuleAggregationUsingSharedChannelRegistry() {
 		// test backward compatibility
 		aggregatedApplicationContext = new AggregateApplicationBuilder(
 				MockBinderRegistryConfiguration.class, "--server.port=0")
 						.from(TestSource.class).to(TestProcessor.class).run();
-		SharedChannelRegistry sharedChannelRegistry = aggregatedApplicationContext
-				.getBean(SharedChannelRegistry.class);
+		org.springframework.cloud.stream.aggregate.SharedChannelRegistry sharedChannelRegistry =
+				aggregatedApplicationContext.getBean(org.springframework.cloud.stream.aggregate.SharedChannelRegistry.class);
 		BindingTargetFactory channelFactory = aggregatedApplicationContext
 				.getBean(BindingTargetFactory.class);
 		assertThat(channelFactory).isNotNull();
@@ -99,6 +101,7 @@ public void testModuleAggregationUsingSharedChannelRegistry() {
 	}
 
 	@Test
+	@SuppressWarnings("unchecked")
 	public void testParentArgsAndSources() {
 		List<String> argsToVerify = new ArrayList<>();
 		argsToVerify.add("--foo1=bar1");
@@ -113,7 +116,6 @@ public void testParentArgsAndSources() {
 				.namespace("foo").to(TestProcessor.class).namespace("bar")
 				.run("--foo3=bar3", "--server.port=0");
 		DirectFieldAccessor aggregateApplicationBuilderAccessor = new DirectFieldAccessor(aggregateApplicationBuilder);
-		@SuppressWarnings("unchecked")
 		final List<String> parentArgs = (List<String>) aggregateApplicationBuilderAccessor.getPropertyValue(
 				"parentArgs");
 		assertThat(parentArgs).containsExactlyInAnyOrder(argsToVerify.toArray(new String[argsToVerify.size()]));
@@ -125,8 +127,8 @@ public void testParentArgsAndSources() {
 	}
 
 	@Test
+	@SuppressWarnings("unchecked")
 	public void testParentArgsAndSourcesWithWebDisabled() {
-		List<String> argsToVerify = new ArrayList<>();
 		AggregateApplicationBuilder aggregateApplicationBuilder = new AggregateApplicationBuilder(
 				MockBinderRegistryConfiguration.class, "--foo1=bar1");
 		final ConfigurableApplicationContext context = aggregateApplicationBuilder
@@ -155,7 +157,8 @@ public void testNamespacePrefixesFromCmdLine() {
 				((SourceConfigurer) aggregateApplicationBuilderAccessor.getPropertyValue("sourceConfigurer"))
 						.getArgs(),
 				new String[] { "--foo1=bar1" }));
-		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers = (List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
+		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers =
+				(List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
 				.getPropertyValue("processorConfigurers");
 		for (AggregateApplicationBuilder.ProcessorConfigurer processorConfigurer : processorConfigurers) {
 			if (processorConfigurer.getNamespace().equals("b")) {
@@ -185,7 +188,8 @@ public void testNamespacePrefixesFromCmdLineVsArgs() {
 				((SourceConfigurer) aggregateApplicationBuilderAccessor.getPropertyValue("sourceConfigurer"))
 						.getArgs(),
 				new String[] { "--fooValue=bara" }));
-		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers = (List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
+		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers =
+				(List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
 				.getPropertyValue("processorConfigurers");
 		for (AggregateApplicationBuilder.ProcessorConfigurer processorConfigurer : processorConfigurers) {
 			if (processorConfigurer.getNamespace().equals("b")) {
@@ -215,7 +219,8 @@ public void testNamespacePrefixesFromCmdLineWithRelaxedNames() {
 				((SourceConfigurer) aggregateApplicationBuilderAccessor.getPropertyValue("sourceConfigurer"))
 						.getArgs(),
 				new String[] { "--fooValue=bara" }));
-		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers = (List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
+		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers =
+				(List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
 				.getPropertyValue("processorConfigurers");
 		for (AggregateApplicationBuilder.ProcessorConfigurer processorConfigurer : processorConfigurers) {
 			if (processorConfigurer.getNamespace().equals("b")) {
@@ -247,7 +252,8 @@ public void testNamespacePrefixesFromCmdLineWithRelaxedNamesAndMorePropertySourc
 				((SourceConfigurer) aggregateApplicationBuilderAccessor.getPropertyValue("sourceConfigurer"))
 						.getArgs(),
 				new String[] { "--fooValue=bara" }));
-		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers = (List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
+		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers =
+				(List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
 				.getPropertyValue("processorConfigurers");
 		for (AggregateApplicationBuilder.ProcessorConfigurer processorConfigurer : processorConfigurers) {
 			if (processorConfigurer.getNamespace().equals("b")) {
@@ -279,9 +285,8 @@ public void testNamespacePrefixesWithoutCmdLinePropertySource() {
 				((SourceConfigurer) aggregateApplicationBuilderAccessor.getPropertyValue("sourceConfigurer"))
 						.getArgs(),
 				new String[] { "--foo-value=sysbara" }));
-		final List<AggregateApplicationBuilder.ProcessorConfigurer> processorConfigurers = (List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
-				.getPropertyValue("processorConfigurers");
-		for (AggregateApplicationBuilder.ProcessorConfigurer processorConfigurer : ((List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
+		for (AggregateApplicationBuilder.ProcessorConfigurer processorConfigurer :
+				((List<AggregateApplicationBuilder.ProcessorConfigurer>) aggregateApplicationBuilderAccessor
 				.getPropertyValue(
 						"processorConfigurers"))) {
 			if (processorConfigurer.getNamespace().equals("b")) {
@@ -329,13 +334,14 @@ public void testNamespacePrefixesWithCAPSProperties() {
 	}
 
 	@Test
+	@SuppressWarnings("deprecation")
 	public void testNamespaces() {
 		aggregatedApplicationContext = new AggregateApplicationBuilder(
 				MockBinderRegistryConfiguration.class, "--server.port=0")
 						.from(TestSource.class).namespace("foo").to(TestProcessor.class)
 						.namespace("bar").run();
-		SharedChannelRegistry sharedChannelRegistry = aggregatedApplicationContext
-				.getBean(SharedChannelRegistry.class);
+		org.springframework.cloud.stream.aggregate.SharedChannelRegistry sharedChannelRegistry =
+				aggregatedApplicationContext.getBean(org.springframework.cloud.stream.aggregate.SharedChannelRegistry.class);
 		BindingTargetFactory channelFactory = aggregatedApplicationContext
 				.getBean(BindingTargetFactory.class);
 		Object fooOutput = sharedChannelRegistry.get("foo.output");
@@ -353,16 +359,22 @@ public void testNamespaces() {
 	public void testBindableProxyFactoryCaching() {
 		AnnotationConfigApplicationContext context = new AnnotationConfigApplicationContext(
 				MockBinderRegistryConfiguration.class,
-				TestSource.class, TestProcessor.class);
+				TestSource2.class, TestProcessor.class);
 		Map<String, BindableProxyFactory> factories = context.getBeansOfType(BindableProxyFactory.class);
 		assertThat(factories).hasSize(2);
 
 		Map<String, Source> sources = context.getBeansOfType(Source.class);
-		assertThat(sources).hasSize(2);
+		assertThat(sources).hasSize(1);
 		for (Source source : sources.values()) {
 			source.output();
 		}
 
+		Map<String, FooSource> fooSources = context.getBeansOfType(FooSource.class);
+		assertThat(fooSources).hasSize(1);
+		for (FooSource source : fooSources.values()) {
+			source.output();
+		}
+
 		Map<String, Processor> processors = context.getBeansOfType(Processor.class);
 		assertThat(processors).hasSize(1);
 		for (Processor processor : processors.values()) {
@@ -377,6 +389,9 @@ public void testBindableProxyFactoryCaching() {
 			if (factory.getObjectType() == Source.class) {
 				assertThat(targetCache).hasSize(1);
 			}
+			if (factory.getObjectType() == FooSource.class) {
+				assertThat(targetCache).hasSize(1);
+			}
 			else if (factory.getObjectType() == Processor.class) {
 				assertThat(targetCache).hasSize(2);
 			}
@@ -399,6 +414,20 @@ public static class TestProcessor {
 
 	}
 
+	public interface FooSource {
+
+		@Output("fooOutput")
+		MessageChannel output();
+
+	}
+
+	@EnableBinding(FooSource.class)
+	@EnableAutoConfiguration
+	public static class TestSource2 {
+
+	}
+
+
 	@Configuration
 	public static class DummyConfig {