From 61e959377dad4217e9c9069f7e0a2b81601e37b4 Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Fri, 20 Feb 2015 14:04:48 +0200 Subject: [PATCH] INT-2230: Fix for Double Global Wire-Taps JIRA: https://jira.spring.io/browse/INT-2230 Previously two global ``s with the same `id` registered two `GlobalChannelInterceptorWrapper` wrappers with generated names for the same (last) `WireTap`. Hence the same `Message` has been sent to the the `wire-tap channel` twice (or more). Fix the issue with overriding `GlobalChannelInterceptorParser#resolveId` with the `.globalChannelInterceptor` suffix. Therefore the last `GlobalChannelInterceptorWrapper` wins overriding all others, if all our ``s has the same `id`. INT-2230: Add `id` to the global `channel-interceptor` Polishing --- .../xml/GlobalChannelInterceptorParser.java | 4 +- .../config/xml/GlobalWireTapParser.java | 17 ++- .../config/xml/spring-integration-4.2.xsd | 1 + ...nnelInterceptorSubElementTests-context.xml | 21 ++-- ...obalChannelInterceptorSubElementTests.java | 32 ++++-- .../GlobalWireTapTests-context.xml | 72 +++++++----- .../interceptor/GlobalWireTapTests.java | 103 ++++++++++-------- 7 files changed, 147 insertions(+), 103 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalChannelInterceptorParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalChannelInterceptorParser.java index ba92cc4c85..63bf1c74b5 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalChannelInterceptorParser.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalChannelInterceptorParser.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. @@ -48,7 +48,7 @@ public class GlobalChannelInterceptorParser extends AbstractBeanDefinitionParser private static final String REF_ATTRIBUTE = "ref"; @Override - protected boolean shouldGenerateId() { + protected boolean shouldGenerateIdAsFallback() { return true; } diff --git a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalWireTapParser.java b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalWireTapParser.java index 89a398dde4..f9b9c95532 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalWireTapParser.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/config/xml/GlobalWireTapParser.java @@ -1,11 +1,11 @@ /* - * Copyright 2002-2011 the original author or authors. - * + * Copyright 2002-2015 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. You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. @@ -15,12 +15,15 @@ package org.springframework.integration.config.xml; import org.w3c.dom.Element; +import org.springframework.beans.factory.BeanDefinitionStoreException; import org.springframework.beans.factory.config.RuntimeBeanReference; +import org.springframework.beans.factory.support.AbstractBeanDefinition; import org.springframework.beans.factory.xml.ParserContext; /** * Parser for the top level 'wire-tap' element * @author David Turanski + * @author Artem Bilan * @since 2.1 * */ @@ -32,4 +35,10 @@ public class GlobalWireTapParser extends GlobalChannelInterceptorParser { return new RuntimeBeanReference(wireTapBeanName); } + @Override + protected String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext) + throws BeanDefinitionStoreException { + return super.resolveId(element, definition, parserContext) + ".globalChannelInterceptor"; + } + } diff --git a/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-4.2.xsd b/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-4.2.xsd index 5b35a311b3..0942ec39b8 100644 --- a/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-4.2.xsd +++ b/spring-integration-core/src/main/resources/org/springframework/integration/config/xml/spring-integration-4.2.xsd @@ -4079,6 +4079,7 @@ + diff --git a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests-context.xml b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests-context.xml index 54446b5380..efebd974b3 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests-context.xml +++ b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests-context.xml @@ -2,22 +2,27 @@ - + + + + + + + + + - + - + - + diff --git a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests.java b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests.java index 09e6deaa44..93e3d44b9c 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalChannelInterceptorSubElementTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2011 the original author or authors. + * Copyright 2002-2015 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. @@ -18,43 +18,51 @@ package org.springframework.integration.channel.interceptor; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import org.junit.Test; import org.junit.runner.RunWith; + import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.annotation.Qualifier; -import org.springframework.context.ApplicationContext; import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.PollableChannel; import org.springframework.messaging.support.GenericMessage; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; /** * @author David Turanski + * @author Artem Bilan * @since 2.0 */ @RunWith(SpringJUnit4ClassRunner.class) @ContextConfiguration +@DirtiesContext public class GlobalChannelInterceptorSubElementTests { - @Autowired - ApplicationContext applicationContext; - - + @Autowired @Qualifier("inputA") MessageChannel inputA; - + @Autowired @Qualifier("wiretap") PollableChannel wiretapChannel; - + + @Autowired + @Qualifier("wiretap1") + PollableChannel wiretap1; + @Test - public void testWiretapSubElement(){ - inputA.send(new GenericMessage("hello")); - Message result = wiretapChannel.receive(100); + public void testWiretapSubElement() { + this.inputA.send(new GenericMessage("hello")); + Message result = this.wiretapChannel.receive(100); assertNotNull(result); - assertEquals("hello",result.getPayload()); + assertEquals("hello", result.getPayload()); + assertNull(this.wiretapChannel.receive(1)); + assertNull(this.wiretap1.receive(1)); } + } diff --git a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests-context.xml b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests-context.xml index 9cca5ba0f2..9af837a4fc 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests-context.xml +++ b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests-context.xml @@ -1,34 +1,46 @@ - - - - - - - - - - - - - - - - - - - - - - - - - - - + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests.java b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests.java index fe4e2ce7e2..8739208c48 100644 --- a/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests.java +++ b/spring-integration-core/src/test/java/org/springframework/integration/channel/interceptor/GlobalWireTapTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2015 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. You may obtain a copy of the License at @@ -10,6 +10,7 @@ * an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the * specific language governing permissions and limitations under the License. */ + package org.springframework.integration.channel.interceptor; import static org.junit.Assert.assertEquals; @@ -25,77 +26,85 @@ import org.springframework.messaging.Message; import org.springframework.messaging.MessageChannel; import org.springframework.messaging.PollableChannel; import org.springframework.messaging.support.GenericMessage; +import org.springframework.test.annotation.DirtiesContext; import org.springframework.test.context.ContextConfiguration; import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; + /** * * @author David Turanski * @author Gary Russell - * + * @author Artem Bilan */ @ContextConfiguration @RunWith(SpringJUnit4ClassRunner.class) +@DirtiesContext public class GlobalWireTapTests { - @Autowired - @Qualifier("channel") - MessageChannel channel; + @Autowired + @Qualifier("channel") + MessageChannel channel; - @Autowired - @Qualifier("random-channel") - MessageChannel anotherChannel; + @Autowired + @Qualifier("random-channel") + MessageChannel anotherChannel; + @Autowired + @Qualifier("wiretap-single") + PollableChannel wiretapSingle; - @Autowired - @Qualifier("wiretap-single") - PollableChannel wiretapSingle; + @Autowired + @Qualifier("wiretap-all") + PollableChannel wiretapAll; - @Autowired - @Qualifier("wiretap-all") - PollableChannel wiretapAll; + @Autowired + @Qualifier("wiretap-all2") + PollableChannel wiretapAll2; + @Test + public void testWireTapsOnTargetedChannel() { + Message message = new GenericMessage("hello"); + this.channel.send(message); + Message wireTapMessage = this.wiretapSingle.receive(100); + assertNotNull(wireTapMessage); - @Test - public void testWireTapsOnTargetedChannel(){ - Message message = new GenericMessage("hello"); - channel.send(message); - Message wireTapMessage = wiretapSingle.receive(100); - assertNotNull(wireTapMessage); + // There should be three messages on this channel. + // One for 'channel', one for 'output', and one for 'wiretapSingle'. + wireTapMessage = this.wiretapAll.receive(100); + int msgCount = 0; + while (wireTapMessage != null) { + msgCount++; + assertEquals(wireTapMessage.getPayload(), message.getPayload()); + wireTapMessage = this.wiretapAll.receive(100); + } - //There should be three messages on this channel. One for 'channel', one for 'output', and one for 'wiretapSingle'. - wireTapMessage = wiretapAll.receive(100); - int msgCount=0; - while (wireTapMessage != null){ - msgCount++; - assertEquals(wireTapMessage.getPayload(),message.getPayload()); - wireTapMessage = wiretapAll.receive(100); - } + assertEquals(3, msgCount); - assertEquals(3,msgCount); - } + assertNull(this.wiretapAll2.receive(1)); + } - @Test - public void testWireTapsOnRandomChannel(){ - Message message = new GenericMessage("hello"); - anotherChannel.send(message); + @Test + public void testWireTapsOnRandomChannel() { + Message message = new GenericMessage("hello"); + anotherChannel.send(message); - //This time no message on wiretapSingle - Message wireTapMessage = wiretapSingle.receive(100); - assertNull(wireTapMessage); + //This time no message on wiretapSingle + Message wireTapMessage = wiretapSingle.receive(100); + assertNull(wireTapMessage); - //There should be two messages on this channel. One for 'channel' and one for 'output' - wireTapMessage = wiretapAll.receive(100); - int msgCount=0; - while (wireTapMessage != null){ - msgCount++; - assertEquals(wireTapMessage.getPayload(),message.getPayload()); - wireTapMessage = wiretapAll.receive(100); - } + //There should be two messages on this channel. One for 'channel' and one for 'output' + wireTapMessage = wiretapAll.receive(100); + int msgCount = 0; + while (wireTapMessage != null) { + msgCount++; + assertEquals(wireTapMessage.getPayload(), message.getPayload()); + wireTapMessage = wiretapAll.receive(100); + } - assertEquals(2,msgCount); - } + assertEquals(2, msgCount); + } }