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

Configure MessageSource if no "messageSource" bean defined #15212

Closed
wants to merge 1 commit into from

Conversation

Projects
None yet
4 participants
@cac03
Copy link
Contributor

commented Nov 19, 2018

Hello.

This PR fixes an issue with MessageSourceAutoConfiguration when a MessageSource bean with non standard name defined.

In particular ApplicationContext expects a MessageSource bean defined only with "messageSource" name, but MessageSourceAutoConfiguration does not convey the same policy.

So when a MessageSource is defined with non-standard name MessageSourceAutoConfiguration does not expose a MessageSource named "messageSource" and at the same time ApplicationContext does not use defined MessageSource and message resolution process fails.

See AbstractApplicationContext.initMessageSource().
It is also stated in the reference documentation:

When an ApplicationContext is loaded, it automatically searches for a MessageSource bean defined in the context. The bean must have the name messageSource. If such a bean is found, all calls to the preceding methods are delegated to the message source. If no message source is found, the ApplicationContext attempts to find a parent containing a bean with the same name. If it does, it uses that bean as the MessageSource. If the ApplicationContext cannot find any source for messages, an empty DelegatingMessageSource is instantiated in order to be able to accept calls to the methods defined above.

Project reproducing the issue.
See created tests. In particular:

Configure MessageSource if no "messageSource" bean defined
Enable MessageSourceAutoConfiguration @OnMissingBeanCondition
by name rather than class since AbstractApplicationContext expects
MessageSource to be defined only with "messageSource" name
@philwebb

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

Thanks for the PR. We think that this change makes sense but we'll target it to 2.2.x just in case someone has overridden AbstractApplicationContext.initMessageSource() to use different logic and is relying on the existing behavior.

@cac03

This comment has been minimized.

Copy link
Contributor Author

commented Nov 21, 2018

@philwebb thank you.
I'm curious to know if you discussed the idea to drop the requirement about MessageSource bean name in AbstractApplicationContext itself.
If yes, then did you decide to leave it as is because of other applications relying on this requirement?

@philwebb

This comment has been minimized.

Copy link
Member

commented Nov 21, 2018

@cac03 We didn't discuss removing that requirement. The AbstractApplicationContext is part of Spring Framework and I don't think they'd be able to make such a change because of back-compatibility concerns.

@snicoll

snicoll approved these changes Dec 3, 2018

@snicoll snicoll self-assigned this Dec 3, 2018

@snicoll snicoll modified the milestones: 2.2.x, 2.2.0.M1 Dec 3, 2018

snicoll added a commit that referenced this pull request Dec 3, 2018

Configure MessageSource if no "messageSource" bean defined
Enable MessageSourceAutoConfiguration OnMissingBeanCondition by name
rather than class since AbstractApplicationContext expects MessageSource
to be defined only with "messageSource" name.

See gh-15212

@snicoll snicoll closed this in ec678ea Dec 3, 2018

snicoll added a commit that referenced this pull request Dec 3, 2018

Merge pull request #15212 from cac03
* pr/15212:
  Polish "Configure MessageSource if no "messageSource" bean defined"
  Configure MessageSource if no "messageSource" bean defined
@snicoll

This comment has been minimized.

Copy link
Member

commented Dec 3, 2018

@cac03 thank you for making your first contribution to Spring Boot. This is now merged with a polish commit.

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.