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

MessageSourceAutoConfiguration relies on internal method call for BeanFactory.getBean() #13824

Closed
dsyer opened this issue Jul 19, 2018 · 7 comments
Assignees
Labels
theme: performance Issues related to general performance type: enhancement A general enhancement
Milestone

Comments

@dsyer
Copy link
Member

dsyer commented Jul 19, 2018

There might be other places. This is the only one I found so far. I assume there's a weird ordering issue that made it the way it is, but it would be nice to refactor it so that CGLib isn't strictly needed to create the beans in this class.

See also #9068

@dsyer dsyer added the theme: performance Issues related to general performance label Jul 19, 2018
@wilkinsona wilkinsona added the status: waiting-for-triage An issue we've not yet triaged label Jul 19, 2018
@dsyer
Copy link
Member Author

dsyer commented Jul 19, 2018

FWIW: this works for me:

	@Bean
	public MessageSource messageSource(MessageSourceProperties properties) {
		ResourceBundleMessageSource messageSource = new ResourceBundleMessageSource();

c.f. (from master):

	@Bean
	public MessageSource messageSource() {
		MessageSourceProperties properties = messageSourceProperties();
		ResourceBundleMessageSource messageSource = new ResourceBundleMessageSource();

@snicoll snicoll self-assigned this Jul 19, 2018
@snicoll
Copy link
Member

snicoll commented Jul 19, 2018

Yup, that's clearly not necessary.

@snicoll snicoll added this to the 2.1.0.M1 milestone Jul 19, 2018
@snicoll snicoll added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Jul 19, 2018
@snicoll
Copy link
Member

snicoll commented Jul 19, 2018

Or not ... #9666 (comment)

@dsyer
Copy link
Member Author

dsyer commented Jul 19, 2018

That comment appears to be wrong or out of date. Need to test with a custom MessageSource bean to make sure, but it seems to work for a custom messages.properties (as in Petclinic for instance) with the code above.

@snicoll
Copy link
Member

snicoll commented Jul 19, 2018

Yep, I've reached the same conclusion.

@snicoll
Copy link
Member

snicoll commented Jul 19, 2018

That being said, I don't think this was added by accident anymore. This auto-configuration has the following:

@ConditionalOnMissingBean(value = MessageSource.class, search = SearchStrategy.CURRENT)

Assuming a hierarchy of ApplicationContext where the parent is also configured via Spring Boot, the auto-configuration could kick-in in the parent and doing such injection allows to use the expected properties set. One might argue that the set will be exactly the same so it doesn't really matter.

Not entirely sure what to do here.

@snicoll snicoll removed this from the 2.1.0.M1 milestone Jul 19, 2018
@snicoll snicoll added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement and removed type: enhancement A general enhancement status: waiting-for-triage An issue we've not yet triaged labels Jul 19, 2018
@snicoll snicoll added this to the 2.1.0.M1 milestone Jul 19, 2018
@snicoll
Copy link
Member

snicoll commented Jul 19, 2018

That will work as the bean in the child will take precedence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
theme: performance Issues related to general performance type: enhancement A general enhancement
Projects
None yet
Development

No branches or pull requests

3 participants