diff --git a/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/OperationInvokingMessageHandler.java b/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/OperationInvokingMessageHandler.java index 7ab2d461f6..dba76edbca 100644 --- a/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/OperationInvokingMessageHandler.java +++ b/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/OperationInvokingMessageHandler.java @@ -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 defaultObjectName; + 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,7 +109,6 @@ 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) { @@ -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 @@ -120,49 +155,18 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa protected Object handleRequestMessage(Message requestMessage) { ObjectName objectName = resolveObjectName(requestMessage); String operation = resolveOperationName(requestMessage); - Map paramsFromMessage = this.resolveParameters(requestMessage); + Map paramsFromMessage = resolveParameters(requestMessage); try { - 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); - } - } + Object result = invokeOperation(requestMessage, objectName, operation, paramsFromMessage); + if (!this.expectReply && result != null) { + if (logger.isWarnEnabled()) { + logger.warn("This component doesn't expect a reply. " + + "The MBean operation '" + operation + "' result '" + result + + "' for '" + objectName + "' is ignored."); } + return null; } - if (hasNoArgOption) { - return this.server.invoke(objectName, operation, null, null); - } - throw new MessagingException(requestMessage, "failed to find JMX operation '" - + operation + "' 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 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 valueClass = value.getClass(); + Class valueClass = value.getClass(); if (valueClass.getName().equals(paramInfo.getType())) { return true; } @@ -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 operation = this.operationName; @@ -220,28 +272,27 @@ public class OperationInvokingMessageHandler extends AbstractReplyProducingMessa return operation; } - @SuppressWarnings({ "unchecked", "rawtypes" }) + @SuppressWarnings("unchecked") private Map resolveParameters(Message message) { Map map; - if (message.getPayload() instanceof Map) { - map = (Map) message.getPayload(); + Object payload = message.getPayload(); + if (payload instanceof Map) { + map = (Map) 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 (payload.getClass().isArray()) { + map = createParameterMapFromList(Arrays.asList(ObjectUtils.toObjectArray(payload))); } else { - map = this.createParameterMapFromList(Collections.singletonList(message.getPayload())); + map = createParameterMapFromList(Collections.singletonList(payload)); } return map; } - @SuppressWarnings("rawtypes") - private Map createParameterMapFromList(List parameters) { - Map map = new HashMap(); + private Map createParameterMapFromList(List parameters) { + Map map = new HashMap<>(); for (int i = 0; i < parameters.size(); i++) { map.put("p" + (i + 1), parameters.get(i)); } diff --git a/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParser.java b/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParser.java index b171aa99bc..7b1b494617 100644 --- a/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParser.java +++ b/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 @@ 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(); diff --git a/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingOutboundGatewayParser.java b/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingOutboundGatewayParser.java index 785fc54d6d..280a6b248d 100644 --- a/spring-integration-jmx/src/main/java/org/springframework/integration/jmx/config/OperationInvokingOutboundGatewayParser.java +++ b/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 @@ 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"); diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/OperationInvokingMessageHandlerTests.java b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/OperationInvokingMessageHandlerTests.java index 7060d7c3b8..0277e820a3 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/OperationInvokingMessageHandlerTests.java +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/OperationInvokingMessageHandlerTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -88,8 +88,7 @@ public class OperationInvokingMessageHandlerTests { @Test public void invocationWithMapPayload() { QueueChannel outputChannel = new QueueChannel(); - OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(); - handler.setServer(server); + OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server); handler.setObjectName(this.objectName); handler.setOutputChannel(outputChannel); handler.setOperationName("x"); @@ -108,8 +107,7 @@ public class OperationInvokingMessageHandlerTests { @Test public void invocationWithPayloadNoReturnValue() { QueueChannel outputChannel = new QueueChannel(); - OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(); - handler.setServer(server); + OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server); handler.setObjectName(this.objectName); handler.setOutputChannel(outputChannel); handler.setOperationName("y"); @@ -122,8 +120,7 @@ public class OperationInvokingMessageHandlerTests { @Test(expected = MessagingException.class) public void invocationWithMapPayloadNotEnoughParameters() { QueueChannel outputChannel = new QueueChannel(); - OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(); - handler.setServer(server); + OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server); handler.setObjectName(this.objectName); handler.setOutputChannel(outputChannel); handler.setOperationName("x"); @@ -141,8 +138,7 @@ public class OperationInvokingMessageHandlerTests { @Test public void invocationWithListPayload() { QueueChannel outputChannel = new QueueChannel(); - OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(); - handler.setServer(server); + OperationInvokingMessageHandler handler = new OperationInvokingMessageHandler(server); handler.setObjectName(this.objectName); handler.setOutputChannel(outputChannel); handler.setOperationName("x"); diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests-context.xml b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests-context.xml index f52b2162b9..0161506b21 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests-context.xml +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests-context.xml @@ -21,7 +21,7 @@ object-name="org.springframework.integration.jmx.config:type=TestBean,name=testBeanAdapter" operation-name="test"> - + @@ -37,8 +37,8 @@ operation-name="test"/> - - + diff --git a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests.java b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests.java index 21d22c711e..d4808f285c 100644 --- a/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests.java +++ b/spring-integration-jmx/src/test/java/org/springframework/integration/jmx/config/OperationInvokingChannelAdapterParserTests.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. @@ -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("test1")); - input.send(new GenericMessage("test2")); - input.send(new GenericMessage("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("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 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("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("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 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 {