You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@camel.apache.org by "Claus Ibsen (JIRA)" <ji...@apache.org> on 2009/05/22 20:58:50 UTC

[jira] Created: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

AggregatorStrategy - should also be invoked for the first exchange
------------------------------------------------------------------

                 Key: CAMEL-1638
                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
             Project: Apache Camel
          Issue Type: Improvement
          Components: camel-core
    Affects Versions: 1.6.1, 2.0-M1
            Reporter: Claus Ibsen
            Assignee: Claus Ibsen
             Fix For: 2.0.0


Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.

This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
And its also a bit confusing as well why the first one is skipped.

And if you aggregator out of order then you dont know which one was the first exchange.

So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.


-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Willem Jiang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51858#action_51858 ] 

Willem Jiang commented on CAMEL-1638:
-------------------------------------

I can't agree with your more on the standard of giving the full power to the end user :) 

Let's call the aggregate method when the aggregator gets the exchange each time.

BTW , We also need to update the comments and change log for it.


> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51857#action_51857 ] 

Claus Ibsen commented on CAMEL-1638:
------------------------------------

But Willem you would like to do you code in one place.

So if you need to count the number of aggregated methods, or do some custom changes on all aggregated exception you can do all that in the same method.

If we skip the very first exchange then you cannot do that. And if you had to do a visit pattern afterwards that might be to late as you have already aggregated the incoming exchanges. So its a chicken and egg situation.

So IMHO the best solution is to also pass in the first exchange in this callback. That gives full power to the end user.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51864#action_51864 ] 

Claus Ibsen commented on CAMEL-1638:
------------------------------------

Wiki updated.

And using
{code}
if (oldExchange == null) {
  return newExchange;
}
{code}

Applies as well for Camel 1.x as basically no harm is done, as oldExchange is newer null in Camel 1.x

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Resolved: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Claus Ibsen resolved CAMEL-1638.
--------------------------------

    Resolution: Fixed

Well spotted Willem.

I have updated the javadoc on AggregationStrategy and added a note on the camel 2.0 release page about API changes.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51863#action_51863 ] 

Claus Ibsen commented on CAMEL-1638:
------------------------------------

Ups will update the Aggregate EIP wiki page as well.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Willem Jiang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51846#action_51846 ] 

Willem Jiang commented on CAMEL-1638:
-------------------------------------

How about adding a callback method for visiting the exchange in AggregatorStragtegy ?
And we can get ride of the annoying  old Exchange "null" checking.
 

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Willem Jiang (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51856#action_51856 ] 

Willem Jiang commented on CAMEL-1638:
-------------------------------------

This call back API gives us enough flexibilities for aggregation
{noformat} 
public Exchange aggregate(Exchange oldExchange, Exchange newExchange)
{noformat} 
I just think if we want to visit the all the exchanges,  we could introduce a
 visit method to look up these exchanges without write a customer aggregator.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51854#action_51854 ] 

Claus Ibsen commented on CAMEL-1638:
------------------------------------

trunk: 777808.

Willem this is still up for discussion, even though I committed a bunch of stuff.

I have also thought of adding a {{boolean first}} parameter to indicate its the very first call.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51847#action_51847 ] 

Claus Ibsen commented on CAMEL-1638:
------------------------------------

Willem could you explain a bit more?

I would also like the AggregationStrategy to be a single method to implement as thats easier for anonym inner classes and also for languages with closure to support more nicely.

I agree we have a catch-22 situation.

If we leave it like the old way then we loose that first callback, that
- takes end users by the surprise as if they aggregate 10 exchanges they would expect 10 callbacks
- you lose 1 callback and have to deal with the situation how to remedy this by eg adding a special "boolean" flag on the exchange that you have
  done your own "i missed the first exchange fixup".

With the new way, we get
- all 10 callbacks is executed and end users is not surprised
- however the first callback have {{null}} for oldExchange

BTW: As a side note I am speculating how to allow a POJO to be AggregatorStrategy so we dont have to use Camel API at all. But I will create a new ticket for that as well.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Work started: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Work on CAMEL-1638 started by Claus Ibsen.

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.


[jira] Commented: (CAMEL-1638) AggregatorStrategy - should also be invoked for the first exchange

Posted by "Claus Ibsen (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/activemq/browse/CAMEL-1638?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=51848#action_51848 ] 

Claus Ibsen commented on CAMEL-1638:
------------------------------------

Willem it isnt that bad with the null check. Not all implementations suffer for that. Check this out

*before*
{code}
    public Exchange aggregate(Exchange oldExchange, Exchange newExchange) {
        Exchange copy = newExchange.copy();
        Message newIn = copy.getIn();
        String oldBody = oldExchange.getIn().getBody(String.class);
        String newBody = newIn.getBody(String.class);
        newIn.setBody(oldBody + "+" + newBody);
        return copy;
    }
{code}

*after*
{code}
    public Exchange aggregate(Exchange oldExchange, Exchange newExchange) {
        if (oldExchange != null) {
            String oldBody = oldExchange.getIn().getBody(String.class);
            String newBody = newExchange.getIn().getBody(String.class);
            newExchange.getIn().setBody(oldBody + "+" + newBody);
        }
        return newExchange;
    }
{code}

The code in after is also very readable. 

> AggregatorStrategy - should also be invoked for the first exchange
> ------------------------------------------------------------------
>
>                 Key: CAMEL-1638
>                 URL: https://issues.apache.org/activemq/browse/CAMEL-1638
>             Project: Apache Camel
>          Issue Type: Improvement
>          Components: camel-core
>    Affects Versions: 2.0-M1, 1.6.1
>            Reporter: Claus Ibsen
>            Assignee: Claus Ibsen
>             Fix For: 2.0.0
>
>
> Using aggregator, splitterl, multicast etc. that supports {{AggregatorStrategy}} all have the flaw that they skip invoking this interface callback for the very first exchange.
> This hazzles the end users implementing this interface if they are to be able to traverse all the exchanges, to eg summing a total amount or the likes.
> And its also a bit confusing as well why the first one is skipped.
> And if you aggregator out of order then you dont know which one was the first exchange.
> So we should invoke this callback always. And the first time the {{oldExchange}} parameter is null.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.