Add support for ReloadableResourceBundleMessageSource #13377

ruifigueira · 2018-06-05T17:26:22Z

This PR adds support for ReloadableResourceBundleMessageSource, by adding a spring.messages.reloadable configuration property, which can be set to true to force the messageSource bean to be an instance of that class.

I'm currently working on a legacy Spring project that is progressively being migrated into Spring Boot, and we use a ReloadableResourceBundleMessageSource there, and we would like to replace it with the MessageSourceAutoConfiguration.

I'm well aware of #3039, and that:

The recommended approach since 1.3 is to use Devtools.

However, Devtools is not an option in this project, and even if it was, reloading all the application takes much more time than the ReloadableResourceBundleMessageSource takes to refresh.

I think that changes to enable ReloadableResourceBundleMessageSource support are minimal, and the new introduced configuration, spring.messages.reloadable is set to false by default, so it's backaward compatible.

This commit adds support for `ReloadableResourceBundleMessageSource`, by adding a `spring.messages.reloadable` configuration property, which can be set to `true` to force the `messageSource` bean to be an instance of that class.

snicoll

We've flagged this one for team attention and it hasn't been triaged yet so need to act on it for now.

I've added a few comments.

snicoll · 2018-06-06T06:59:22Z

...re/src/main/java/org/springframework/boot/autoconfigure/context/MessageSourceProperties.java

@@ -71,6 +73,12 @@
 	 */
 	private boolean useCodeAsDefaultMessage = false;

