Spring Integration
  1. Spring Integration
  2. INT-639

<chain/> doesn't work with annotated handlers

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Works as Designed
    • Affects Version/s: 1.0.2
    • Fix Version/s: 2.2 RC1
    • Component/s: None
    • Labels:
      None

      Description

      I've created a little testcase where a chain is wired with an annotated bean (@Aggregator) it fails with the following exception:

      Caused by: java.lang.IllegalArgumentException: Cannot convert value of type [org.springframework.integration.config.chain.AnnotatedAggregator] to required type [org.springframework.integration.message.MessageHandler] for property 'handlers[0]': no matching editors or conversion strategy found
      at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:231)
      at org.springframework.beans.TypeConverterDelegate.convertToTypedCollection(TypeConverterDelegate.java:464)
      at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:190)
      at org.springframework.beans.TypeConverterDelegate.convertIfNecessary(TypeConverterDelegate.java:138)
      at org.springframework.beans.BeanWrapperImpl.convertForProperty(BeanWrapperImpl.java:386)
      ... 50 more

        Activity

        Hide
        Iwein Fuld added a comment -

        A project level patch to show this behavior. If you spot a mistake please comment.

        Show
        Iwein Fuld added a comment - A project level patch to show this behavior. If you spot a mistake please comment.
        Hide
        Oleg Zhurakousky added a comment -

        I am moving it to 2.2 since it is a bit more complicated than I thought for several reasons:

        1. Chain implies order and that is why it works very well with XML based configuration because XML implies single handler while annotated components can have several methods where each is a handler.
        For example:

        @Component
        public class Barista {
        
           @ServiceActivator(inputChannel="hotDrinkBarista", outputChannel="preparedDrinks")
           public Drink prepareHotDrink(OrderItem orderItem) {..}
                        
           @ServiceActivator(inputChannel="coldDrinkBarista", outputChannel="preparedDrinks")
           public Drink prepareColdDrink(OrderItem orderItem) {..}
        }
        

        2. Annotated components can have a combination of channel configurations and other attributes via annotation and/or XML while with XML name-space we went an extra mile to ensure that what you can configure outside of the chain (e.g., channels) can not be configured in the chain and we have yet to do it for Annotation and now I wonder if we even should.
        In fact here is an example of one of the private methods which is part of Annotation bootstrap:

        private boolean shouldCreateEndpoint(Annotation annotation) {
                Object inputChannel = AnnotationUtils.getValue(annotation, "inputChannel");
                return (inputChannel != null && inputChannel instanceof String
                                        && StringUtils.hasText((String) inputChannel));
        }
        

        Obviously in the chain not only inputChannel would not be required it should also be prohibited

        May be some type of @Chain??? we'll see.

        With Scala DSL on the way the question might be if its worth doing at all

        Show
        Oleg Zhurakousky added a comment - I am moving it to 2.2 since it is a bit more complicated than I thought for several reasons: 1. Chain implies order and that is why it works very well with XML based configuration because XML implies single handler while annotated components can have several methods where each is a handler. For example: @Component public class Barista { @ServiceActivator(inputChannel= "hotDrinkBarista" , outputChannel= "preparedDrinks" ) public Drink prepareHotDrink(OrderItem orderItem) {..} @ServiceActivator(inputChannel= "coldDrinkBarista" , outputChannel= "preparedDrinks" ) public Drink prepareColdDrink(OrderItem orderItem) {..} } 2. Annotated components can have a combination of channel configurations and other attributes via annotation and/or XML while with XML name-space we went an extra mile to ensure that what you can configure outside of the chain (e.g., channels) can not be configured in the chain and we have yet to do it for Annotation and now I wonder if we even should. In fact here is an example of one of the private methods which is part of Annotation bootstrap: private boolean shouldCreateEndpoint(Annotation annotation) { Object inputChannel = AnnotationUtils.getValue(annotation, "inputChannel" ); return (inputChannel != null && inputChannel instanceof String && StringUtils.hasText(( String ) inputChannel)); } Obviously in the chain not only inputChannel would not be required it should also be prohibited May be some type of @Chain??? we'll see. With Scala DSL on the way the question might be if its worth doing at all
        Hide
        Oleg Zhurakousky added a comment -

        Artem, can you take a look at this one. As you can see its been sitting out there for a while and I figured since you have experienced similar problem with adapter in the chain you may take care of this one as well.

        Show
        Oleg Zhurakousky added a comment - Artem, can you take a look at this one. As you can see its been sitting out there for a while and I figured since you have experienced similar problem with adapter in the chain you may take care of this one as well.
        Hide
        Artem Bilan added a comment -

        Before I start describe my investigations, let me say note about annotation configuration.
        On the practices there are not much usage of "Annotation DSL" for configuration message flows, e.g.

        @ServiceActivator(inputChannel="inputChannel1", outputChannel= "outputChannel1")
        

        In most cases it is simple @ServiceActivator on the method and that's all. It allows to minimize a bit xml-config:

        <service-activator ref="serviceActivatorWithAnnotatedMethod"/>
        

        Configure flow with annotations doesn't follow with KISS principle: our message-driven architecture is stretched to different methods in different classes throughout the project. With such configuration it isn't so clear to understand how our message will travel.
        Many guys still ask: "Why we need Spring Integration if we already have Apache Camel, which does the same and allows to configure transparent message flow?"
        Community want to see everything in one place: xml, Scala, Groovy. And Also they often ask about Java DSL as it is in the Camel. I think we have it already - with annotations:

        @Component
        public class SpringIntegrationAnnotationConfig {
        
        	public static interface FlowGateway {
        
        		@Gateway(requestChannel = "inputChannel")
        		Object gateway(Object payload);
        	}
        	
        	@ServiceActivator(inputChannel="inputChannel", outputChannel="transformChannel")
        	public Object service(Object param) {
        		...
        	}
        
        	@Transformer(inputChannel="transformChannel", outputChannel="routeChannel")
        	public Object transform(Object param) {
        		...
        	}
        	
        	@Router(inputChannel="routeChannel")
        	public Object route(Object param) {
        		...
        	}
        }
        

        In the most cases it looks like procedural programming: we combine a lot of auto-created components around one class. I don't like it at all. Here we don't follow with loose-coupling principle. However I like @Publisher - it's realy an aspect
        And now about problem described here.
        In the first: Iwein's sample. As we see in the StackTrace <chain> requires, that his sub-components should be MessageHandler. So, the first thought that comes: add implements MessageHandler to Iwein's AnnotatedAggregator. But then @Aggregator will be redundant.
        But we know that tag <aggregator> understands this annotation. So, we change our config to:

        <chain input-channel="input" output-channel="output">
        <aggregator>
          <beans:bean class="org.springframework.integration.config.chain.AnnotatedAggregator" />
        </aggregator>
        </chain>
        

        And everything works OK! Ease, clear, consistent. Now we see in the <chain> config what's going on with our Message.
        In the second: we can add to ChainParser#parseChild logic about parsing "Annotation DSL", which will produce real MessageHandler, but in the end our <chain> will be like this:

        <chain>
         <beans:bean class="AnnotatedFilter" />
         <beans:bean class="AnnotatedServiceActivator" />
         <beans:bean class="AnnotatedTransformer" />
         <beans:bean class="AnnotatedAggregator" />
         <beans:bean class="AnnotatedRouter" />
        </chain>
        

        Then we can go further: move all annotated methods into one Class and parse them all in the ChainParser with special order. So, our config will be:

        <chain>
         <beans:bean class="AnnotatedMegaEndpoint" />
        </chain>
        

        What does it mean at a glance? Not clear, not flexible, not loosely-coupled...

        So, I think this issue isn't a bug. It is normal configuration behavior, we should distinctly place inside the <chain> what we mean.
        It's about XML.
        Now about this one:

        May be some type of @Chain??? we'll see.

        I agree it may be interestingly to introduce something like this:

        @Chain(inputChannel="inputChannel")
        public class ChainConfig {
        
        	@ChainOrder(2)
        	@Autowired
        	@Qualifier("some.handler")
        	private MessageHandler someHandler;
        
        	@NestedChain(order = 3)
        	@Autowired
        	private ChainConfig2 chainConfig2;
        	
        	@ChainOrder(0)
        	@ServiceActivator
        	public Object service(Object param) {
        		...
        	}
        
        	@ChainOrder(1)
        	@Transformer
        	public Object transform(Object param) {
        		...
        	}
        
        	@ChainOrder(4)
        	@Router
        	public Object route(Object param) {
        		...
        	}
        }
        
        @Chain
        public class ChainConfig2 {
        
        	@ChainOrder(0)
        	@ServiceActivator
        	public Object service(Object param) {
        		...
        	}
        
        	@ChainOrder(1)
        	@ServiceActivator
        	public Object service2(Object param) {
        		...
        	}
        }
        

        But will it be so clear & flexible as XML?
        WDYT?

        And a snack: look into my commit and see what I've found about annotations process: https://github.com/artembilan/spring-integration/commit/c121f1b9a4fc3f4a3f6e827fdaf4b1f663e73fda
        1. AnnotatedEndpoint. We can combine several messaging annotation on the one method . DRY is in full force! But is it so loosely-coupled
        & flexible and KISS as we are saying about Spring Integration application architecture?
        2. AnnotatedEndpointTests-context.xml. We can mark our class with @MessageEndpoint and also we can define it as regular spring bean in the context. And also we can add <context:component-scan />. So, we have two beans of our AnnotatedEndpoint.
        3. thanks to <si:annotation-config/> we have an ability to register Messaging Endpoints in the context based on annotations config. But it is done twice: we have the same two beans of AnnotatedEndpoint. So we end up with two instances of endpoint on the same channel...
        In the case of dirrect channel, thanks to 'round-robin', it works OK, but if it is publish-subscribe-channel?.. And also if our annotated method throws Exception: in the end UnicastingDispatcher produces AggregateMessageDeliveryException with two identical exceptions...
        WDYT again?

        Show
        Artem Bilan added a comment - Before I start describe my investigations, let me say note about annotation configuration. On the practices there are not much usage of "Annotation DSL" for configuration message flows, e.g. @ServiceActivator(inputChannel= "inputChannel1" , outputChannel= "outputChannel1" ) In most cases it is simple @ServiceActivator on the method and that's all. It allows to minimize a bit xml-config: <service-activator ref= "serviceActivatorWithAnnotatedMethod" /> Configure flow with annotations doesn't follow with KISS principle: our message-driven architecture is stretched to different methods in different classes throughout the project. With such configuration it isn't so clear to understand how our message will travel. Many guys still ask: "Why we need Spring Integration if we already have Apache Camel, which does the same and allows to configure transparent message flow?" Community want to see everything in one place: xml, Scala, Groovy. And Also they often ask about Java DSL as it is in the Camel. I think we have it already - with annotations: @Component public class SpringIntegrationAnnotationConfig { public static interface FlowGateway { @Gateway(requestChannel = "inputChannel" ) Object gateway( Object payload); } @ServiceActivator(inputChannel= "inputChannel" , outputChannel= "transformChannel" ) public Object service( Object param) { ... } @Transformer(inputChannel= "transformChannel" , outputChannel= "routeChannel" ) public Object transform( Object param) { ... } @Router(inputChannel= "routeChannel" ) public Object route( Object param) { ... } } In the most cases it looks like procedural programming : we combine a lot of auto-created components around one class . I don't like it at all. Here we don't follow with loose-coupling principle. However I like @Publisher - it's realy an aspect And now about problem described here. In the first: Iwein's sample. As we see in the StackTrace <chain> requires, that his sub-components should be MessageHandler . So, the first thought that comes: add implements MessageHandler to Iwein's AnnotatedAggregator . But then @Aggregator will be redundant. But we know that tag <aggregator> understands this annotation. So, we change our config to: <chain input-channel= "input" output-channel= "output" > <aggregator> <beans:bean class= "org.springframework.integration.config.chain.AnnotatedAggregator" /> </aggregator> </chain> And everything works OK! Ease, clear, consistent. Now we see in the <chain> config what's going on with our Message. In the second: we can add to ChainParser#parseChild logic about parsing "Annotation DSL" , which will produce real MessageHandler , but in the end our <chain> will be like this: <chain> <beans:bean class= "AnnotatedFilter" /> <beans:bean class= "AnnotatedServiceActivator" /> <beans:bean class= "AnnotatedTransformer" /> <beans:bean class= "AnnotatedAggregator" /> <beans:bean class= "AnnotatedRouter" /> </chain> Then we can go further: move all annotated methods into one Class and parse them all in the ChainParser with special order. So, our config will be: <chain> <beans:bean class= "AnnotatedMegaEndpoint" /> </chain> What does it mean at a glance? Not clear, not flexible, not loosely-coupled... So, I think this issue isn't a bug. It is normal configuration behavior, we should distinctly place inside the <chain> what we mean. It's about XML. Now about this one: May be some type of @Chain??? we'll see. I agree it may be interestingly to introduce something like this: @Chain(inputChannel= "inputChannel" ) public class ChainConfig { @ChainOrder(2) @Autowired @Qualifier( "some.handler" ) private MessageHandler someHandler; @NestedChain(order = 3) @Autowired private ChainConfig2 chainConfig2; @ChainOrder(0) @ServiceActivator public Object service( Object param) { ... } @ChainOrder(1) @Transformer public Object transform( Object param) { ... } @ChainOrder(4) @Router public Object route( Object param) { ... } } @Chain public class ChainConfig2 { @ChainOrder(0) @ServiceActivator public Object service( Object param) { ... } @ChainOrder(1) @ServiceActivator public Object service2( Object param) { ... } } But will it be so clear & flexible as XML? WDYT? And a snack: look into my commit and see what I've found about annotations process: https://github.com/artembilan/spring-integration/commit/c121f1b9a4fc3f4a3f6e827fdaf4b1f663e73fda 1. AnnotatedEndpoint. We can combine several messaging annotation on the one method . DRY is in full force! But is it so loosely-coupled & flexible and KISS as we are saying about Spring Integration application architecture? 2. AnnotatedEndpointTests-context.xml. We can mark our class with @MessageEndpoint and also we can define it as regular spring bean in the context. And also we can add <context:component-scan /> . So, we have two beans of our AnnotatedEndpoint . 3. thanks to <si:annotation-config/> we have an ability to register Messaging Endpoints in the context based on annotations config. But it is done twice: we have the same two beans of AnnotatedEndpoint . So we end up with two instances of endpoint on the same channel... In the case of dirrect channel, thanks to 'round-robin', it works OK, but if it is publish-subscribe-channel ?.. And also if our annotated method throws Exception: in the end UnicastingDispatcher produces AggregateMessageDeliveryException with two identical exceptions... WDYT again?
        Hide
        Iwein Fuld added a comment -

        The only reason I ever logged the bug is because of the principle of least surprise. If you put annotated handlers in a chain you'd expect it to work. Magically. Somehow.

        It's not simple to get this to work nicely however.

        This issue is not about @Chain, it is about a <chain><beans:bean id="annotated" .../></chain>. Since it received no upvotes in the past three years, I'd opt for letting it hang here until someone actually bumps into it and complains.

        Show
        Iwein Fuld added a comment - The only reason I ever logged the bug is because of the principle of least surprise. If you put annotated handlers in a chain you'd expect it to work. Magically. Somehow. It's not simple to get this to work nicely however. This issue is not about @Chain, it is about a <chain><beans:bean id="annotated" .../></chain>. Since it received no upvotes in the past three years, I'd opt for letting it hang here until someone actually bumps into it and complains.
        Hide
        Artem Bilan added a comment - - edited

        Closed as 'Work as Designed', because the mentioned Exception in the Describtion really reflects what is wrong and what should be done to solve the config's problem.
        So, if we can configure <chain> just in the XML, its config should as clear as posible.
        And if we want to use some annotated handler inside the <chain> we should wrap it with tag which reflect annotated handler's mission. See comments above.

        Show
        Artem Bilan added a comment - - edited Closed as 'Work as Designed', because the mentioned Exception in the Describtion really reflects what is wrong and what should be done to solve the config's problem. So, if we can configure <chain> just in the XML, its config should as clear as posible. And if we want to use some annotated handler inside the <chain> we should wrap it with tag which reflect annotated handler's mission. See comments above.

          People

          • Assignee:
            Artem Bilan
            Reporter:
            Iwein Fuld
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: