INT-3916: Don't Use CTOR Injection in FactoryBean

JIRA: https://jira.spring.io/browse/INT-3916

The `JpaOutboundGatewayFactoryBean` used CTOR injection for the `JpaExecutor`.
That one, in turn, uses CTOR injection for the `EntityManagerFactory`.

Such a dependency may cause the `early bean instantiating` in case of `AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck()`.
And we end up with the `BeanCurrentlyInCreationException`.

Therefore no one `FactoryBean` should use CTOR injection if there is a potential hierarchical dependency.

NOTE: there is no tests on the matter, since we don't change the components behavior.
The `JPA` sample application will be changed to the Boot to track this fix.

**Cherry-pick to 4.2.x**

Address PR comments and fix other `FactoryBean`s for the same issue, when it is reasonable

Polishing

Address PR comments

Make setter `setSockJsTaskScheduler` as `public`

spring-integration-core/src/main/java/org/springframework/integration/config/CorrelationStrategyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2010 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -18,6 +18,7 @@ package org.springframework.integration.config;
 import java.lang.reflect.Method;
 
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.integration.IntegrationMessageHeaderAccessor;
 import org.springframework.integration.aggregator.CorrelationStrategy;
 import org.springframework.integration.aggregator.HeaderAttributeCorrelationStrategy;
@@ -26,52 +27,81 @@ import org.springframework.integration.util.MessagingAnnotationUtils;
 import org.springframework.util.StringUtils;
 
 /**
- * Convenience factory for XML configuration of a {@link CorrelationStrategy}. Encapsulates the knowledge of the default
- * strategy and search algorithms for POJO and annotated methods.
+ * Convenience factory for XML configuration of a {@link CorrelationStrategy}.
+ * Encapsulates the knowledge of the default strategy and search algorithms for POJO and annotated methods.
  *
  * @author Dave Syer
+ * @author Artem Bilan
  *
  */
-public class CorrelationStrategyFactoryBean implements FactoryBean<CorrelationStrategy> {
+public class CorrelationStrategyFactoryBean implements FactoryBean<CorrelationStrategy>, InitializingBean {
 
-	private CorrelationStrategy delegate = new HeaderAttributeCorrelationStrategy(IntegrationMessageHeaderAccessor.CORRELATION_ID);
+	private Object target;
 
-	/**
-	 * Create a factory and set up the delegate which clients of the factory will see as its product.
-	 *
-	 * @param target the target object (null if default strategy is acceptable)
-	 */
-	public CorrelationStrategyFactoryBean(Object target) {
-		this(target, null);
+	private String methodName;
+
+	private CorrelationStrategy strategy =
+			new HeaderAttributeCorrelationStrategy(IntegrationMessageHeaderAccessor.CORRELATION_ID);
+
+	public CorrelationStrategyFactoryBean() {
 	}
 
 	/**
-	 * Create a factory and set up the delegate which clients of the factory will see as its product.
-	 *
+	 * Create a factory and set up the strategy which clients of the factory will see as its product.
+	 * @param target the target object (null if default strategy is acceptable)
+	 * @deprecated since {@literal 4.2.5} in favor of appropriate setters
+	 * to avoid {@code BeanCurrentlyInCreationException}
+	 * during {@code AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck()}
+	 */
+	@Deprecated
+	public CorrelationStrategyFactoryBean(Object target) {
+		this.target = target;
+	}
+
+	/**
+	 * Create a factory and set up the strategy which clients of the factory will see as its product.
 	 * @param target the target object (null if default strategy is acceptable)
 	 * @param methodName the method name to invoke in the target (null if it can be inferred)
+	 * @deprecated since {@literal 4.2.5} in favor of appropriate setters
+	 * to avoid {@code BeanCurrentlyInCreationException}
+	 * during {@code AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck()}
 	 */
+	@Deprecated
 	public CorrelationStrategyFactoryBean(Object target, String methodName) {
-		if (target instanceof CorrelationStrategy && !StringUtils.hasText(methodName)) {
-			delegate = (CorrelationStrategy) target;
+		this.target = target;
+		this.methodName = methodName;
+	}
+
+	public void setTarget(Object target) {
+		this.target = target;
+	}
+
+	public void setMethodName(String methodName) {
+		this.methodName = methodName;
+	}
+
+	@Override
+	public void afterPropertiesSet() throws Exception {
+		if (this.target instanceof CorrelationStrategy && !StringUtils.hasText(this.methodName)) {
+			this.strategy = (CorrelationStrategy) this.target;
 			return;
 		}
-		if (target != null) {
-			if (StringUtils.hasText(methodName)) {
-				delegate = new MethodInvokingCorrelationStrategy(target, methodName);
+		if (this.target != null) {
+			if (StringUtils.hasText(this.methodName)) {
+				this.strategy = new MethodInvokingCorrelationStrategy(this.target, this.methodName);
 			}
 			else {
-				Method method = MessagingAnnotationUtils.findAnnotatedMethod(target,
+				Method method = MessagingAnnotationUtils.findAnnotatedMethod(this.target,
 						org.springframework.integration.annotation.CorrelationStrategy.class);
 				if (method != null) {
-					delegate = new MethodInvokingCorrelationStrategy(target, method);
+					this.strategy = new MethodInvokingCorrelationStrategy(this.target, method);
 				}
 			}
 		}
 	}
 
 	public CorrelationStrategy getObject() throws Exception {
-		return delegate;
+		return this.strategy;
 	}
 
 	public Class<?> getObjectType() {

spring-integration-core/src/main/java/org/springframework/integration/config/ReleaseStrategyFactoryBean.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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.
@@ -13,6 +13,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.springframework.integration.config;
 
 import java.lang.reflect.Method;
@@ -21,6 +22,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.beans.factory.FactoryBean;
+import org.springframework.beans.factory.InitializingBean;
 import org.springframework.integration.aggregator.MethodInvokingReleaseStrategy;
 import org.springframework.integration.aggregator.ReleaseStrategy;
 import org.springframework.integration.aggregator.SequenceSizeReleaseStrategy;
@@ -28,55 +30,83 @@ import org.springframework.integration.util.MessagingAnnotationUtils;
 import org.springframework.util.StringUtils;
 
 /**
- * Convenience factory for XML configuration of a {@link ReleaseStrategy}. Encapsulates the knowledge of the default
- * strategy and search algorithms for POJO and annotated methods.
+ * Convenience factory for XML configuration of a {@link ReleaseStrategy}.
+ * Encapsulates the knowledge of the default strategy and search algorithms for POJO and annotated methods.
  *
  * @author Dave Syer
  * @author Gary Russell
+ * @author Artem Bilan
  *
  */
-public class ReleaseStrategyFactoryBean implements FactoryBean<ReleaseStrategy> {
+public class ReleaseStrategyFactoryBean implements FactoryBean<ReleaseStrategy>, InitializingBean {
 
 	private static final Log logger = LogFactory.getLog(ReleaseStrategyFactoryBean.class);
 
-	private ReleaseStrategy delegate = new SequenceSizeReleaseStrategy();
+	private Object target;
 
-	/**
-	 * Create a factory and set up the delegate which clients of the factory will see as its product.
-	 *
-	 * @param target the target object (null if default strategy is acceptable)
-	 */
-	public ReleaseStrategyFactoryBean(Object target) {
-		this(target, null);
+	private String methodName;
+
+	private ReleaseStrategy strategy = new SequenceSizeReleaseStrategy();
+
+	public ReleaseStrategyFactoryBean() {
 	}
 
 	/**
-	 * Create a factory and set up the delegate which clients of the factory will see as its product.
-	 *
+	 * Create a factory and set up the strategy which clients of the factory will see as its product.
+	 * @param target the target object (null if default strategy is acceptable)
+	 * @deprecated since {@literal 4.2.5} in favor of appropriate setters
+	 * to avoid {@code BeanCurrentlyInCreationException}
+	 * during {@code AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck()}
+	 */
+	@Deprecated
+	public ReleaseStrategyFactoryBean(Object target) {
+		this.target = target;
+	}
+
+	/**
+	 * Create a factory and set up the strategy which clients of the factory will see as its product.
 	 * @param target the target object (null if default strategy is acceptable)
 	 * @param methodName the method name to invoke in the target (null if it can be inferred)
+	 * @deprecated since {@literal 4.2.5} in favor of appropriate setters
+	 * to avoid {@code BeanCurrentlyInCreationException}
+	 * during {@code AbstractAutowireCapableBeanFactory.getSingletonFactoryBeanForTypeCheck()}
 	 */
+	@Deprecated
 	public ReleaseStrategyFactoryBean(Object target, String methodName) {
-		if (target instanceof ReleaseStrategy && !StringUtils.hasText(methodName)) {
-			this.delegate = (ReleaseStrategy) target;
+		this.target = target;
+		this.methodName = methodName;
+	}
+
+	public void setTarget(Object target) {
+		this.target = target;
+	}
+
+	public void setMethodName(String methodName) {
+		this.methodName = methodName;
+	}
+
+	@Override
+	public void afterPropertiesSet() throws Exception {
+		if (this.target instanceof ReleaseStrategy && !StringUtils.hasText(this.methodName)) {
+			this.strategy = (ReleaseStrategy) this.target;
 			return;
 		}
-		if (target != null) {
-			if (StringUtils.hasText(methodName)) {
-				this.delegate = new MethodInvokingReleaseStrategy(target, methodName);
+		if (this.target != null) {
+			if (StringUtils.hasText(this.methodName)) {
+				this.strategy = new MethodInvokingReleaseStrategy(this.target, this.methodName);
 			}
 			else {
-				Method method = MessagingAnnotationUtils.findAnnotatedMethod(target,
+				Method method = MessagingAnnotationUtils.findAnnotatedMethod(this.target,
 						org.springframework.integration.annotation.ReleaseStrategy.class);
 				if (method != null) {
-					this.delegate = new MethodInvokingReleaseStrategy(target, method);
+					this.strategy = new MethodInvokingReleaseStrategy(this.target, method);
 				}
 				else {
 					if (logger.isWarnEnabled()) {
 						logger.warn("No ReleaseStrategy annotated method found on "
-								+ target.getClass().getSimpleName()
+								+ this.target.getClass().getSimpleName()
 								+ "; falling back to SequenceSizeReleaseStrategy, target:"
-								+ target + ", methodName:" + methodName);
+								+ this.target + ", methodName:" + this.methodName);
 					}
 				}
 			}
@@ -90,7 +120,7 @@ public class ReleaseStrategyFactoryBean implements FactoryBean<ReleaseStrategy>
 
 	@Override
 	public ReleaseStrategy getObject() throws Exception {
-		return this.delegate;
+		return this.strategy;
 	}
 
 	@Override

spring-integration-core/src/main/java/org/springframework/integration/config/xml/IntegrationNamespaceUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2015 the original author or authors.
+ * Copyright 2002-2016 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
@@ -266,15 +266,16 @@ public abstract class IntegrationNamespaceUtils {
 	}
 
 	/**
-	 * Get a text value from a named attribute if it exists, otherwise check for a nested element of the same name. If
-	 * both are specified it is an error, but if neither is specified, just returns null.
+	 * Get a text value from a named attribute if it exists, otherwise check for a nested element of the same name.
+	 * If both are specified it is an error, but if neither is specified, just returns null.
 	 *
 	 * @param element a DOM node
 	 * @param name the name of the property (attribute or child element)
 	 * @param parserContext the current context
-	 * @return the text from the attribite or element or null
+	 * @return the text from the attribute or element or null
 	 */
-	public static String getTextFromAttributeOrNestedElement(Element element, String name, ParserContext parserContext) {
+	public static String getTextFromAttributeOrNestedElement(Element element, String name,
+	                                                         ParserContext parserContext) {
 		String attr = element.getAttribute(name);
 		Element childElement = DomUtils.getChildElementByTagName(element, name);
 		if (StringUtils.hasText(attr) && childElement != null) {
@@ -305,7 +306,8 @@ public abstract class IntegrationNamespaceUtils {
 			parserContext.getReaderContext().error(
 				"Ambiguous definition. Inner bean " + (innerComponentDefinition.getBeanDefinition().getBeanClassName())
 						+ " declaration and \"ref\" " + ref + " are not allowed together on element " +
-					IntegrationNamespaceUtils.createElementDescription(element) + ".", parserContext.extractSource(element));
+					IntegrationNamespaceUtils.createElementDescription(element) + ".",
+					parserContext.extractSource(element));
 		}
 		return innerComponentDefinition;
 	}
@@ -616,13 +618,14 @@ public abstract class IntegrationNamespaceUtils {
 		return adapter;
 	}
 
-	private static BeanMetadataElement createAdapter(BeanMetadataElement ref, String method, String unqualifiedClassName) {
+	private static BeanMetadataElement createAdapter(BeanMetadataElement ref, String method,
+	                                                 String unqualifiedClassName) {
 		BeanDefinitionBuilder builder = BeanDefinitionBuilder
 				.genericBeanDefinition(IntegrationConfigUtils.BASE_PACKAGE + ".config." + unqualifiedClassName
 						+ "FactoryBean");
-		builder.addConstructorArgValue(ref);
+		builder.addPropertyValue("target", ref);
 		if (StringUtils.hasText(method)) {
-			builder.addConstructorArgValue(method);
+			builder.addPropertyValue("methodName", method);
 		}
 		return builder.getBeanDefinition();
 	}

spring-integration-core/src/test/java/org/springframework/integration/config/ReleaseStrategyFactoryBeanTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2014 the original author or authors.
+ * Copyright 2014-2016 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.
@@ -15,10 +15,12 @@
  */
 package org.springframework.integration.config;
 
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
 
 import java.util.Collection;
 
@@ -32,34 +34,49 @@ import org.springframework.integration.test.util.TestUtils;
 
 /**
  * @author Gary Russell
+ * @author Artem Bilan
  * @since 3.0.2
- *
  */
 public class ReleaseStrategyFactoryBeanTests {
 
-	public void testRefWithMethod() throws Exception {
+	@Test
+	public void testRefWithNoMethod() throws Exception {
 		Foo foo = new Foo();
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(foo, "doRelease");
-		ReleaseStrategy delegate = factory.getObject();
-		assertThat(delegate, instanceOf(MethodInvokingReleaseStrategy.class));
-		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.targetObject", Foo.class), is(foo));
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.setTarget(foo);
+		factory.setMethodName("doRelease");
+		try {
+			factory.afterPropertiesSet();
+			fail("IllegalArgumentException expected");
+		}
+		catch (Exception e) {
+			assertThat(e, instanceOf(IllegalArgumentException.class));
+			assertThat(e.getMessage(), containsString("Target object of type " +
+							"[class org.springframework.integration.config.ReleaseStrategyFactoryBeanTests$Foo] " +
+					"has no eligible methods for handling Messages."));
+		}
 	}
 
 	@Test
 	public void testRefWithMethodWithDifferentAnnotatedMethod() throws Exception {
 		Bar bar = new Bar();
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(bar, "doRelease2");
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.setTarget(bar);
+		factory.setMethodName("doRelease2");
+		factory.afterPropertiesSet();
 		ReleaseStrategy delegate = factory.getObject();
 		assertThat(delegate, instanceOf(MethodInvokingReleaseStrategy.class));
 		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.targetObject", Bar.class), is(bar));
-		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.handlerMethod.expression.expression", String.class),
+		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.handlerMethod.expression.expression"),
 				equalTo("#target.doRelease2(messages)"));
 	}
 
 	@Test
 	public void testRefWithNoMethodWithAnnotation() throws Exception {
 		Bar bar = new Bar();
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(bar);
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.setTarget(bar);
+		factory.afterPropertiesSet();
 		ReleaseStrategy delegate = factory.getObject();
 		assertThat(delegate, instanceOf(MethodInvokingReleaseStrategy.class));
 		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.targetObject", Bar.class), is(bar));
@@ -67,7 +84,8 @@ public class ReleaseStrategyFactoryBeanTests {
 
 	@Test
 	public void testNoRefNoMethod() throws Exception {
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(null);
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.afterPropertiesSet();
 		ReleaseStrategy delegate = factory.getObject();
 		assertThat(delegate, instanceOf(SequenceSizeReleaseStrategy.class));
 	}
@@ -75,7 +93,9 @@ public class ReleaseStrategyFactoryBeanTests {
 	@Test
 	public void testRefWithNoMethodNoAnnotation() throws Exception {
 		Foo foo = new Foo();
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(foo);
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.setTarget(foo);
+		factory.afterPropertiesSet();
 		ReleaseStrategy delegate = factory.getObject();
 		assertThat(delegate, instanceOf(SequenceSizeReleaseStrategy.class));
 	}
@@ -83,19 +103,24 @@ public class ReleaseStrategyFactoryBeanTests {
 	@Test
 	public void testRefThatImplements() throws Exception {
 		Baz baz = new Baz();
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(baz);
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.setTarget(baz);
+		factory.afterPropertiesSet();
 		ReleaseStrategy delegate = factory.getObject();
-		assertThat((Baz) delegate, is(baz));
+		assertThat(delegate, is(baz));
 	}
 
 	@Test
 	public void testRefThatImplementsWithDifferentMethod() throws Exception {
 		Baz baz = new Baz();
-		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean(baz, "doRelease2");
+		ReleaseStrategyFactoryBean factory = new ReleaseStrategyFactoryBean();
+		factory.setTarget(baz);
+		factory.setMethodName("doRelease2");
+		factory.afterPropertiesSet();
 		ReleaseStrategy delegate = factory.getObject();
 		assertThat(delegate, instanceOf(MethodInvokingReleaseStrategy.class));
 		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.targetObject", Baz.class), is(baz));
-		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.handlerMethod.expression.expression", String.class),
+		assertThat(TestUtils.getPropertyValue(delegate, "adapter.delegate.handlerMethod.expression.expression"),
 				equalTo("#target.doRelease2(messages)"));
 	}