+	/**
+	 * Whether to use a {@link ReloadableResourceBundleMessageSource} instead of the


Configuration key descriptions can't have javadoc tag (see documentation). I think we need to improve the description as high-level descriptions shouldn't contain implementation details (rather it should describe what the feature does).

snicoll · 2018-06-06T07:04:02Z

...java/org/springframework/boot/autoconfigure/context/MessageSourceAutoConfigurationTests.java

@@ -220,6 +224,45 @@ public void existingMessageSourceInParentIsIgnored() {
 								.isEqualTo("bar")));
 	}

+	@Test
+	public void testDefaultReloadableValueMessageSource() {
+		this.contextRunner.withPropertyValues("spring.messages.basename:test/messages")


This assertion can be added to the existing basic test

snicoll · 2018-06-06T07:04:17Z

...java/org/springframework/boot/autoconfigure/context/MessageSourceAutoConfigurationTests.java

+				"spring.messages.reloadable:true").run((context) -> {
+					assertThat(getDeclaredMessageSource(context))
+							.isInstanceOf(ReloadableResourceBundleMessageSource.class);
+					assertThat(context.getMessage("foo", null, "Foo message", Locale.UK))


What is the purpose of this assertion?

philwebb · 2018-06-06T20:47:07Z

We discussed this a little and we're not too sure on the best way to continue. It would certainly be nice to offer an easy way to plug-in a custom ResourceBundleMessageSource but we're not sure if we should do that with a property.

There is some precedent for controlling caching with properties. For example with spring.thymeleaf.cache which does a similar thing for Thymeleaf templates. We need a little more time to consider the options. If we do add property support we should also change DevTools to make use of it so a full restart isn't required.

ruifigueira · 2018-06-08T10:41:03Z

Thanks for the input. I understand that having a configuration property that "decides" which implementation to use is not ideal at all, and I wouldn't mind if I had to explicitly declare a bean for that:

@Bean
public MessageSource messageSource() {
  return new ReloadableResourceBundleMessageSource();
}

But then, I would have to configure it myself (even MessageSourceProperties bean is not available once a MessageSource bean is explicitly declared).

One possible idea would be to configure that declared MessageSource bean in the MessageSourceAutoConfiguration using the MessageSourceProperties.

See gh-13377

* pr/13377: Polish "Add ReloadableResourceBundleMessageSource support" Add ReloadableResourceBundleMessageSource support

candrews · 2018-10-05T03:32:42Z

Should dev tools set spring.messages.reloadable to true when it's active?

snicoll · 2018-10-05T07:16:09Z

@candrews probably, yes. Can you please create a separate issue?

wilkinsona · 2018-10-29T16:18:23Z

With some regret, we've decided to revert this change. With hindsight, we don't think that the approach taken here is quite right. In particular, setting reloadable: true breaks the default approach of placing messages files in src/main/resources which isn't ideal. We're not yet sure what the right approach should be, or if it should have a boolean reloadable property. Rather than backing ourselves into a corner by shipping this in 2.1, we'd like to give ourselves the time and freedom to consider all alternatives without having to fit the configuration model that was proposed here.

wilkinsona · 2018-10-29T16:20:04Z

I've opened #15007.

Closes gh-14882 See gh-14699 See gh-13377

ruifigueira · 2018-10-30T11:15:34Z

I understand, I honestly didn't expect this to be merged, because we were still discussing if my approach was the best one. I was already convinced it wasn't, and your point regarding files placement is a very good point against this approach.

php-coder · 2020-12-14T10:43:55Z

With some regret, we've decided to revert this change.

BTW it seems like it wasn't removed from the changelogs:

P.S. Yes, I know it's quite old and 2.1 is EOL.

snicoll · 2020-12-14T10:59:16Z

@php-coder this change was reverted after RC1 was released so the current changelog is accurate.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jun 5, 2018

philwebb added the for: team-attention An issue we'd like other members of the team to review label Jun 5, 2018

Support for ReloadableResourceBundleMessageSource

302f3c9

This commit adds support for `ReloadableResourceBundleMessageSource`, by adding a `spring.messages.reloadable` configuration property, which can be set to `true` to force the `messageSource` bean to be an instance of that class.

ruifigueira force-pushed the feature/reloadable-message-source branch from 9f15868 to 302f3c9 Compare June 5, 2018 19:08

snicoll reviewed Jun 6, 2018

View reviewed changes

philwebb added type: enhancement A general enhancement and removed for: team-attention An issue we'd like other members of the team to review status: waiting-for-triage An issue we've not yet triaged labels Jun 13, 2018

philwebb added this to the Backlog milestone Jun 13, 2018

philwebb self-assigned this Oct 4, 2018

philwebb closed this in 22abe35 Oct 5, 2018

philwebb added a commit that referenced this pull request Oct 5, 2018

Polish "Add ReloadableResourceBundleMessageSource support"

11ac9bf

See gh-13377

philwebb added a commit that referenced this pull request Oct 5, 2018

Merge pull request #13377 from ruifigueira

486a063

* pr/13377: Polish "Add ReloadableResourceBundleMessageSource support" Add ReloadableResourceBundleMessageSource support

snicoll modified the milestones: 2.1.x, 2.1.0.RC1 Oct 5, 2018

candrews mentioned this pull request Oct 5, 2018

Make devtools set spring.messages.reloadable=true #14699

Closed

wilkinsona mentioned this pull request Oct 29, 2018

Upgrade to 2.1.0.RC1 breaks message source with DevTools #14882

Closed

wilkinsona mentioned this pull request Oct 29, 2018

Add support for auto-configuring a ReloadableResourceBundleMessageSource #15007

Open

wilkinsona added a commit that referenced this pull request Oct 29, 2018

Revert reloadable message source support

743782d

Closes gh-14882 See gh-14699 See gh-13377

ruifigueira deleted the feature/reloadable-message-source branch March 15, 2021 09:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for ReloadableResourceBundleMessageSource #13377

Add support for ReloadableResourceBundleMessageSource #13377

ruifigueira commented Jun 5, 2018

snicoll left a comment

snicoll Jun 6, 2018

snicoll Jun 6, 2018

snicoll Jun 6, 2018

philwebb commented Jun 6, 2018

ruifigueira commented Jun 8, 2018 •

edited

candrews commented Oct 5, 2018

snicoll commented Oct 5, 2018

wilkinsona commented Oct 29, 2018

wilkinsona commented Oct 29, 2018

ruifigueira commented Oct 30, 2018

php-coder commented Dec 14, 2020 •

edited

snicoll commented Dec 14, 2020

Add support for ReloadableResourceBundleMessageSource #13377

Add support for ReloadableResourceBundleMessageSource #13377

Conversation

ruifigueira commented Jun 5, 2018

snicoll left a comment

Choose a reason for hiding this comment

snicoll Jun 6, 2018

Choose a reason for hiding this comment

snicoll Jun 6, 2018

Choose a reason for hiding this comment

snicoll Jun 6, 2018

Choose a reason for hiding this comment

philwebb commented Jun 6, 2018

ruifigueira commented Jun 8, 2018 • edited

candrews commented Oct 5, 2018

snicoll commented Oct 5, 2018

wilkinsona commented Oct 29, 2018

wilkinsona commented Oct 29, 2018

ruifigueira commented Oct 30, 2018

php-coder commented Dec 14, 2020 • edited

snicoll commented Dec 14, 2020

ruifigueira commented Jun 8, 2018 •

edited

php-coder commented Dec 14, 2020 •

edited