New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MessagePostprocessor seems not to get called on reply #719

Closed
konraifen88 opened this Issue Mar 9, 2018 · 5 comments

Comments

Projects
None yet
3 participants
@konraifen88

konraifen88 commented Mar 9, 2018

I am trying to invoke an MessagePostProcessor to apply/verify a header before sending/after receiving an message.

I have managed to add the PostProcessor functionality for following situations:

  • Before send (in publisher)
  • After receive (in publisher)
  • After receive (in consumer with an AdviceChain, but I do not think that this is the meant sollution)

But I am not able to set it up for sending reply. When looking into the code I can see that the reply is handled inside the AbstractAdaptableMessageListener where an replyPostProcessor is available.

But this replyPostProcessor never is set, so it is always null and because of that never executed.

Little bit more detail:
The AbstractAdaptableMessageListener is created in the MethodRabbitListenerEndpoint.createMessageListener method. After creating a new MessagingMessageListenerAdapter replyToAddress, messageConverter and beanResolver are set and the listener is returned. I think that here the replyPostProcessor also should be set, too.

But I do not have this great understanding of the code to say this for sure.

The version I am using is 1.7.6.RELEASE

@artembilan

This comment has been minimized.

Show comment
Hide comment
@artembilan

artembilan Mar 9, 2018

Member

Yeah... Looks like this is an omission on the Framework side.

Has to be fixed.

As a workaround I suggest to return the whole org.springframework.amqp.core.Message from your @RabbitListener method.

Or extend SimpleRabbitListenerContainerFactory and in the end of overloaded initializeContainer call the SimpleMessageListenerContainer.getListener(), cast it to the AbstractAdaptableMessageListener and then inject MessagePostProcessor via the mentioned setReplyPostProcessor().

Member

artembilan commented Mar 9, 2018

Yeah... Looks like this is an omission on the Framework side.

Has to be fixed.

As a workaround I suggest to return the whole org.springframework.amqp.core.Message from your @RabbitListener method.

Or extend SimpleRabbitListenerContainerFactory and in the end of overloaded initializeContainer call the SimpleMessageListenerContainer.getListener(), cast it to the AbstractAdaptableMessageListener and then inject MessagePostProcessor via the mentioned setReplyPostProcessor().

@garyrussell

This comment has been minimized.

Show comment
Hide comment
@garyrussell

garyrussell Mar 9, 2018

Member

After receive (in consumer with an AdviceChain, but I do not think that this is the meant sollution)

In 2.0 we added setAfterReceivePostProcessors to the container factory.

If we fix this (replyMPP) in 1.7.x, we should probably back port that too.

Member

garyrussell commented Mar 9, 2018

After receive (in consumer with an AdviceChain, but I do not think that this is the meant sollution)

In 2.0 we added setAfterReceivePostProcessors to the container factory.

If we fix this (replyMPP) in 1.7.x, we should probably back port that too.

@konraifen88

This comment has been minimized.

Show comment
Hide comment
@konraifen88

konraifen88 Mar 11, 2018

Thanks for the quick reply. For the moment I've managed to do my post processing during, or better before/after the message conversion. For that I've created my own Jackson2JsonMessageConverter and add/process my header in this step.

Edit:
Would it be possible to have 2 different interfaces for the message post processing. Say an MessageReceivePostProcessor and a MessageSendPostProcessor. Then it would be possible to create Beans of the postProcessors and they will be added to the correct places (send, reply, receive).
But I think this will be an new feature request and "off-topic" for this issue.

konraifen88 commented Mar 11, 2018

Thanks for the quick reply. For the moment I've managed to do my post processing during, or better before/after the message conversion. For that I've created my own Jackson2JsonMessageConverter and add/process my header in this step.

Edit:
Would it be possible to have 2 different interfaces for the message post processing. Say an MessageReceivePostProcessor and a MessageSendPostProcessor. Then it would be possible to create Beans of the postProcessors and they will be added to the correct places (send, reply, receive).
But I think this will be an new feature request and "off-topic" for this issue.

@garyrussell

This comment has been minimized.

Show comment
Hide comment
@garyrussell

garyrussell Mar 12, 2018

Member

@konraifen88 - please open a JIRA "Improvement" issue against Spring AMQP; when that is implemented, we'd then need a change to Spring Boot, to auto configure them. They can probably be simple "marker" interfaces that extend MessagePostProcessor.

This would have to be a 2.1 feature (in both cases), I'm afraid, unless you feel the current situation is over-burdensome.

Member

garyrussell commented Mar 12, 2018

@konraifen88 - please open a JIRA "Improvement" issue against Spring AMQP; when that is implemented, we'd then need a change to Spring Boot, to auto configure them. They can probably be simple "marker" interfaces that extend MessagePostProcessor.

This would have to be a 2.1 feature (in both cases), I'm afraid, unless you feel the current situation is over-burdensome.

garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Mar 12, 2018

spring-projectsGH-719: Container Factory: Add replyPostProcessor
Resolves spring-projects#719

Add `replyPostProcessor` to the container factory.

garyrussell added a commit to garyrussell/spring-amqp that referenced this issue Mar 12, 2018

spring-projectsGH-719: Container Factory: Add replyPostProcessor
Resolves spring-projects#719

Add `replyPostProcessor` to the container factory.
@konraifen88

This comment has been minimized.

Show comment
Hide comment
@konraifen88

konraifen88 Mar 13, 2018

Thanks for the fast fix here.

Like you told I opened up an improvement issue (AMQP-803).

Edit:
Sorry hit the wrong button, didn't want to close this issue.

konraifen88 commented Mar 13, 2018

Thanks for the fast fix here.

Like you told I opened up an improvement issue (AMQP-803).

Edit:
Sorry hit the wrong button, didn't want to close this issue.

@konraifen88 konraifen88 reopened this Mar 13, 2018

artembilan added a commit that referenced this issue Mar 13, 2018

GH-719: Container Factory: Add replyPostProcessor
Resolves #719

Add `replyPostProcessor` to the container factory.

* Polishing - support an array of MPPs for consistency with other places.

* Polishing - PR Comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment