From 9443c1c5dd59397e7ab07723794b61064bb15abe Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Mon, 4 Apr 2016 18:21:10 +0200 Subject: [PATCH] Fix Auto-Startup for @JmsListener Ignore container's auto-startup once the context is refreshed. Issue: SPR-14015 (cherry picked from commit 996c1cc) --- .../config/JmsListenerEndpointRegistry.java | 67 ++++++++++++------- .../jms/annotation/EnableJmsTests.java | 54 +++++++++++++-- .../JmsListenerContainerTestFactory.java | 11 ++- .../config/MessageListenerTestContainer.java | 19 ++++-- 4 files changed, 115 insertions(+), 36 deletions(-) diff --git a/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistry.java b/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistry.java index 5ffa326c4d..4d48fb31fe 100644 --- a/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistry.java +++ b/spring-jms/src/main/java/org/springframework/jms/config/JmsListenerEndpointRegistry.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -29,7 +29,11 @@ import org.apache.commons.logging.LogFactory; import org.springframework.beans.factory.BeanInitializationException; import org.springframework.beans.factory.DisposableBean; import org.springframework.beans.factory.InitializingBean; +import org.springframework.context.ApplicationContext; +import org.springframework.context.ApplicationContextAware; +import org.springframework.context.ApplicationListener; import org.springframework.context.SmartLifecycle; +import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.jms.listener.MessageListenerContainer; import org.springframework.util.Assert; @@ -53,7 +57,8 @@ import org.springframework.util.Assert; * @see MessageListenerContainer * @see JmsListenerContainerFactory */ -public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecycle { +public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecycle, + ApplicationContextAware, ApplicationListener { protected final Log logger = LogFactory.getLog(getClass()); @@ -62,6 +67,23 @@ public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecyc private int phase = Integer.MAX_VALUE; + private ApplicationContext applicationContext; + + private boolean contextRefreshed; + + + @Override + public void setApplicationContext(ApplicationContext applicationContext) { + this.applicationContext = applicationContext; + } + + @Override + public void onApplicationEvent(ContextRefreshedEvent event) { + if (event.getApplicationContext() == this.applicationContext) { + this.contextRefreshed = true; + } + } + /** * Return the {@link MessageListenerContainer} with the specified id or @@ -92,7 +114,6 @@ public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecyc return Collections.unmodifiableCollection(this.listenerContainers.values()); } - /** * Create a message listener container for the given {@link JmsListenerEndpoint}. *

