From 5c4d13ca265f3cd4ba6323cfe5611a1053dd055c Mon Sep 17 00:00:00 2001 From: Dave Syer Date: Mon, 14 Mar 2011 17:49:12 +0000 Subject: [PATCH] AMQP-118: removed template constructor from RabbitAdmin --- .../amqp/rabbit/config/AdminParser.java | 31 ++------ .../amqp/rabbit/core/RabbitAdmin.java | 5 -- .../amqp/rabbit/config/spring-rabbit-1.0.xsd | 14 ---- .../amqp/rabbit/config/AdminParserTests.java | 75 ++++--------------- ...hingConnectionFactoryIntegrationTests.java | 6 +- .../amqp/rabbit/core/RabbitAdminTests.java | 5 +- .../core/RabbitBindingIntegrationTests.java | 15 ++-- ...nerBrokerInterruptionIntegrationTests.java | 11 --- ...veryCachingConnectionIntegrationTests.java | 26 +++++-- .../config/AdminParserTests-2-context.xml | 13 +--- .../config/AdminParserTests-3-context.xml | 15 ---- .../config/AdminParserTests-4-context.xml | 17 ----- .../config/AdminParserTests-5-context.xml | 14 ---- .../config/AdminParserTests-6-context.xml | 19 ----- 14 files changed, 58 insertions(+), 208 deletions(-) delete mode 100644 spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-3-context.xml delete mode 100644 spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-4-context.xml delete mode 100644 spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-5-context.xml delete mode 100644 spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-6-context.xml diff --git a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/AdminParser.java b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/AdminParser.java index 16276535..d1d13479 100644 --- a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/AdminParser.java +++ b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/config/AdminParser.java @@ -25,12 +25,10 @@ import org.w3c.dom.Element; */ class AdminParser extends AbstractSingleBeanDefinitionParser { - private static final String TEMPLATE_ATTRIBUTE = "template"; - private static final String CONNECTION_FACTORY_ATTRIBUTE = "connection-factory"; private static final String PHASE_ATTRIBUTE = "phase"; - + private static final String AUTO_STARTUP_ATTRIBUTE = "auto-startup"; @Override @@ -50,30 +48,15 @@ class AdminParser extends AbstractSingleBeanDefinitionParser { @Override protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) { - boolean templateAttributeExist = element.getAttributeNode(TEMPLATE_ATTRIBUTE) != null; - boolean connectionFactoryAttributeExist = element.getAttributeNode(CONNECTION_FACTORY_ATTRIBUTE) != null; - // Only one of 'templateRef' or 'connectionFactoryRef' can be set. - if (templateAttributeExist && connectionFactoryAttributeExist) { - parserContext.getReaderContext().error( - "Either '" + TEMPLATE_ATTRIBUTE + "' or '" + CONNECTION_FACTORY_ATTRIBUTE - + "' attribute must be set.", element); - } - - String templateRef = element.getAttribute(TEMPLATE_ATTRIBUTE); String connectionFactoryRef = element.getAttribute(CONNECTION_FACTORY_ATTRIBUTE); - + // At least one of 'templateRef' or 'connectionFactoryRef' attribute must be set. - if (!StringUtils.hasText(templateRef) && !StringUtils.hasText(connectionFactoryRef)) { - parserContext.getReaderContext().error( - "One of '" + TEMPLATE_ATTRIBUTE + "' or '" + CONNECTION_FACTORY_ATTRIBUTE - + "' attribute must be set.", element); + if (!StringUtils.hasText(connectionFactoryRef)) { + parserContext.getReaderContext().error("A '" + CONNECTION_FACTORY_ATTRIBUTE + "' attribute must be set.", + element); } - - if (StringUtils.hasText(templateRef)) { - // Use constructor with template parameter - builder.addConstructorArgReference(templateRef); - } else if (StringUtils.hasText(connectionFactoryRef)) { + if (StringUtils.hasText(connectionFactoryRef)) { // Use constructor with connectionFactory parameter builder.addConstructorArgReference(connectionFactoryRef); } @@ -83,7 +66,7 @@ class AdminParser extends AbstractSingleBeanDefinitionParser { if (StringUtils.hasText(attributeValue)) { builder.addPropertyValue("phase", attributeValue); } - + attributeValue = element.getAttribute(AUTO_STARTUP_ATTRIBUTE); if (StringUtils.hasText(attributeValue)) { builder.addPropertyValue("autoStartup", attributeValue); diff --git a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java index 52b74bb5..417e98b7 100644 --- a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java +++ b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/core/RabbitAdmin.java @@ -61,11 +61,6 @@ public class RabbitAdmin implements AmqpAdmin, ApplicationContextAware, SmartLif this.rabbitTemplate = new RabbitTemplate(connectionFactory); } - public RabbitAdmin(RabbitTemplate rabbitTemplate) { - Assert.notNull(rabbitTemplate, "RabbitTemplate must not be null"); - this.rabbitTemplate = rabbitTemplate; - } - public void setAutoStartup(boolean autoStartup) { this.autoStartup = autoStartup; } diff --git a/spring-rabbit/src/main/resources/org/springframework/amqp/rabbit/config/spring-rabbit-1.0.xsd b/spring-rabbit/src/main/resources/org/springframework/amqp/rabbit/config/spring-rabbit-1.0.xsd index 514096ab..6593ec5c 100644 --- a/spring-rabbit/src/main/resources/org/springframework/amqp/rabbit/config/spring-rabbit-1.0.xsd +++ b/spring-rabbit/src/main/resources/org/springframework/amqp/rabbit/config/spring-rabbit-1.0.xsd @@ -551,20 +551,6 @@ ]]> - - - - - - - - - - data() { - Object[][] data = new Object[][] { // - params(0, true), // #0 - params(1, false), // #1 - params(2, false), // #2 - params(3, false), // #3 - params(4, false), // #4 - params(5, true, "admin-test", 12, false, false), // #5 - params(6, true, "admin-test", 12, false, true) // #6 - }; - return Arrays.asList(data); - } - - private static Object[] params(int index, boolean validContext) { - return params(index, validContext, null, Integer.MIN_VALUE, true, false); - } - - /** - * - * @param contextIndex The index of spring context. Context file name template: - * <class-name>-<contextIndex>-context.xml - * @param validContext true if spring-context is expected to be loaded without failures. - * @param adminBeanName The bean name of expected rabbit admin. If its not specified - rabbit admin will be - * retrieved by type. - * @param expectedPhase 'phase' expected in {@link RabbitAdmin}. - * @param expectedAutoStartup 'autoStartup' expected in {@link RabbitAdmin}. - * @param initialisedWithTemplat true if {@link RabbitAdmin} in spring-context initialized by passing - * {@link RabbitTemplate} as constructor parameter, false - initialized by passing - * {@link ConnectionFactory} as constructor parameter. - * @return - */ - private static Object[] params(int contextIndex, boolean validContext, String adminBeanName, int expectedPhase, - boolean expectedAutoStartup, boolean initialisedWithTemplat) { - return new Object[] { contextIndex, validContext, adminBeanName, expectedPhase, expectedAutoStartup, - initialisedWithTemplat }; + @Test + public void testInvalid() throws Exception { + contextIndex = 1; + validContext = false; + doTest(); } @Test - public void testParse() throws Exception { + public void testValid() throws Exception { + contextIndex = 2; + validContext = true; + expectedPhase = 12; + doTest(); + } + + private void doTest() throws Exception { // Create context XmlBeanFactory beanFactory = loadContext(); if (beanFactory == null) { diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactoryIntegrationTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactoryIntegrationTests.java index a747fd21..c9d82bdb 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactoryIntegrationTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/connection/CachingConnectionFactoryIntegrationTests.java @@ -44,7 +44,7 @@ public class CachingConnectionFactoryIntegrationTests { RabbitTemplate template = new RabbitTemplate(connectionFactory); - RabbitAdmin admin = new RabbitAdmin(template); + RabbitAdmin admin = new RabbitAdmin(connectionFactory); Queue queue = admin.declareQueue(); template.convertAndSend(queue.getName(), "message"); String result = (String) template.receiveAndConvert(queue.getName()); @@ -57,7 +57,7 @@ public class CachingConnectionFactoryIntegrationTests { RabbitTemplate template = new RabbitTemplate(connectionFactory); - RabbitAdmin admin = new RabbitAdmin(template); + RabbitAdmin admin = new RabbitAdmin(connectionFactory); Queue queue = admin.declareQueue(); template.convertAndSend(queue.getName(), "message"); @@ -80,7 +80,7 @@ public class CachingConnectionFactoryIntegrationTests { RabbitTemplate template2 = new RabbitTemplate(connectionFactory); template1.setChannelTransacted(true); - RabbitAdmin admin = new RabbitAdmin(template1); + RabbitAdmin admin = new RabbitAdmin(connectionFactory); Queue queue = admin.declareQueue(); template1.convertAndSend(queue.getName(), "message"); diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java index 6bbf7a73..eaccc119 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitAdminTests.java @@ -3,14 +3,15 @@ package org.springframework.amqp.rabbit.core; import static org.junit.Assert.fail; import org.junit.Test; +import org.springframework.amqp.rabbit.connection.ConnectionFactory; public class RabbitAdminTests { @Test public void testSettingOfNullRabbitTemplate() { - RabbitTemplate rabbitTemplate = null; + ConnectionFactory connectionFactory = null; try { - new RabbitAdmin(rabbitTemplate); + new RabbitAdmin(connectionFactory); fail("should have thrown IllegalStateException when RabbitTemplate is not set."); } catch (IllegalArgumentException e) { diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitBindingIntegrationTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitBindingIntegrationTests.java index 3296c797..0a309f3a 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitBindingIntegrationTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/core/RabbitBindingIntegrationTests.java @@ -12,6 +12,7 @@ import org.springframework.amqp.core.Message; import org.springframework.amqp.core.Queue; import org.springframework.amqp.core.TopicExchange; import org.springframework.amqp.rabbit.connection.CachingConnectionFactory; +import org.springframework.amqp.rabbit.connection.ConnectionFactory; import org.springframework.amqp.rabbit.listener.BlockingQueueConsumer; import org.springframework.amqp.rabbit.support.RabbitAccessor; import org.springframework.amqp.rabbit.test.BrokerRunning; @@ -23,7 +24,9 @@ public class RabbitBindingIntegrationTests { private static Queue queue = new Queue("test.queue"); - private RabbitTemplate template = new RabbitTemplate(new CachingConnectionFactory()); + private ConnectionFactory connectionFactory = new CachingConnectionFactory(); + + private RabbitTemplate template = new RabbitTemplate(connectionFactory ); @Rule public BrokerRunning brokerIsRunning = BrokerRunning.isRunningWithEmptyQueue(queue); @@ -31,7 +34,7 @@ public class RabbitBindingIntegrationTests { @Test public void testSendAndReceiveWithTopicSingleCallback() throws Exception { - final RabbitAdmin admin = new RabbitAdmin(template); + final RabbitAdmin admin = new RabbitAdmin(connectionFactory); final TopicExchange exchange = new TopicExchange("topic"); admin.declareExchange(exchange); template.setExchange(exchange.getName()); @@ -71,7 +74,7 @@ public class RabbitBindingIntegrationTests { @Test public void testSendAndReceiveWithNonDefaultExchange() throws Exception { - final RabbitAdmin admin = new RabbitAdmin(template); + final RabbitAdmin admin = new RabbitAdmin(connectionFactory); final TopicExchange exchange = new TopicExchange("topic"); admin.declareExchange(exchange); @@ -110,7 +113,7 @@ public class RabbitBindingIntegrationTests { // @Ignore("Not sure yet if we need to support a use case like this") public void testSendAndReceiveWithTopicConsumeInBackground() throws Exception { - RabbitAdmin admin = new RabbitAdmin(template); + RabbitAdmin admin = new RabbitAdmin(connectionFactory); TopicExchange exchange = new TopicExchange("topic"); admin.declareExchange(exchange); template.setExchange(exchange.getName()); @@ -147,7 +150,7 @@ public class RabbitBindingIntegrationTests { @Test public void testSendAndReceiveWithTopicTwoCallbacks() throws Exception { - RabbitAdmin admin = new RabbitAdmin(template); + RabbitAdmin admin = new RabbitAdmin(connectionFactory); TopicExchange exchange = new TopicExchange("topic"); admin.declareExchange(exchange); template.setExchange(exchange.getName()); @@ -199,7 +202,7 @@ public class RabbitBindingIntegrationTests { @Test public void testSendAndReceiveWithFanout() throws Exception { - RabbitAdmin admin = new RabbitAdmin(template); + RabbitAdmin admin = new RabbitAdmin(connectionFactory); FanoutExchange exchange = new FanoutExchange("fanout"); admin.declareExchange(exchange); template.setExchange(exchange.getName()); diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerBrokerInterruptionIntegrationTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerBrokerInterruptionIntegrationTests.java index 1b68240f..33fd89b5 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerBrokerInterruptionIntegrationTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerBrokerInterruptionIntegrationTests.java @@ -17,7 +17,6 @@ import org.junit.After; import org.junit.Before; import org.junit.Rule; import org.junit.Test; -import org.springframework.amqp.AmqpIllegalStateException; import org.springframework.amqp.core.AcknowledgeMode; import org.springframework.amqp.core.Message; import org.springframework.amqp.core.Queue; @@ -138,16 +137,6 @@ public class MessageListenerBrokerInterruptionIntegrationTests { } - @Test(expected=AmqpIllegalStateException.class) - public void testListenerDoesNotRecoverFromMissingQueue() throws Exception { - - CountDownLatch latch = new CountDownLatch(messageCount); - container = createContainer("nonexistent", new VanillaListener(latch), connectionFactory); - - brokerAdmin.stopBrokerApplication(); - - } - private SimpleMessageListenerContainer createContainer(String queueName, Object listener, ConnectionFactory connectionFactory) { SimpleMessageListenerContainer container = new SimpleMessageListenerContainer(connectionFactory); diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerRecoveryCachingConnectionIntegrationTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerRecoveryCachingConnectionIntegrationTests.java index 70a8d442..ae0362e4 100644 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerRecoveryCachingConnectionIntegrationTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerRecoveryCachingConnectionIntegrationTests.java @@ -14,6 +14,7 @@ import org.apache.log4j.Level; import org.junit.After; import org.junit.Rule; import org.junit.Test; +import org.springframework.amqp.AmqpIllegalStateException; import org.springframework.amqp.core.AcknowledgeMode; import org.springframework.amqp.core.Message; import org.springframework.amqp.core.Queue; @@ -23,6 +24,7 @@ import org.springframework.amqp.rabbit.connection.ConnectionFactory; import org.springframework.amqp.rabbit.connection.ConnectionProxy; import org.springframework.amqp.rabbit.core.ChannelAwareMessageListener; import org.springframework.amqp.rabbit.core.RabbitTemplate; +import org.springframework.amqp.rabbit.listener.MessageListenerBrokerInterruptionIntegrationTests.VanillaListener; import org.springframework.amqp.rabbit.listener.adapter.MessageListenerAdapter; import org.springframework.amqp.rabbit.test.BrokerRunning; import org.springframework.amqp.rabbit.test.BrokerTestUtils; @@ -80,7 +82,7 @@ public class MessageListenerRecoveryCachingConnectionIntegrationTests { acknowledgeMode = AcknowledgeMode.MANUAL; CountDownLatch latch = new CountDownLatch(messageCount); - container = createContainer(new ManualAckListener(latch), createConnectionFactory()); + container = createContainer(queue.getName(), new ManualAckListener(latch), createConnectionFactory()); for (int i = 0; i < messageCount; i++) { template.convertAndSend(queue.getName(), i + "foo"); } @@ -100,7 +102,7 @@ public class MessageListenerRecoveryCachingConnectionIntegrationTests { RabbitTemplate template = new RabbitTemplate(createConnectionFactory()); CountDownLatch latch = new CountDownLatch(messageCount); - container = createContainer(new AbortChannelListener(latch), createConnectionFactory()); + container = createContainer(queue.getName(), new AbortChannelListener(latch), createConnectionFactory()); for (int i = 0; i < messageCount; i++) { template.convertAndSend(queue.getName(), i + "foo"); } @@ -120,7 +122,7 @@ public class MessageListenerRecoveryCachingConnectionIntegrationTests { RabbitTemplate template = new RabbitTemplate(createConnectionFactory()); CountDownLatch latch = new CountDownLatch(messageCount); - container = createContainer(new AbortChannelListener(latch), createConnectionFactory()); + container = createContainer(queue.getName(), new AbortChannelListener(latch), createConnectionFactory()); assertEquals(concurrentConsumers, container.getActiveConsumerCount()); for (int i = 0; i < messageCount; i++) { @@ -147,8 +149,8 @@ public class MessageListenerRecoveryCachingConnectionIntegrationTests { CountDownLatch latch = new CountDownLatch(messageCount); ConnectionFactory connectionFactory = createConnectionFactory(); - container = createContainer(new CloseConnectionListener((ConnectionProxy) connectionFactory.createConnection(), - latch), connectionFactory); + container = createContainer(queue.getName(), new CloseConnectionListener((ConnectionProxy) connectionFactory.createConnection(), + latch), connectionFactory); for (int i = 0; i < messageCount; i++) { template.convertAndSend(queue.getName(), i + "foo"); } @@ -171,7 +173,7 @@ public class MessageListenerRecoveryCachingConnectionIntegrationTests { acknowledgeMode = AcknowledgeMode.MANUAL; CountDownLatch latch = new CountDownLatch(messageCount); - container = createContainer(new ManualAckListener(latch), connectionFactory); + container = createContainer(queue.getName(), new ManualAckListener(latch), connectionFactory); for (int i = 0; i < messageCount; i++) { template.convertAndSend(queue.getName(), i + "foo"); } @@ -186,10 +188,18 @@ public class MessageListenerRecoveryCachingConnectionIntegrationTests { } - private SimpleMessageListenerContainer createContainer(Object listener, ConnectionFactory connectionFactory) { + @Test(expected=AmqpIllegalStateException.class) + public void testListenerDoesNotRecoverFromMissingQueue() throws Exception { + // TODO: with only 1 this test tends to fail + concurrentConsumers = 3; + CountDownLatch latch = new CountDownLatch(messageCount); + container = createContainer("nonexistent", new VanillaListener(latch), createConnectionFactory()); + } + + private SimpleMessageListenerContainer createContainer(String queueName, Object listener, ConnectionFactory connectionFactory) { SimpleMessageListenerContainer container = new SimpleMessageListenerContainer(connectionFactory); container.setMessageListener(new MessageListenerAdapter(listener)); - container.setQueueName(queue.getName()); + container.setQueueName(queueName); container.setTxSize(txSize); container.setPrefetchCount(txSize); container.setConcurrentConsumers(concurrentConsumers); diff --git a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-2-context.xml b/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-2-context.xml index 3147298a..4a3f47ad 100644 --- a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-2-context.xml +++ b/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-2-context.xml @@ -1,19 +1,14 @@ - - - - - - + + diff --git a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-3-context.xml b/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-3-context.xml deleted file mode 100644 index 3cd1d578..00000000 --- a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-3-context.xml +++ /dev/null @@ -1,15 +0,0 @@ - - - - - - - - - diff --git a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-4-context.xml b/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-4-context.xml deleted file mode 100644 index ffcd8a39..00000000 --- a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-4-context.xml +++ /dev/null @@ -1,17 +0,0 @@ - - - - - - - - - - - - - diff --git a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-5-context.xml b/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-5-context.xml deleted file mode 100644 index 4a3f47ad..00000000 --- a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-5-context.xml +++ /dev/null @@ -1,14 +0,0 @@ - - - - - - - - - diff --git a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-6-context.xml b/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-6-context.xml deleted file mode 100644 index 2bf7516b..00000000 --- a/spring-rabbit/src/test/resources/org/springframework/amqp/rabbit/config/AdminParserTests-6-context.xml +++ /dev/null @@ -1,19 +0,0 @@ - - - - - - - - - - - - -