Details
-
Type:
New Feature
-
Status: Closed
-
Priority:
Major
-
Resolution: Complete
-
Affects Version/s: None
-
Fix Version/s: 2.2 M4 Sprint 4, 2.2 M4
-
Component/s: Core
-
Labels:
-
Reference URL:
Issue Links
Activity
- All
- Comments
- Work Log
- History
- Activity
- Transitions
Hi Artem,
We were just talking about this; we were thinking maybe just transactional is ok for now, and we can add advice chain later, if someone has a use case. However, if you have it already, go ahead and issue a PR and we can take a look.
Gary Russell
added a comment - Hi Artem,
We were just talking about this; we were thinking maybe just transactional is ok for now, and we can add advice chain later, if someone has a use case. However, if you have it already, go ahead and issue a PR and we can take a look. Thank you, guys, for your attention.
Issue PR is here: https://github.com/SpringSource/spring-integration/pull/546
Artem Bilan
added a comment - Thank you, guys, for your attention.
Issue PR is here: https://github.com/SpringSource/spring-integration/pull/546 Hi, Gary!
Thank you for merging my PR, but I was changing my mind and was going to rework dalayer's 'advice-chain' ability according to your 'request-handler-advice-chain'.
And also I was planing to remove <transactional> as you didn't provide it in the 'request-handler-advice-chain'.
However If you think that my work OK with some your polishing, let it be.
Now a little question: Does it make sense to mention this changes in the Manual and the Migration Guide?
And my apologies that I stop contributing intensively.
Artem Bilan
added a comment - Hi, Gary!
Thank you for merging my PR, but I was changing my mind and was going to rework dalayer's 'advice-chain' ability according to your 'request-handler-advice-chain'.
And also I was planing to remove <transactional> as you didn't provide it in the 'request-handler-advice-chain'.
However If you think that my work OK with some your polishing, let it be.
Now a little question: Does it make sense to mention this changes in the Manual and the Migration Guide?
And my apologies that I stop contributing intensively. Hi Artem,
No; providing the ability to making the delayer's "dispatcher" thread transactional is completely orthogonal to the <request-handler-advice-chain/> implementation. If I had added that feature to the delayer, it would apply to the storing of the delayed message only, and nothing to do with dispatching the delayed message later.
Also, the <request-handler-advice-chain /> is not intended to make the handleRequestMessage() transactional, it is to add advices such as expression evaluating (after success or failure) against the inbound message - such as to remove or move a file payload; circuit breaker advice; retry advice. Of course, someone could add a tx advice, but that is certainly not the intent of the feature.
Note: I did not add <request-handler-advice-chain/> to the delayer; at the time, I didn't think it made sense. I suppose one could make an argument that a retry advice might be useful when using, say, a JDBC message store. If you think it's important, open a JIRA (or we can wait until someone asks for it). Note that, while merging, I changed your property to 'delayedAdviceChain' to avoid collision with the ARPMH superclass property (which is not currently exposed on the delay handler).
Gary Russell
added a comment - Hi Artem,
No; providing the ability to making the delayer's "dispatcher" thread transactional is completely orthogonal to the <request-handler-advice-chain/> implementation. If I had added that feature to the delayer, it would apply to the storing of the delayed message only, and nothing to do with dispatching the delayed message later.
Also, the <request-handler-advice-chain /> is not intended to make the handleRequestMessage() transactional, it is to add advices such as expression evaluating (after success or failure) against the inbound message - such as to remove or move a file payload; circuit breaker advice; retry advice. Of course, someone could add a tx advice, but that is certainly not the intent of the feature.
Note: I did not add <request-handler-advice-chain/> to the delayer; at the time, I didn't think it made sense. I suppose one could make an argument that a retry advice might be useful when using, say, a JDBC message store. If you think it's important, open a JIRA (or we can wait until someone asks for it). Note that, while merging, I changed your property to 'delayedAdviceChain' to avoid collision with the ARPMH superclass property (which is not currently exposed on the delay handler).
OK, Gary.
Thank you very much for your clarification.
I agree with you that now <request-handler-advice-chain/> doesn't make sense to DelayHandler#handleRequestMessage().
But who know what people will come up... Time will tell.
Nevertheless we should keep in mind: now we "advice" the relaese message thread, but the last one invokes handleRequestMessage() in the end. And people may be a bit confused that message-flow has different behavior when Message is delayed and when not.
Artem Bilan
added a comment - OK, Gary.
Thank you very much for your clarification.
I agree with you that now <request-handler-advice-chain/> doesn't make sense to DelayHandler#handleRequestMessage() .
But who know what people will come up... Time will tell.
Nevertheless we should keep in mind: now we "advice" the relaese message thread, but the last one invokes handleRequestMessage() in the end. And people may be a bit confused that message-flow has different behavior when Message is delayed and when not. 
Hi, all
A little question about consistency: there is only one method (releaseMessage) in the DelayHandler which is invoked in the separate thread and which initiates followed message-flow via send Message into output-channel. So, only this method and his thread need to be wrapped with transaction.
Will it be OK if we'll use here just TransactionTemplate around DelayHandler#releaseMessage instead of TransactionInterceptor and proxying of entire DelayHandler as it done for JpaOutboundGateway ?