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`
This commit is contained in:
Artem Bilan
2019-02-22 15:05:43 -05:00
committed by Gary Russell
parent 0ce6d1cef8
commit 4f34d922c1
5 changed files with 41 additions and 62 deletions

View File

@@ -65,7 +65,7 @@ import org.springframework.util.ObjectUtils;
*/
public class OperationInvokingMessageHandler extends AbstractReplyProducingMessageHandler implements InitializingBean {
private MBeanServerConnection server;
private final MBeanServerConnection server;
private ObjectName defaultObjectName;
@@ -73,17 +73,6 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
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 class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
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 class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
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 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
}
/**
* 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;

View File

@@ -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>

View File

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

View File

@@ -27,7 +27,6 @@ import org.junit.Test;
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;
@@ -56,21 +55,21 @@ public class OperationInvokingChannelAdapterParserTests {
@Autowired
private MessageChannel input;
@Autowired
private MessageChannel input2;
@Autowired
private MessageChannel operationWithNonNullReturn;
@Autowired
private MessageChannel operationInvokingWithinChain;
private MessageChannel operationInvokingWithinChain;
@Autowired
private MessageChannel operationWithinChainWithNonNullReturn;
private MessageChannel operationWithinChainWithNonNullReturn;
@Autowired
private TestBean testBean;
@Autowired
private BeanFactory beanFactory;
@Autowired
@Qualifier("operationWithNonNullReturn.handler")
private OperationInvokingMessageHandler operationWithNonNullReturnHandler;
@@ -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 class OperationInvokingChannelAdapterParserTests {
// 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,7 +133,8 @@ public class OperationInvokingChannelAdapterParserTests {
@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)
@@ -142,7 +142,7 @@ public class OperationInvokingChannelAdapterParserTests {
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.");
@@ -150,8 +150,10 @@ public class OperationInvokingChannelAdapterParserTests {
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 @@ public class OperationInvokingChannelAdapterParserTests {
}
}
}