This create the necessary infrastructure to honor that endpoint @@ -114,8 +135,9 @@ public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecyc String id = endpoint.getId(); Assert.notNull(id, "Endpoint id must not be null"); synchronized (this.listenerContainers) { - Assert.state(!this.listenerContainers.containsKey(id), - "Another endpoint is already registered with id '" + id + "'"); + if (this.listenerContainers.containsKey(id)) { + throw new IllegalStateException("Another endpoint is already registered with id '" + id + "'"); + } MessageListenerContainer container = createListenerContainer(endpoint, factory); this.listenerContainers.put(id, container); if (startImmediately) { @@ -166,21 +188,6 @@ public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecyc } - @Override - public void destroy() { - for (MessageListenerContainer listenerContainer : getListenerContainers()) { - if (listenerContainer instanceof DisposableBean) { - try { - ((DisposableBean) listenerContainer).destroy(); - } - catch (Throwable ex) { - logger.warn("Failed to destroy message listener container", ex); - } - } - } - } - - // Delegating implementation of SmartLifecycle @Override @@ -228,15 +235,29 @@ public class JmsListenerEndpointRegistry implements DisposableBean, SmartLifecyc /** * Start the specified {@link MessageListenerContainer} if it should be started - * on startup. + * on startup or when start is called explicitly after startup. * @see MessageListenerContainer#isAutoStartup() */ - private static void startIfNecessary(MessageListenerContainer listenerContainer) { - if (listenerContainer.isAutoStartup()) { + private void startIfNecessary(MessageListenerContainer listenerContainer) { + if (this.contextRefreshed || listenerContainer.isAutoStartup()) { listenerContainer.start(); } } + @Override + public void destroy() { + for (MessageListenerContainer listenerContainer : getListenerContainers()) { + if (listenerContainer instanceof DisposableBean) { + try { + ((DisposableBean) listenerContainer).destroy(); + } + catch (Throwable ex) { + logger.warn("Failed to destroy message listener container", ex); + } + } + } + } + private static class AggregatingCallback implements Runnable { diff --git a/spring-jms/src/test/java/org/springframework/jms/annotation/EnableJmsTests.java b/spring-jms/src/test/java/org/springframework/jms/annotation/EnableJmsTests.java index c2a350e526..a48e401ced 100644 --- a/spring-jms/src/test/java/org/springframework/jms/annotation/EnableJmsTests.java +++ b/spring-jms/src/test/java/org/springframework/jms/annotation/EnableJmsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -102,6 +102,31 @@ public class EnableJmsTests extends AbstractJmsAnnotationDrivenTests { testDefaultContainerFactoryConfiguration(context); } + @Test + public void containerAreStartedByDefault() { + ConfigurableApplicationContext context = new AnnotationConfigApplicationContext( + EnableJmsDefaultContainerFactoryConfig.class, DefaultBean.class); + JmsListenerContainerTestFactory factory = + context.getBean(JmsListenerContainerTestFactory.class); + MessageListenerTestContainer container = factory.getListenerContainers().get(0); + assertTrue(container.isAutoStartup()); + assertTrue(container.isStarted()); + } + + @Test + public void containerCanBeStarterViaTheRegistry() { + ConfigurableApplicationContext context = new AnnotationConfigApplicationContext( + EnableJmsAutoStartupFalseConfig.class, DefaultBean.class); + JmsListenerContainerTestFactory factory = + context.getBean(JmsListenerContainerTestFactory.class); + MessageListenerTestContainer container = factory.getListenerContainers().get(0); + assertFalse(container.isAutoStartup()); + assertFalse(container.isStarted()); + JmsListenerEndpointRegistry registry = context.getBean(JmsListenerEndpointRegistry.class); + registry.start(); + assertTrue(container.isStarted()); + } + @Override @Test public void jmsHandlerMethodFactoryConfiguration() throws JMSException { @@ -132,9 +157,8 @@ public class EnableJmsTests extends AbstractJmsAnnotationDrivenTests { @Test public void unknownFactory() { thrown.expect(BeanCreationException.class); - thrown.expectMessage("customFactory"); // Not found - new AnnotationConfigApplicationContext( - EnableJmsSampleConfig.class, CustomBean.class); + thrown.expectMessage("customFactory"); // not found + new AnnotationConfigApplicationContext(EnableJmsSampleConfig.class, CustomBean.class); } @Test @@ -145,11 +169,11 @@ public class EnableJmsTests extends AbstractJmsAnnotationDrivenTests { context.getBean("jmsListenerContainerFactory", JmsListenerContainerTestFactory.class); assertEquals(0, defaultFactory.getListenerContainers().size()); - context.getBean(LazyBean.class); // trigger lazy resolution + context.getBean(LazyBean.class); // trigger lazy resolution assertEquals(1, defaultFactory.getListenerContainers().size()); MessageListenerTestContainer container = defaultFactory.getListenerContainers().get(0); assertTrue("Should have been started " + container, container.isStarted()); - context.close(); // Close and stop the listeners + context.close(); // close and stop the listeners assertTrue("Should have been stopped " + container, container.isStopped()); } @@ -286,6 +310,24 @@ public class EnableJmsTests extends AbstractJmsAnnotationDrivenTests { } + @Configuration + @EnableJms + static class EnableJmsAutoStartupFalseConfig implements JmsListenerConfigurer { + + @Override + public void configureJmsListeners(JmsListenerEndpointRegistrar registrar) { + registrar.setContainerFactory(simpleFactory()); + } + + @Bean + public JmsListenerContainerTestFactory simpleFactory() { + JmsListenerContainerTestFactory factory = new JmsListenerContainerTestFactory(); + factory.setAutoStartup(false); + return factory; + } + } + + @Component @Lazy static class LazyBean { diff --git a/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerTestFactory.java b/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerTestFactory.java index d51e06ee63..611b823dcd 100644 --- a/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerTestFactory.java +++ b/spring-jms/src/test/java/org/springframework/jms/config/JmsListenerContainerTestFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -26,9 +26,17 @@ import java.util.Map; */ public class JmsListenerContainerTestFactory implements JmsListenerContainerFactory { + private boolean autoStartup = true; + private final Map listenerContainers = new LinkedHashMap<>(); + + public void setAutoStartup(boolean autoStartup) { + this.autoStartup = autoStartup; + } + + public List getListenerContainers() { return new ArrayList<>(this.listenerContainers.values()); } @@ -40,6 +48,7 @@ public class JmsListenerContainerTestFactory implements JmsListenerContainerFact @Override public MessageListenerTestContainer createListenerContainer(JmsListenerEndpoint endpoint) { MessageListenerTestContainer container = new MessageListenerTestContainer(endpoint); + container.setAutoStartup(this.autoStartup); this.listenerContainers.put(endpoint.getId(), container); return container; } diff --git a/spring-jms/src/test/java/org/springframework/jms/config/MessageListenerTestContainer.java b/spring-jms/src/test/java/org/springframework/jms/config/MessageListenerTestContainer.java index 7c723b7f42..e21b532088 100644 --- a/spring-jms/src/test/java/org/springframework/jms/config/MessageListenerTestContainer.java +++ b/spring-jms/src/test/java/org/springframework/jms/config/MessageListenerTestContainer.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2015 the original author or authors. + * Copyright 2002-2016 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. @@ -26,11 +26,12 @@ import org.springframework.jms.support.destination.DestinationResolver; /** * @author Stephane Nicoll */ -public class MessageListenerTestContainer - implements MessageListenerContainer, InitializingBean, DisposableBean { +public class MessageListenerTestContainer implements MessageListenerContainer, InitializingBean, DisposableBean { private final JmsListenerEndpoint endpoint; + private boolean autoStartup = true; + private boolean startInvoked; private boolean initializationInvoked; @@ -39,10 +40,16 @@ public class MessageListenerTestContainer private boolean destroyInvoked; + MessageListenerTestContainer(JmsListenerEndpoint endpoint) { this.endpoint = endpoint; } + + public void setAutoStartup(boolean autoStartup) { + this.autoStartup = autoStartup; + } + public JmsListenerEndpoint getEndpoint() { return endpoint; } @@ -86,7 +93,7 @@ public class MessageListenerTestContainer @Override public boolean isAutoStartup() { - return true; + return this.autoStartup; } @Override @@ -127,8 +134,7 @@ public class MessageListenerTestContainer @Override public void destroy() { if (!stopInvoked) { - throw new IllegalStateException("Stop should have been invoked before " + - "destroy on " + this); + throw new IllegalStateException("Stop should have been invoked before " + "destroy on " + this); } destroyInvoked = true; } @@ -144,4 +150,5 @@ public class MessageListenerTestContainer sb.append('}'); return sb.toString(); } + }