Skip to content

Auto-configure a MessageConverter for Rabbit if one exists in the context #5088

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

Closed
0xSalman opened this issue Feb 4, 2016 · 11 comments
Closed
Assignees
Labels
type: enhancement A general enhancement
Milestone

Comments

@0xSalman
Copy link

0xSalman commented Feb 4, 2016

Hey there,

This is a feature request. Could you please add a property i.e., spring.rabbitmq.message-converter and use it to set message converter on rabbit template and listener when auto configuring them. Thank you

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 4, 2016
@wilkinsona
Copy link
Member

I'm not keen on using a property to configure a MessageConverter as it's not clear how the converter would be created. For example, what would happen if you want to use a ContentTypeDelegatingMessageConverter? How would you set its default converter or its delegates?

If you want to create a RabbitTemplate with a specific converter, you should declare your own @Bean for it. You can still use the auto-configured ConnectionFactory:

@Bean
public RabbitTemplate rabbitTemplate(ConnectionFactory connectionFactory) {
    RabbitTemplate template = new RabbitTemplate(connectionFactory);
    template.setMessageConverter(…);
    return template;
}

It's not so easy to configure a converter on the listener container as creating it isn't as straightforward. You could post-process the SimpleRabbitListenerContainerFactory bean, but that's not a very elegant solution. Perhaps there's something we can do to make it easier to customize. /cc @snicoll

@wilkinsona wilkinsona changed the title Property to set MessageConverter for RabbitTemplate & Listener Make it easier to customize the auto-configured SimpleRabbitListenerContainerFactory Feb 5, 2016
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 5, 2016
@snicoll
Copy link
Member

snicoll commented Feb 5, 2016

There is the same request for the JMS support and I was waiting for @ConditionalOnSingleCandidate to actually land in our code base first. The deal is that if there is only one MessageConverter we automatically associate it to the component we create.

How does that sound?

@snicoll snicoll added the status: waiting-for-feedback We need additional information before we can continue label Feb 12, 2016
@snicoll snicoll added this to the 1.4.0.M2 milestone Feb 12, 2016
@snicoll
Copy link
Member

snicoll commented Feb 12, 2016

See #4284

@spring-projects-issues
Copy link
Collaborator

If you would like us to look at this issue, please provide the requested information. If the information is not provided within the next 7 days this issue will be closed.

@spring-projects-issues spring-projects-issues added the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 19, 2016
@snicoll
Copy link
Member

snicoll commented Feb 19, 2016

I'll take that as a yes. I don't like the idea of providing a configuration entry for that anyway.

@snicoll snicoll removed the status: waiting-for-feedback We need additional information before we can continue label Feb 19, 2016
@0xSalman
Copy link
Author

@snicoll when you say a MessageConverter is available, I'm assuming you mean MessageConverter bean is available. I supposes it's a good work around. Thanks

@snicoll
Copy link
Member

snicoll commented Feb 20, 2016

Yes. I don't really see what you mean by workaround though.

@0xSalman
Copy link
Author

I was alluding towards the difference between configuring with a property vs creating a MessageConverter bean.

@snicoll
Copy link
Member

snicoll commented Feb 22, 2016

In that case, I think configuring via a property is a bad idea (programming by properties). Note that as of 1.3.3 there is a new service you can use to quickly create your own container factory, see RabbitListenerContainerFactoryConfigurer

@0xSalman
Copy link
Author

When you say configuring/programming via property is a bad idea, are you referring to this particular case or is that your take in general?

Thanks for pointing out RabbitListenerContainerFactoryConfigurer - it was helpful.

While configuring my own SimpleRabbitListenerContainerFactory, I noticed that RabbitAnnotationDrivenConfiguration is checking, whether this bean is missing, by name. Don't you think it would be better to use class reference (SimpleRabbitListenerContainerFactory.class) instead?

@snicoll snicoll removed the status: feedback-reminder We've sent a reminder that we need additional information before we can continue label Feb 22, 2016
@snicoll
Copy link
Member

snicoll commented Feb 22, 2016

Let's not start a chat here please, there's gitter for that. The check on name is on purpose as we are configuring the default factory and that's how the implementation looks it up .

@snicoll snicoll changed the title Make it easier to customize the auto-configured SimpleRabbitListenerContainerFactory Auto-configure a MessageConverter for Rabbit if one exists in the context Feb 29, 2016
@snicoll snicoll self-assigned this Feb 29, 2016
henrikerola pushed a commit to henrikerola/spring-boot that referenced this issue Mar 3, 2016
If a `MessageConverter` bean is available, we now associate it to the
created `RabbitTemplate` and `RabbitListenerContainerFactory`. That way,
registering a custom `MessageConverter` is all that's needed.

The Rabbit auto-configuration is now using the new `ObjectProvider` that
offers a nicer API to detect if a primary candidate is available for
optional collaborators.

Closes spring-projectsgh-5088
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

4 participants