GH-2759: Fix CorrelationData.future #2761
Conversation
Fixes spring-projects#2759 The outbound endpoints wrap user correlation data in a wrapper. If the user data is a `CorrelationData`, we must delegate methods involving the `Future<?>` and `returnedMessage` to the user data. **cherry-pick to 2.1 and switch AMQP to snapshots**
|
|
||
| @Override | ||
| public org.springframework.amqp.core.Message getReturnedMessage() { | ||
| return super.getReturnedMessage(); |
There was a problem hiding this comment.
Looks like you are missing here something: since you delegate to the userData in the setReturnedMessage(), I guess you need to do the same here as well.
Otherwise this override just doesn't make sense.
There was a problem hiding this comment.
No, in the setter I call super too; but you are correct, this should be removed.
| this.correlationDataGenerator.processMessage(requestMessage), requestMessage); | ||
| Object userData = this.correlationDataGenerator.processMessage(requestMessage); | ||
| if (userData != null) { | ||
| correlationData = new CorrelationDataWrapper(messageId.toString(), userData, requestMessage); |
There was a problem hiding this comment.
Looks like previously we have had a CorrelationDataWrapper instance independently of the userData, but now you restrict it only if that one is not null.
Also I see that you are protected against null in the CorrelationDataWrapper any way.
So, what is the motivation to not do that any more?
There was a problem hiding this comment.
Because we get an NPE in handleConfirm() if the expression resolves to null (bug - try to create a message with a null payload).
I should have mentioned it in the commit comment and, I suppose, this part should be back-ported.
There was a problem hiding this comment.
Should we emit WARN then in case of correlationDataGenerator resolves to null?
There was a problem hiding this comment.
I decided to go with DEBUG (there is a corresponding DEBUG in handleConfirm).
Someone might want to use this as a feature ("I don't care about a confirm for this message").
|
Cherry-picked to Do we need to back-port further down to |
|
Yes, but just the NPE protection - the future was added in S-A 2.1. I can create a new PR if you prefer. |
|
I'm OK to back-port that part. Thanks for confirmation! |
**Cherry-pick to 4.3.x** Related to #2761
**Cherry-pick to 4.3.x** Related to #2761 # Conflicts: # spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AbstractAmqpOutboundEndpoint.java
|
Back-ported NPE fix to |
Fixes #2759
The outbound endpoints wrap user correlation data in a wrapper.
If the user data is a
CorrelationData, we must delegate methodsinvolving the
Future<?>andreturnedMessageto the user data.cherry-pick to 2.1 and switch AMQP to snapshots