From 010ce8b01fa71b30a99b241fbb41c8f799baef80 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 10 Mar 2021 16:22:53 -0500 Subject: [PATCH] Fail JmsMDEndpoint if container has a listener Right now the `JmsMessageDrivenEndpoint` only logs a warn about already provided listener. Turns out this is not enough for end-users and they may miss the warn message in the logs somehow having some problems at runtime trying to determine find out the root of unexpected behavior * Change the warn message to the `Assert.isNull()` to fail fast. Same is done in the `KafkaMessageDrivenChannelAdapter` & `AmqpInboundChannelAdapter` --- .../jms/JmsMessageDrivenEndpoint.java | 10 ++--- .../config/JmsMessageDrivenEndpointTests.java | 40 ++++++++++--------- 2 files changed, 26 insertions(+), 24 deletions(-) diff --git a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsMessageDrivenEndpoint.java b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsMessageDrivenEndpoint.java index b2bf2027dd..f4e8d617cc 100644 --- a/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsMessageDrivenEndpoint.java +++ b/spring-integration-jms/src/main/java/org/springframework/integration/jms/JmsMessageDrivenEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2021 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. @@ -74,10 +74,10 @@ public class JmsMessageDrivenEndpoint extends MessageProducerSupport implements Assert.notNull(listenerContainer, "listener container must not be null"); Assert.notNull(listener, "listener must not be null"); - if (listenerContainer.getMessageListener() != null) { - logger.warn("The provided listener container already has a MessageListener implementation, " + - "but it will be overridden by the provided ChannelPublishingJmsMessageListener."); - } + Assert.isNull(listenerContainer.getMessageListener(), + "The listenerContainer provided to a JMS Inbound Endpoint " + + "must not have a MessageListener configured since the endpoint " + + "configures its own listener implementation."); listenerContainer.setMessageListener(listener); this.listener = listener; this.listenerContainer = listenerContainer; diff --git a/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsMessageDrivenEndpointTests.java b/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsMessageDrivenEndpointTests.java index 772f94157b..a115df322d 100644 --- a/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsMessageDrivenEndpointTests.java +++ b/spring-integration-jms/src/test/java/org/springframework/integration/jms/config/JmsMessageDrivenEndpointTests.java @@ -17,10 +17,11 @@ package org.springframework.integration.jms.config; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; +import static org.mockito.Mockito.mock; -import javax.jms.ConnectionFactory; +import javax.jms.MessageListener; -import org.apache.activemq.ActiveMQConnectionFactory; import org.junit.jupiter.api.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -28,10 +29,10 @@ import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.integration.channel.QueueChannel; import org.springframework.integration.config.EnableIntegration; +import org.springframework.integration.jms.ActiveMQMultiContextTests; import org.springframework.integration.jms.ChannelPublishingJmsMessageListener; import org.springframework.integration.jms.JmsMessageDrivenEndpoint; import org.springframework.integration.test.util.TestUtils; -import org.springframework.jms.connection.CachingConnectionFactory; import org.springframework.jms.core.JmsTemplate; import org.springframework.jms.listener.AbstractMessageListenerContainer; import org.springframework.jms.listener.DefaultMessageListenerContainer; @@ -40,12 +41,25 @@ import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; /** * @author Gary Russell + * @author Artem Bilan + * * @since 5.1 * */ @SpringJUnitConfig @DirtiesContext -public class JmsMessageDrivenEndpointTests { +public class JmsMessageDrivenEndpointTests extends ActiveMQMultiContextTests { + + @Test + public void testListenerIsAlreadyProvided() { + DefaultMessageListenerContainer container = new DefaultMessageListenerContainer(); + container.setMessageListener(mock(MessageListener.class)); + assertThatIllegalArgumentException() + .isThrownBy(() -> new JmsMessageDrivenEndpoint(container, new ChannelPublishingJmsMessageListener())) + .withMessage("The listenerContainer provided to a JMS Inbound Endpoint must not have " + + "a MessageListener configured since the endpoint configures its own listener implementation."); + } + @Test public void testStopStart(@Autowired JmsTemplate template, @@ -63,32 +77,20 @@ public class JmsMessageDrivenEndpointTests { @EnableIntegration public static class Config { - - @Bean - public ConnectionFactory cf() { - return new ActiveMQConnectionFactory("vm://localhost?broker.persistent=false"); - } - - @Bean - public CachingConnectionFactory ccf() { - return new CachingConnectionFactory(cf()); - } - @Bean public JmsTemplate template() { - return new JmsTemplate(ccf()); + return new JmsTemplate(connectionFactory); } @Bean public JmsMessageDrivenEndpoint inbound() { - JmsMessageDrivenEndpoint endpoint = new JmsMessageDrivenEndpoint(container(), listener()); - return endpoint; + return new JmsMessageDrivenEndpoint(container(), listener()); } @Bean public AbstractMessageListenerContainer container() { DefaultMessageListenerContainer container = new DefaultMessageListenerContainer(); - container.setConnectionFactory(cf()); + container.setConnectionFactory(amqFactory); container.setSessionTransacted(true); container.setDestinationName("stop.start"); return container;