From fb7d81c4a2a2a1e9684c6e7521d168fec4b986b0 Mon Sep 17 00:00:00 2001 From: Sebastien Deleuze Date: Mon, 31 Mar 2014 09:05:28 +0200 Subject: [PATCH 1/2] Fix default configuration Adding a ChannelInterceptor does not suppress default executor settings anymore in the XML namespace. Issue: SPR-11623 --- .../MessageBrokerBeanDefinitionParser.java | 5 ++-- ...essageBrokerBeanDefinitionParserTests.java | 9 ++++++ ...broker-customchannels-default-executor.xml | 30 +++++++++++++++++++ 3 files changed, 42 insertions(+), 2 deletions(-) create mode 100644 spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java index 4bfc632782..400a83e026 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java @@ -176,9 +176,10 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser { ParserContext parserCxt, Object source) { RootBeanDefinition executorDef = null; + Element executor = null; if (channelElement != null) { - Element executor = DomUtils.getChildElementByTagName(channelElement, "executor"); + executor = DomUtils.getChildElementByTagName(channelElement, "executor"); if (executor != null) { executorDef = new RootBeanDefinition(ThreadPoolTaskExecutor.class); String attrValue = executor.getAttribute("core-pool-size"); @@ -199,7 +200,7 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser { } } } - else if (!channelName.equals("brokerChannel")) { + if ((channelElement == null && !channelName.equals("brokerChannel")) || (channelElement != null && executor == null)) { executorDef = new RootBeanDefinition(ThreadPoolTaskExecutor.class); executorDef.getPropertyValues().add("corePoolSize", Runtime.getRuntime().availableProcessors() * 2); executorDef.getPropertyValues().add("maxPoolSize", Integer.MAX_VALUE); diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java index 632fe34101..4b5cd1f289 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java @@ -293,6 +293,15 @@ public class MessageBrokerBeanDefinitionParserTests { testExecutor("brokerChannel", 102, 202, 602); } + // SPR-11623 + + @Test + public void customChannelsWithDefaultExecutor() { + loadBeanDefinitions("websocket-config-broker-customchannels-default-executor.xml"); + List> subscriberTypes = Arrays.>asList(SubProtocolWebSocketHandler.class); + testExecutor("clientOutboundChannel", Runtime.getRuntime().availableProcessors() * 2, Integer.MAX_VALUE, 60); + } + @Test public void messageConverters() { loadBeanDefinitions("websocket-config-broker-converters.xml"); diff --git a/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml b/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml new file mode 100644 index 0000000000..278f3fef3d --- /dev/null +++ b/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml @@ -0,0 +1,30 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + From 6ec3de6029e61ea4b88889ca12ea3d19fe29a4c6 Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Tue, 1 Apr 2014 11:16:13 -0400 Subject: [PATCH 2/2] Fix issue with default executor for broker channel The default for the broker channel should be "no executor". Issue: SPR-11623 --- .../MessageBrokerBeanDefinitionParser.java | 37 ++++++++++++------- ...essageBrokerBeanDefinitionParserTests.java | 4 +- ...broker-customchannels-default-executor.xml | 16 ++++---- 3 files changed, 34 insertions(+), 23 deletions(-) diff --git a/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java b/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java index 400a83e026..c79df74ef9 100644 --- a/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java +++ b/spring-websocket/src/main/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParser.java @@ -176,11 +176,15 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser { ParserContext parserCxt, Object source) { RootBeanDefinition executorDef = null; - Element executor = null; - - if (channelElement != null) { - executor = DomUtils.getChildElementByTagName(channelElement, "executor"); - if (executor != null) { + if (channelElement == null) { + executorDef = getDefaultExecutorBeanDefinition(channelName); + } + else { + Element executor = DomUtils.getChildElementByTagName(channelElement, "executor"); + if (executor == null) { + executorDef = getDefaultExecutorBeanDefinition(channelName); + } + else { executorDef = new RootBeanDefinition(ThreadPoolTaskExecutor.class); String attrValue = executor.getAttribute("core-pool-size"); if (!StringUtils.isEmpty(attrValue)) { @@ -200,22 +204,16 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser { } } } - if ((channelElement == null && !channelName.equals("brokerChannel")) || (channelElement != null && executor == null)) { - executorDef = new RootBeanDefinition(ThreadPoolTaskExecutor.class); - executorDef.getPropertyValues().add("corePoolSize", Runtime.getRuntime().availableProcessors() * 2); - executorDef.getPropertyValues().add("maxPoolSize", Integer.MAX_VALUE); - executorDef.getPropertyValues().add("queueCapacity", Integer.MAX_VALUE); - } - ConstructorArgumentValues values = new ConstructorArgumentValues(); + ConstructorArgumentValues argValues = new ConstructorArgumentValues(); if (executorDef != null) { executorDef.getPropertyValues().add("threadNamePrefix", channelName + "-"); String executorName = channelName + "Executor"; registerBeanDefByName(executorName, executorDef, parserCxt, source); - values.addIndexedArgumentValue(0, new RuntimeBeanReference(executorName)); + argValues.addIndexedArgumentValue(0, new RuntimeBeanReference(executorName)); } - RootBeanDefinition channelDef = new RootBeanDefinition(ExecutorSubscribableChannel.class, values, null); + RootBeanDefinition channelDef = new RootBeanDefinition(ExecutorSubscribableChannel.class, argValues, null); if (channelElement != null) { Element interceptorsElement = DomUtils.getChildElementByTagName(channelElement, "interceptors"); @@ -227,6 +225,17 @@ class MessageBrokerBeanDefinitionParser implements BeanDefinitionParser { return new RuntimeBeanReference(channelName); } + private RootBeanDefinition getDefaultExecutorBeanDefinition(String channelName) { + if (channelName.equals("brokerChannel")) { + return null; + } + RootBeanDefinition executorDef = new RootBeanDefinition(ThreadPoolTaskExecutor.class); + executorDef.getPropertyValues().add("corePoolSize", Runtime.getRuntime().availableProcessors() * 2); + executorDef.getPropertyValues().add("maxPoolSize", Integer.MAX_VALUE); + executorDef.getPropertyValues().add("queueCapacity", Integer.MAX_VALUE); + return executorDef; + } + private RuntimeBeanReference registerSubProtocolWebSocketHandler(Element element, RuntimeBeanReference clientInChannel, RuntimeBeanReference clientOutChannel, RuntimeBeanReference userSessionRegistry, ParserContext parserCxt, Object source) { diff --git a/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java b/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java index 4b5cd1f289..1c9b295d97 100644 --- a/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java +++ b/spring-websocket/src/test/java/org/springframework/web/socket/config/MessageBrokerBeanDefinitionParserTests.java @@ -298,8 +298,10 @@ public class MessageBrokerBeanDefinitionParserTests { @Test public void customChannelsWithDefaultExecutor() { loadBeanDefinitions("websocket-config-broker-customchannels-default-executor.xml"); - List> subscriberTypes = Arrays.>asList(SubProtocolWebSocketHandler.class); + + testExecutor("clientInboundChannel", Runtime.getRuntime().availableProcessors() * 2, Integer.MAX_VALUE, 60); testExecutor("clientOutboundChannel", Runtime.getRuntime().availableProcessors() * 2, Integer.MAX_VALUE, 60); + assertFalse(this.appContext.containsBean("brokerChannelExecutor")); } @Test diff --git a/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml b/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml index 278f3fef3d..4c8de3eede 100644 --- a/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml +++ b/spring-websocket/src/test/resources/org/springframework/web/socket/config/websocket-config-broker-customchannels-default-executor.xml @@ -1,16 +1,14 @@ - - - + - @@ -18,13 +16,15 @@ - + + + + + - -