Skip to content
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

JmsMessagingTemplate is not correctly configured [SPR-15965] #20517

Closed
spring-issuemaster opened this issue Sep 15, 2017 · 6 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Sep 15, 2017

Filip Hrisafov opened SPR-15965 and commented

The JmsMessagingTemplate does not set the payload MessageConverter from the JmsTemplate.

The RabbitMessagingTemplate sets the payload converter from the JmsTemplate. I think that the same needs to be done for the JmsMessagingTemplate as well.

I have also created an issue for Spring Boot #10305 and I was redirected here for the fix.


Affects: 4.3.11

Issue Links:

  • #20514 TaskExecutorRegistration.getTaskExecutor() overrides executor properties of a provided ThreadPoolTaskExecutor

Referenced from: commits b275a06, 35af7ff

Backported to: 4.3.12

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Stéphane Nicoll commented

Can we take a step back here please? I want to make sure that we're talking about the same problem.

Can you share the exact message converter you set in the JmsTemplate and what kind of outcome you expect if you pass that JmsTemplate to JmsMessagingTemplate?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Juergen Hoeller commented

Looking at this with Stéphane a bit further, we assume that this is about reusing a custom JmsTemplate-configured converter as a payload converter behind JmsMessagingTemplate's default MessagingMessageConverter, provided that no such adapter has been configured on the JmsMessagingTemplate itself. This is exactly what RabbitMessagingTemplate does indeed, and alignment is certainly desirable there. We can implement this the same way, tracking a local converterSet flag and passing the JmsTemplate converter on in JmsMessagingTemplate.afterPropertiesSet if necessary.

Let's address this for 5.0 GA and consider a backport to 4.3.12 (scheduled for a month later).

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Filip Hrisafov commented

Sorry, my example in the Spring Boot issue is wrong. After I posted it and I started looking more into it and how I should fix it for me (by providing my own implementation), I saw that doing it like with the RabbitMessagingTemplate is the correct way. I didn't update my initial code there (should I, so it is more clear for the future?)

So to sum up, what @Juergen wrote is correct. If the JmsTemplate has a converter set and the converter in the JmsMessagingTemplate has not been explicitly set the converter from the JmsTemplate should be used as the payload converter for the JmsMessagingTemplate. Basically the exact same logic as the RabbitMessagingTemplate has.

At the time of posting the issue, I didn't know that much about the different types of converters.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 18, 2017

Stéphane Nicoll commented

Juergen Hoeller I've pushed a fix on master. Would you please review and backport accordingly? Thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 19, 2017

Juergen Hoeller commented

This looks good to me; I'll backport it along with a few other commits tomorrow.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 19, 2017

Filip Hrisafov commented

Thanks a lot for doing this so fast, really appreciate it. Looking forward to the next 4.3.12 release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.