INT-2549: Ignore MBean call reply in op-invoc-c-a

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

* Add `expectReply` property into the `OperationInvokingMessageHandler`
to align the one-way and request-reply behavior with all other similar
components in Spring Integration
* Ignore an operation invocation result in case of `expectReply == false`
and log warning
* Provide some refactoring into the `OperationInvokingMessageHandler`
to fix Sonar complains about complexity

**Cherry-pick to 5.0.x, 4.3.x**

# Conflicts:
#	spring-integration-jmx/src/main/java/org/springframework/integration/jmx/OperationInvokingMessageHandler.java
#	spring-integration-jmx/src/test/java/org/springframework/integration/jmx/OperationInvokingMessageHandlerTests.java

Author: artembilan

spring-integration-jmx/src/main/java/org/springframework/integration/jmx/OperationInvokingMessageHandler.java

@@ -59,36 +59,62 @@
  * @author Mark Fisher
  * @author Oleg Zhurakousky
  * @author Gary Russell
+ * @author Artem Bilan
+ *
  * @since 2.0
  */
 public class OperationInvokingMessageHandler extends AbstractReplyProducingMessageHandler implements InitializingBean {
 
-	private volatile MBeanServerConnection server;
+	private MBeanServerConnection server;
+
+	private ObjectName defaultObjectName;
+
+	private String operationName;
 
-	private volatile ObjectName objectName;
+	private boolean expectReply = true;
+
+	/**
+	 * Construct an instance with no arguments; for backward compatibility.
+	 * The {@link #setServer(MBeanServerConnection)} must be used as well.
+	 * The {@link #OperationInvokingMessageHandler(MBeanServerConnection)}
+	 * is a preferred way for instantiation.
+	 * @since 4.3.20
+	 * @deprecated since 4.3.20
+	 */
+	@Deprecated
+	public OperationInvokingMessageHandler() {
+	}
 
-	private volatile String operationName;
+	/**
+	 * Construct an instance based on the provided {@link MBeanServerConnection}.
+	 * @param server the {@link MBeanServerConnection} to use.
+	 * @since 4.3.20
+	 */
+	public OperationInvokingMessageHandler(MBeanServerConnection server) {
+		Assert.notNull(server, "MBeanServer is required.");
+		this.server = server;
+	}
 
 	/**
 	 * Provide a reference to the MBeanServer within which the MBean
 	 * target for operation invocation has been registered.
-	 *
 	 * @param server The MBean server connection.
+	 * @deprecated since 4.3.20 in favor of {@link #OperationInvokingMessageHandler(MBeanServerConnection)}
 	 */
+	@Deprecated
 	public void setServer(MBeanServerConnection server) {
 		this.server = server;
 	}
 
 	/**
 	 * Specify a default ObjectName to use when no such header is
 	 * available on the Message being handled.
-	 *
 	 * @param objectName The object name.
 	 */
 	public void setObjectName(String objectName) {
 		try {
 			if (objectName != null) {
-				this.objectName = ObjectNameManager.getInstance(objectName);
+				this.defaultObjectName = ObjectNameManager.getInstance(objectName);
 			}
 		}
 		catch (MalformedObjectNameException e) {
@@ -99,16 +125,25 @@ public void setObjectName(String objectName) {
 	/**
 	 * Specify an operation name to be invoked when no such
 	 * header is available on the Message being handled.
-	 *
 	 * @param operationName The operation name.
 	 */
 	public void setOperationName(String operationName) {
 		this.operationName = operationName;
 	}
 
+	/**
+	 * Specify whether a reply Message is expected. If not, this handler will simply return null for a
+	 * successful response or throw an Exception for a non-successful response. The default is true.
+	 * @param expectReply true if a reply is expected.
+	 * @since 4.3.20
+	 */
+	public void setExpectReply(boolean expectReply) {
+		this.expectReply = expectReply;
+	}
+
 	@Override
 	public String getComponentType() {
-		return "jmx:operation-invoking-channel-adapter";
+		return this.expectReply ? "jmx:operation-invoking-outbound-gateway" : "jmx:operation-invoking-channel-adapter";
 	}
 
 	@Override
@@ -118,51 +153,20 @@ protected void doInit() {
 
 	@Override
 	protected Object handleRequestMessage(Message<?> requestMessage) {
-		ObjectName objectName = this.resolveObjectName(requestMessage);
-		String operationName = this.resolveOperationName(requestMessage);
-		Map<String, Object> paramsFromMessage = this.resolveParameters(requestMessage);
+		ObjectName objectName = resolveObjectName(requestMessage);
+		String operationName = resolveOperationName(requestMessage);
+		Map<String, Object> paramsFromMessage = resolveParameters(requestMessage);
 		try {
-			MBeanInfo mbeanInfo = this.server.getMBeanInfo(objectName);
-			MBeanOperationInfo[] opInfoArray = mbeanInfo.getOperations();
-			boolean hasNoArgOption = false;
-			for (MBeanOperationInfo opInfo : opInfoArray) {
-				if (operationName.equals(opInfo.getName())) {
-					MBeanParameterInfo[] paramInfoArray = opInfo.getSignature();
-					if (paramInfoArray.length == 0) {
-						hasNoArgOption = true;
-					}
-					if (paramInfoArray.length == paramsFromMessage.size()) {
-						int index = 0;
-						Object[] values = new Object[paramInfoArray.length];
-						String[] signature = new String[paramInfoArray.length];
-						for (MBeanParameterInfo paramInfo : paramInfoArray) {
-							Object value = paramsFromMessage.get(paramInfo.getName());
-							if (value == null) {
-								/*
-								 * With Spring 3.2.3 and greater, the parameter names are
-								 * registered instead of the JVM's default p1, p2 etc.
-								 * Fall back to that naming style if not found.
-								 */
-								value = paramsFromMessage.get("p" + (index + 1));
-							}
-							if (value != null && valueTypeMatchesParameterType(value, paramInfo)) {
-								values[index] = value;
-								signature[index] = paramInfo.getType();
-								index++;
-							}
-						}
-						if (index == paramInfoArray.length) {
-							return this.server.invoke(objectName, operationName, values, signature);
-						}
-					}
+			Object result = invokeOperation(requestMessage, objectName, operationName, paramsFromMessage);
+			if (!this.expectReply && result != null) {
+				if (logger.isWarnEnabled()) {
+					logger.warn("This component doesn't expect a reply. " +
+							"The MBean operation '" + operationName + "' result '" + result +
+							"' for '" + objectName + "' is ignored.");
 				}
+				return null;
 			}
-			if (hasNoArgOption) {
-				return this.server.invoke(objectName, operationName, null, null);
-			}
-			throw new MessagingException(requestMessage, "failed to find JMX operation '"
-					+ operationName + "' on MBean [" + objectName + "] of type [" + mbeanInfo.getClassName()
-					+ "] with " + paramsFromMessage.size() + " parameters: " + paramsFromMessage);
+			return result;
 		}
 		catch (JMException e) {
 			throw new MessageHandlingException(requestMessage, "failed to invoke JMX operation '" +
@@ -174,8 +178,56 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
 		}
 	}
 
+	private Object invokeOperation(Message<?> requestMessage, ObjectName objectName, String operation,
+			Map<String, Object> paramsFromMessage) throws JMException, IOException {
+
+		MBeanInfo mbeanInfo = this.server.getMBeanInfo(objectName);
+		MBeanOperationInfo[] opInfoArray = mbeanInfo.getOperations();
+		boolean hasNoArgOption = false;
+		for (MBeanOperationInfo opInfo : opInfoArray) {
+			if (operation.equals(opInfo.getName())) {
+				MBeanParameterInfo[] paramInfoArray = opInfo.getSignature();
+				if (paramInfoArray.length == 0) {
+					hasNoArgOption = true;
+				}
+				if (paramInfoArray.length == paramsFromMessage.size()) {
+					int index = 0;
+					Object[] values = new Object[paramInfoArray.length];
+					String[] signature = new String[paramInfoArray.length];
+					for (MBeanParameterInfo paramInfo : paramInfoArray) {
+						Object value = paramsFromMessage.get(paramInfo.getName());
+						if (value == null) {
+							/*
+							 * With Spring 3.2.3 and greater, the parameter names are
+							 * registered instead of the JVM's default p1, p2 etc.
+							 * Fall back to that naming style if not found.
+							 */
+							value = paramsFromMessage.get("p" + (index + 1));
+						}
+						if (value != null && valueTypeMatchesParameterType(value, paramInfo)) {
+							values[index] = value;
+							signature[index] = paramInfo.getType();
+							index++;
+						}
+					}
+					if (index == paramInfoArray.length) {
+						return this.server.invoke(objectName, operation, values, signature);
+					}
+				}
+			}
+		}
+		if (hasNoArgOption) {
+			return this.server.invoke(objectName, operation, null, null);
+		}
+		else {
+			throw new MessagingException(requestMessage, "failed to find JMX operation '"
+					+ operation + "' on MBean [" + objectName + "] of type [" + mbeanInfo.getClassName()
+					+ "] with " + paramsFromMessage.size() + " parameters: " + paramsFromMessage);
+		}
+	}
+
 	private boolean valueTypeMatchesParameterType(Object value, MBeanParameterInfo paramInfo) {
-		Class<? extends Object> valueClass = value.getClass();
+		Class<?> valueClass = value.getClass();
 		if (valueClass.getName().equals(paramInfo.getType())) {
 			return true;
 		}
@@ -189,7 +241,7 @@ private boolean valueTypeMatchesParameterType(Object value, MBeanParameterInfo p
 	 * First checks if defaultObjectName is set, otherwise falls back on  {@link JmxHeaders#OBJECT_NAME} header.
 	 */
 	private ObjectName resolveObjectName(Message<?> message) {
-		ObjectName objectName = this.objectName;
+		ObjectName objectName = this.defaultObjectName;
 		if (objectName == null) {
 			Object objectNameHeader = message.getHeaders().get(JmxHeaders.OBJECT_NAME);
 			if (objectNameHeader instanceof ObjectName) {
@@ -209,7 +261,7 @@ else if (objectNameHeader instanceof String) {
 	}
 
 	/**
-	  * First checks if defaultOperationName is set, otherwise falls back on  {@link JmxHeaders#OPERATION_NAME} header.
+	 * First checks if defaultOperationName is set, otherwise falls back on  {@link JmxHeaders#OPERATION_NAME} header.
 	 */
 	private String resolveOperationName(Message<?> message) {
 		String operationName = this.operationName;
@@ -220,31 +272,27 @@ private String resolveOperationName(Message<?> message) {
 		return operationName;
 	}
 
-	@SuppressWarnings({ "unchecked", "rawtypes" })
+	@SuppressWarnings("unchecked")
 	private Map<String, Object> resolveParameters(Message<?> message) {
 		Map<String, Object> map = null;
-		if (message.getPayload() instanceof Map) {
-			map = (Map<String, Object>) message.getPayload();
-		}
-		else if (message.getPayload() instanceof List) {
-			map = this.createParameterMapFromList((List) message.getPayload());
+		Object payload = message.getPayload();
+		if (payload instanceof Map) {
+			map = (Map<String, Object>) payload;
 		}
-		else if (message.getPayload() != null && message.getPayload().getClass().isArray()) {
-			map = this.createParameterMapFromList(
-					Arrays.asList(ObjectUtils.toObjectArray(message.getPayload())));
+		else if (payload instanceof List) {
+			map = createParameterMapFromList((List<?>) payload);
 		}
-		else if (message.getPayload() != null) {
-			map = this.createParameterMapFromList(Collections.singletonList(message.getPayload()));
+		else if (payload.getClass().isArray()) {
+			map = createParameterMapFromList(Arrays.asList(ObjectUtils.toObjectArray(payload)));
 		}
 		else {
-			map = Collections.EMPTY_MAP;
+			map = createParameterMapFromList(Collections.singletonList(payload));
 		}
 		return map;
 	}
 
-	@SuppressWarnings("rawtypes")
-	private Map<String, Object> createParameterMapFromList(List parameters) {
-		Map<String, Object> map = new HashMap<String, Object>();
+	private Map<String, Object> createParameterMapFromList(List<?> parameters) {
+		Map<String, Object> map = new HashMap<>();
 		for (int i = 0; i < parameters.size(); i++) {
 			map.put("p" + (i + 1), parameters.get(i));
 		}

spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -28,6 +28,8 @@
 /**
  * @author Mark Fisher
  * @author Gary Russell
+ * @author Artem Bilan
+ *
  * @since 2.0
  */
 public class OperationInvokingChannelAdapterParser extends AbstractOutboundChannelAdapterParser {
@@ -40,7 +42,8 @@ protected boolean shouldGenerateIdAsFallback() {
 	@Override
 	protected AbstractBeanDefinition parseConsumer(Element element, ParserContext parserContext) {
 		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(OperationInvokingMessageHandler.class);
-		IntegrationNamespaceUtils.setReferenceIfAttributeDefined(builder, element, "server");
+		builder.addConstructorArgReference(element.getAttribute("server"));
+		builder.addPropertyValue("expectReply", false);
 		IntegrationNamespaceUtils.setValueIfAttributeDefined(builder, element, "object-name");
 		IntegrationNamespaceUtils.setValueIfAttributeDefined(builder, element, "operation-name");
 		return builder.getBeanDefinition();

spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingOutboundGatewayParser.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -27,6 +27,7 @@
 /**
  * @author Oleg Zhurakousky
  * @author Artem Bilan
+ *
  * @since 2.0
  */
 public class OperationInvokingOutboundGatewayParser extends AbstractConsumerEndpointParser {
@@ -39,7 +40,7 @@ protected String getInputChannelAttributeName() {
 	@Override
 	protected BeanDefinitionBuilder parseHandler(Element element, ParserContext parserContext) {
 		BeanDefinitionBuilder builder = BeanDefinitionBuilder.rootBeanDefinition(OperationInvokingMessageHandler.class);
-		IntegrationNamespaceUtils.setReferenceIfAttributeDefined(builder, element, "server");
+		builder.addConstructorArgReference(element.getAttribute("server"));
 		IntegrationNamespaceUtils.setValueIfAttributeDefined(builder, element, "object-name");
 		IntegrationNamespaceUtils.setValueIfAttributeDefined(builder, element, "operation-name");
 		IntegrationNamespaceUtils.setReferenceIfAttributeDefined(builder, element, "reply-channel", "outputChannel");

spring-integration-jmx/src/test/java/org/springframework/integration/jmx/OperationInvokingMessageHandlerTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2019 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.
@@ -73,8 +73,7 @@ public void cleanup() throws Exception {
 	@Test
 	public void invocationWithMapPayload() throws Exception {
 		QueueChannel outputChannel = new QueueChannel();
-		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler();
-		handler.setServer(this.server);
+		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server);
 		handler.setObjectName(this.objectName);
 		handler.setOutputChannel(outputChannel);
 		handler.setOperationName("x");
@@ -93,8 +92,7 @@ public void invocationWithMapPayload() throws Exception {
 	@Test
 	public void invocationWithPayloadNoReturnValue() throws Exception {
 		QueueChannel outputChannel = new QueueChannel();
-		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler();
-		handler.setServer(this.server);
+		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server);
 		handler.setObjectName(this.objectName);
 		handler.setOutputChannel(outputChannel);
 		handler.setOperationName("y");
@@ -107,8 +105,7 @@ public void invocationWithPayloadNoReturnValue() throws Exception {
 	@Test(expected = MessagingException.class)
 	public void invocationWithMapPayloadNotEnoughParameters() throws Exception {
 		QueueChannel outputChannel = new QueueChannel();
-		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler();
-		handler.setServer(this.server);
+		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server);
 		handler.setObjectName(this.objectName);
 		handler.setOutputChannel(outputChannel);
 		handler.setOperationName("x");
@@ -126,8 +123,7 @@ public void invocationWithMapPayloadNotEnoughParameters() throws Exception {
 	@Test
 	public void invocationWithListPayload() throws Exception {
 		QueueChannel outputChannel = new QueueChannel();
-		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler();
-		handler.setServer(this.server);
+		OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server);
 		handler.setObjectName(this.objectName);
 		handler.setOutputChannel(outputChannel);
 		handler.setOperationName("x");

spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests-context.xml

@@ -20,7 +20,7 @@
 			object-name="org.springframework.integration.jmx.config:type=TestBean,name=testBeanAdapter"
 			operation-name="test">
 		<jmx:request-handler-advice-chain>
-			<bean class="org.springframework.integration.jmx.config.OperationInvokingChannelAdapterParserTests$FooADvice" />
+			<bean class="org.springframework.integration.jmx.config.OperationInvokingChannelAdapterParserTests.FooAdvice" />
 		</jmx:request-handler-advice-chain>
 	</jmx:operation-invoking-channel-adapter>
 
@@ -36,8 +36,8 @@
 				operation-name="test"/>
 	</si:chain>
 
-	<si:chain input-channel="operationWithinChainWithNonNullReturn">
-		<jmx:operation-invoking-channel-adapter
+	<si:chain id="chainWithOperation" input-channel="operationWithinChainWithNonNullReturn">
+		<jmx:operation-invoking-channel-adapter id="operationWithinChainWithNonNullReturnHandler"
 				object-name="org.springframework.integration.jmx.config:type=TestBean,name=testBeanAdapter"
 				operation-name="testWithReturn"/>
 	</si:chain>