INT-4573: Fix logic in the OperationInvokingMH

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

* Fix check order for headers first in the `resolveObjectName()` and
`resolveOperationName()`.
Then fallback to defaults configured on the `OperationInvokingMessageHandler`
* Remove also a deprecated API in the `OperationInvokingMessageHandler`
* Remove requirements for `object-name` and `operation-name` XML
attributes since those options can be provided in the request message
headers
* Fix `OperationInvokingChannelAdapterParserTests` for proper headers
resolution
* Mention `JmxHeaders.OBJECT_NAME` and `JmxHeaders.OPERATION_NAME`
headers in the `jmx.adoc`

Author: artembilan

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

@@ -65,25 +65,14 @@
  */
 public class OperationInvokingMessageHandler extends AbstractReplyProducingMessageHandler implements InitializingBean {
 
-	private MBeanServerConnection server;
+	private final MBeanServerConnection server;
 
 	private ObjectName defaultObjectName;
 
 	private String operationName;
 
 	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() {
-	}
 
 	/**
 	 * Construct an instance based on the provided {@link MBeanServerConnection}.
@@ -95,17 +84,6 @@ public OperationInvokingMessageHandler(MBeanServerConnection server) {
 		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.
@@ -146,11 +124,6 @@ public String getComponentType() {
 		return this.expectReply ? "jmx:operation-invoking-outbound-gateway" : "jmx:operation-invoking-channel-adapter";
 	}
 
-	@Override
-	protected void doInit() {
-		Assert.notNull(this.server, "MBeanServer is required.");
-	}
-
 	@Override
 	protected Object handleRequestMessage(Message<?> requestMessage) {
 		ObjectName objectName = resolveObjectName(requestMessage);
@@ -245,35 +218,36 @@ private boolean valueTypeMatchesParameterType(Object value, MBeanParameterInfo p
 	}
 
 	/**
-	 * First checks if defaultObjectName is set, otherwise falls back on  {@link JmxHeaders#OBJECT_NAME} header.
+	 * First checks {@link JmxHeaders#OBJECT_NAME} header in the request message,
+	 * otherwise falls back to {@link #defaultObjectName}.
 	 */
 	private ObjectName resolveObjectName(Message<?> message) {
-		ObjectName objectName = this.defaultObjectName;
-		if (objectName == null) {
-			Object objectNameHeader = message.getHeaders().get(JmxHeaders.OBJECT_NAME);
-			if (objectNameHeader instanceof ObjectName) {
-				objectName = (ObjectName) objectNameHeader;
+		ObjectName objectName;
+		Object objectNameHeader = message.getHeaders().get(JmxHeaders.OBJECT_NAME);
+		if (objectNameHeader != null) {
+			try {
+				objectName = ObjectNameManager.getInstance(objectNameHeader);
 			}
-			else if (objectNameHeader instanceof String) {
-				try {
-					objectName = ObjectNameManager.getInstance(objectNameHeader);
-				}
-				catch (MalformedObjectNameException e) {
-					throw new IllegalArgumentException(e);
-				}
+			catch (MalformedObjectNameException e) {
+				throw new IllegalArgumentException(e);
 			}
 		}
-		Assert.notNull(objectName, "Failed to resolve ObjectName.");
+		else {
+			objectName = this.defaultObjectName;
+
+		}
+		Assert.notNull(objectName, "Failed to resolve ObjectName: either from headers or 'defaultObjectName'.");
 		return objectName;
 	}
 
 	/**
-	 * First checks if defaultOperationName is set, otherwise falls back on  {@link JmxHeaders#OPERATION_NAME} header.
+	 * First checks if {@link JmxHeaders#OPERATION_NAME} header,
+	 * otherwise falls back to {@link #operationName}.
 	 */
 	private String resolveOperationName(Message<?> message) {
-		String operation = this.operationName;
+		String operation = message.getHeaders().get(JmxHeaders.OPERATION_NAME, String.class);
 		if (operation == null) {
-			operation = message.getHeaders().get(JmxHeaders.OPERATION_NAME, String.class);
+			operation = this.operationName;
 		}
 		Assert.notNull(operation, "Failed to resolve operation name.");
 		return operation;

spring-integration-jmx/src/main/resources/org/springframework/integration/jmx/config/spring-integration-jmx-5.2.xsd

@@ -318,8 +318,8 @@
 		</xsd:annotation>
 		<xsd:complexContent>
 			<xsd:extension base="mbeanServerIdentifierType">
-				<xsd:attribute name="object-name" type="xsd:string" use="required" />
-				<xsd:attribute name="operation-name" type="xsd:string" use="required" />
+				<xsd:attribute name="object-name" type="xsd:string" />
+				<xsd:attribute name="operation-name" type="xsd:string" />
 			</xsd:extension>
 		</xsd:complexContent>
 	</xsd:complexType>

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

@@ -43,4 +43,6 @@
 				operation-name="testWithReturn"/>
 	</si:chain>
 
+	<jmx:operation-invoking-channel-adapter id="input2"/>
+
 </beans>

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

@@ -27,7 +27,6 @@
 import org.junit.runner.RunWith;
 
 import org.springframework.beans.DirectFieldAccessor;
-import org.springframework.beans.factory.BeanFactory;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.beans.factory.annotation.Qualifier;
 import org.springframework.integration.handler.advice.AbstractRequestHandlerAdvice;
@@ -57,19 +56,19 @@ public class OperationInvokingChannelAdapterParserTests {
 	private MessageChannel input;
 
 	@Autowired
-	private MessageChannel operationWithNonNullReturn;
+	private MessageChannel input2;
 
 	@Autowired
-	private MessageChannel 	operationInvokingWithinChain;
+	private MessageChannel operationWithNonNullReturn;
 
 	@Autowired
-	private MessageChannel 	operationWithinChainWithNonNullReturn;
+	private MessageChannel operationInvokingWithinChain;
 
 	@Autowired
-	private TestBean testBean;
+	private MessageChannel operationWithinChainWithNonNullReturn;
 
 	@Autowired
-	private BeanFactory beanFactory;
+	private TestBean testBean;
 
 	@Autowired
 	@Qualifier("operationWithNonNullReturn.handler")
@@ -79,7 +78,7 @@ public class OperationInvokingChannelAdapterParserTests {
 	@Qualifier("chainWithOperation$child.operationWithinChainWithNonNullReturnHandler.handler")
 	private OperationInvokingMessageHandler operationWithinChainWithNonNullReturnHandler;
 
-	private static volatile int adviceCalled;
+	private static int adviceCalled;
 
 	@After
 	public void resetLists() {
@@ -119,9 +118,9 @@ public void testOutboundAdapterWithNonNullReturn() {
 	// Headers should be ignored
 	public void adapterWitJmxHeaders() {
 		assertThat(testBean.messages.size()).isEqualTo(0);
-		input.send(this.createMessage("1"));
-		input.send(this.createMessage("2"));
-		input.send(this.createMessage("3"));
+		this.input2.send(createMessage("1"));
+		this.input2.send(createMessage("2"));
+		this.input2.send(createMessage("3"));
 		assertThat(testBean.messages.size()).isEqualTo(3);
 	}
 
@@ -134,24 +133,27 @@ public void testInvokeOperationWithinChain() {
 	@Test
 	public void testOperationWithinChainWithNonNullReturn() {
 		Log logger =
-				spy(TestUtils.getPropertyValue(this.operationWithinChainWithNonNullReturnHandler, "logger", Log.class));
+				spy(TestUtils.getPropertyValue(this.operationWithinChainWithNonNullReturnHandler, "logger",
+						Log.class));
 
 		willReturn(true)
 				.given(logger)
 				.isWarnEnabled();
 
 		new DirectFieldAccessor(this.operationWithinChainWithNonNullReturnHandler)
 				.setPropertyValue("logger", logger);
-			this.operationWithinChainWithNonNullReturn.send(new GenericMessage<>("test1"));
+		this.operationWithinChainWithNonNullReturn.send(new GenericMessage<>("test1"));
 		verify(logger).warn("This component doesn't expect a reply. " +
 				"The MBean operation 'testWithReturn' result '[test1]' for " +
 				"'org.springframework.integration.jmx.config:type=TestBean,name=testBeanAdapter' is ignored.");
 	}
 
 	private Message<?> createMessage(String payload) {
 		return MessageBuilder.withPayload(payload)
-			.setHeader(JmxHeaders.OBJECT_NAME, "org.springframework.integration.jmx.config:type=TestBean,name=foo")
-			.setHeader(JmxHeaders.OPERATION_NAME, "blah").build();
+				.setHeader(JmxHeaders.OBJECT_NAME,
+						"org.springframework.integration.jmx.config:type=TestBean,name=testBeanAdapter")
+				.setHeader(JmxHeaders.OPERATION_NAME, "test")
+				.build();
 	}
 
 	public static class FooAdvice extends AbstractRequestHandlerAdvice {
@@ -163,4 +165,5 @@ protected Object doInvoke(ExecutionCallback callback, Object target, Message<?>
 		}
 
 	}
+
 }

src/reference/asciidoc/jmx.adoc

@@ -184,7 +184,7 @@ You can also implement your own filter.
 
 The operation-invoking channel adapter enables message-driven invocation of any managed operation exposed by an MBean.
 Each invocation requires the operation name to be invoked and the object name of the target MBean.
-Both of these must be explicitly provided by adapter configuration, as the following example shows:
+Both of these must be explicitly provided by adapter configuration or via `JmxHeaders.OBJECT_NAME` and `JmxHeaders.OPERATION_NAME` message headers, respectively:
 
 ====
 [source,xml]