From 57f7fd53f0a0981e92cfcdef495ba5e13caa4ebc Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Thu, 5 Sep 2019 08:57:42 -0400 Subject: [PATCH] GH-1080: SMLC: Fix concurrency configuration order Fixes https://github.com/spring-projects/spring-amqp/issues/1080 When we have a configuration like this: ``` container.setConcurrentConsumers(1); container.setMaxConcurrentConsumers(1); container.setConcurrency("2-5"); ``` we fail with an assertion like `'concurrentConsumers' cannot be more than 'maxConcurrentConsumers'` * Change the order in the `SimpleMessageListenerContainer.setConcurrency()` how we populate `maxConcurrentConsumers` and `concurrentConsumers` **Cherry-pick to 2.1.x and 1.7.x** * * Reset `concurrentConsumers` and `maxConcurrentConsumers` to their default before parsing `concurrency` string * * Validate concurrency values before setting into properties * * Reset old values and call setters for concurrency bits --- .../listener/SimpleMessageListenerContainer.java | 11 ++++++++--- ...ageListenerContainerLifecycleIntegrationTests.java | 10 ++++++++++ 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/SimpleMessageListenerContainer.java b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/SimpleMessageListenerContainer.java index 94ca2ba8..d8552cbf 100644 --- a/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/SimpleMessageListenerContainer.java +++ b/spring-rabbit/src/main/java/org/springframework/amqp/rabbit/listener/SimpleMessageListenerContainer.java @@ -217,9 +217,14 @@ public class SimpleMessageListenerContainer extends AbstractMessageListenerConta try { int separatorIndex = concurrency.indexOf('-'); if (separatorIndex != -1) { - setConcurrentConsumers(Integer.parseInt(concurrency.substring(0, separatorIndex))); - setMaxConcurrentConsumers( - Integer.parseInt(concurrency.substring(separatorIndex + 1, concurrency.length()))); + int concurrentConsumers = Integer.parseInt(concurrency.substring(0, separatorIndex)); + int maxConcurrentConsumers = Integer.parseInt(concurrency.substring(separatorIndex + 1)); + Assert.isTrue(maxConcurrentConsumers >= concurrentConsumers, + "'maxConcurrentConsumers' value must be at least 'concurrentConsumers'"); + this.concurrentConsumers = 1; + this.maxConcurrentConsumers = null; + setConcurrentConsumers(concurrentConsumers); + setMaxConcurrentConsumers(maxConcurrentConsumers); } else { setConcurrentConsumers(Integer.parseInt(concurrency)); diff --git a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerContainerLifecycleIntegrationTests.java b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerContainerLifecycleIntegrationTests.java index 31d62345..f5ad5cb0 100755 --- a/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerContainerLifecycleIntegrationTests.java +++ b/spring-rabbit/src/test/java/org/springframework/amqp/rabbit/listener/MessageListenerContainerLifecycleIntegrationTests.java @@ -472,6 +472,16 @@ public class MessageListenerContainerLifecycleIntegrationTests { ((DisposableBean) template.getConnectionFactory()).destroy(); } + @Test + public void testConcurrencyConfiguration() { + SimpleMessageListenerContainer container = new SimpleMessageListenerContainer(); + container.setConcurrentConsumers(1); + container.setMaxConcurrentConsumers(1); + container.setConcurrency("2-5"); + + assertThat(TestUtils.getPropertyValue(container, "concurrentConsumers")).isEqualTo(2); + assertThat(TestUtils.getPropertyValue(container, "maxConcurrentConsumers")).isEqualTo(5); + } @Configuration static class LongLiveConsumerConfig {