Spring/spring-integration
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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
This commit is contained in:
Artem Bilan
2019-01-23 14:55:00 -05:00
parent b4a2de12aa
commit c723b69f01
6 changed files with 194 additions and 119 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -59,22 +59,49 @@ import org.springframework.util.ObjectUtils;
* @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 volatile ObjectName objectName;
private ObjectName defaultObjectName;
private volatile String operationName;
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}.
* @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;
}
@@ -82,13 +109,12 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
/**
* 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 class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
/**
* 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 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
@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 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
}
}
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 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
* 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 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
}
/**
* 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 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa
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();
Object payload = message.getPayload();
if (payload instanceof Map) {
map = (Map<String, Object>) payload;
}
else if (message.getPayload() instanceof List) {
map = this.createParameterMapFromList((List) message.getPayload());
else if (payload instanceof List) {
map = createParameterMapFromList((List<?>) payload);
}
else if (message.getPayload() != null && message.getPayload().getClass().isArray()) {
map = this.createParameterMapFromList(
Arrays.asList(ObjectUtils.toObjectArray(message.getPayload())));
}
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));
}

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

@@ -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 @@ import org.springframework.integration.jmx.OperationInvokingMessageHandler;
/**
* @author Mark Fisher
* @author Gary Russell
* @author Artem Bilan
*
* @since 2.0
*/
public class OperationInvokingChannelAdapterParser extends AbstractOutboundChannelAdapterParser {
@@ -40,7 +42,8 @@ public class OperationInvokingChannelAdapterParser extends AbstractOutboundChann
@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();

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

@@ -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 @@ import org.springframework.integration.jmx.OperationInvokingMessageHandler;
/**
* @author Oleg Zhurakousky
* @author Artem Bilan
*
* @since 2.0
*/
public class OperationInvokingOutboundGatewayParser extends AbstractConsumerEndpointParser {
@@ -39,7 +40,7 @@ public class OperationInvokingOutboundGatewayParser extends AbstractConsumerEndp
@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");

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

@@ -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 class OperationInvokingMessageHandlerTests {
@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 class OperationInvokingMessageHandlerTests {
@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 class OperationInvokingMessageHandlerTests {
@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 class OperationInvokingMessageHandlerTests {
@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");

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

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

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

@@ -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.
@@ -17,34 +17,39 @@
package org.springframework.integration.jmx.config;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.mockito.BDDMockito.willReturn;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
import org.apache.commons.logging.Log;
import org.junit.After;
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;
import org.springframework.integration.jmx.JmxHeaders;
import org.springframework.integration.jmx.OperationInvokingMessageHandler;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.integration.test.util.TestUtils;
import org.springframework.messaging.Message;
import org.springframework.messaging.MessageChannel;
import org.springframework.messaging.MessagingException;
import org.springframework.messaging.support.GenericMessage;
import org.springframework.test.annotation.DirtiesContext;
import org.springframework.test.context.ContextConfiguration;
import org.springframework.test.context.junit4.SpringJUnit4ClassRunner;
import org.springframework.test.context.junit4.SpringRunner;
/**
* @author Mark Fisher
* @author Oleg Zhurakousky
* @author Artem Bilan
* @author Gary Russell
*
* @since 2.0
*/
@ContextConfiguration
@RunWith(SpringJUnit4ClassRunner.class)
@RunWith(SpringRunner.class)
@DirtiesContext
public class OperationInvokingChannelAdapterParserTests {
@@ -63,6 +68,17 @@ public class OperationInvokingChannelAdapterParserTests {
@Autowired
private TestBean testBean;
@Autowired
private BeanFactory beanFactory;
@Autowired
@Qualifier("operationWithNonNullReturn.handler")
private OperationInvokingMessageHandler operationWithNonNullReturnHandler;
@Autowired
@Qualifier("chainWithOperation$child.operationWithinChainWithNonNullReturnHandler.handler")
private OperationInvokingMessageHandler operationWithinChainWithNonNullReturnHandler;
private static volatile int adviceCalled;
@After
@@ -72,30 +88,36 @@ public class OperationInvokingChannelAdapterParserTests {
@Test
public void adapterWithDefaults() throws Exception {
public void adapterWithDefaults() {
assertEquals(0, testBean.messages.size());
input.send(new GenericMessage<String>("test1"));
input.send(new GenericMessage<String>("test2"));
input.send(new GenericMessage<String>("test3"));
input.send(new GenericMessage<>("test1"));
input.send(new GenericMessage<>("test2"));
input.send(new GenericMessage<>("test3"));
assertEquals(3, testBean.messages.size());
assertEquals(3, adviceCalled);
}
@Test
public void testOutboundAdapterWithNonNullReturn() throws Exception {
try {
operationWithNonNullReturn.send(new GenericMessage<String>("test1"));
fail("Expect MessagingException about non-null return");
}
catch (Exception e) {
assertTrue(e instanceof MessagingException);
// TODO Add check exception's message about 'must have a void return' after <jmx:operation-invoking-channel-adapter/> refactoring
}
public void testOutboundAdapterWithNonNullReturn() {
Log logger = spy(TestUtils.getPropertyValue(this.operationWithNonNullReturnHandler, "logger", Log.class));
willReturn(true)
.given(logger)
.isWarnEnabled();
new DirectFieldAccessor(this.operationWithNonNullReturnHandler)
.setPropertyValue("logger", logger);
this.operationWithNonNullReturn.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.");
}
@Test
// Headers should be ignored
public void adapterWitJmxHeaders() throws Exception {
public void adapterWitJmxHeaders() {
assertEquals(0, testBean.messages.size());
input.send(this.createMessage("1"));
input.send(this.createMessage("2"));
@@ -104,21 +126,26 @@ public class OperationInvokingChannelAdapterParserTests {
}
@Test //INT-2275
public void testInvokeOperationWithinChain() throws Exception {
operationInvokingWithinChain.send(new GenericMessage<String>("test1"));
public void testInvokeOperationWithinChain() {
operationInvokingWithinChain.send(new GenericMessage<>("test1"));
assertEquals(1, testBean.messages.size());
}
@Test //INT-2275
public void testOperationWithinChainWithNonNullReturn() throws Exception {
try {
operationWithinChainWithNonNullReturn.send(new GenericMessage<String>("test1"));
fail("Expect MessagingException about non-null return");
}
catch (Exception e) {
assertTrue(e instanceof MessagingException);
// TODO Add check exception's message about 'must have a void return' after <jmx:operation-invoking-channel-adapter/> refactoring
}
@Test
public void testOperationWithinChainWithNonNullReturn() {
Log logger =
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"));
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) {
@@ -127,7 +154,7 @@ public class OperationInvokingChannelAdapterParserTests {
.setHeader(JmxHeaders.OPERATION_NAME, "blah").build();
}
public static class FooADvice extends AbstractRequestHandlerAdvice {
public static class FooAdvice extends AbstractRequestHandlerAdvice {
@Override
protected Object doInvoke(ExecutionCallback callback, Object target, Message<?> message) throws Exception {