jamesfredley commented on code in PR #16722:
URL: https://github.com/apache/grails-core/pull/16722#discussion_r4406681332
##########
grails-i18n/src/main/groovy/org/grails/plugins/i18n/I18nEnvironmentPostProcessor.java:
##########
@@ -147,12 +149,10 @@ private List<String> compose(ConfigurableEnvironment
environment, List<String> d
}
private List<String> declaredBasenames(ConfigurableEnvironment
environment) {
- String declared = environment.getProperty(BASENAME_PROPERTY);
- if (declared == null || declared.isBlank()) {
- return List.of();
- }
+ List<String> declared =
Binder.get(environment).bind(BASENAME_PROPERTY, Bindable.listOf(String.class))
Review Comment:
Declared names are published when no descriptor is found. The new spec
stores spring.messages.basename[0] and [1] on a class loader with no
descriptor, then asserts getProperty(BASENAME_PROPERTY, 'messages') is the
composed list. Restoring the early return leaves that property unset, so the
assertion fails.
##########
grails-i18n/src/test/groovy/org/grails/plugins/i18n/I18nEnvironmentPostProcessorSpec.groovy:
##########
@@ -118,6 +120,27 @@ locales=fr
environment.getProperty(I18nEnvironmentPostProcessor.BASENAME_PROPERTY) ==
'config/i18n/custom,messages'
}
+ void 'declared base names in #format form stay ahead of discovered
bundles'() {
+ given: 'Grails configuration exposes non-empty lists through indexed
properties'
+ NavigableMap config = new NavigableMap()
+ config.merge([spring: [messages: [basename: declared]]], false)
+ MockEnvironment environment = new MockEnvironment()
+ environment.propertySources.addLast(new
NavigableMapPropertySource('application', config))
+
+ when:
+ new I18nEnvironmentPostProcessor(bootstrapContext)
+ .postProcessEnvironment(environment,
applicationUsing(getClass().classLoader))
+
+ then: 'the declared order is preserved before the discovered
application bundle'
+
environment.getProperty(I18nEnvironmentPostProcessor.BASENAME_PROPERTY) ==
+ 'config/i18n/custom,config/i18n/other,messages'
+
+ where:
+ format | declared
+ 'list' | [' config/i18n/custom ', '', 'config/i18n/other',
'config/i18n/custom']
+ 'comma-separated' | ' config/i18n/custom , ,
config/i18n/other,config/i18n/custom '
+ }
Review Comment:
The spec now pins a plain MapPropertySource with spring.messages.basename[0]
and [1], both when a descriptor is present and when it is not.
##########
grails-i18n/src/test/groovy/org/grails/plugins/i18n/I18nEnvironmentPostProcessorSpec.groovy:
##########
@@ -118,6 +120,27 @@ locales=fr
environment.getProperty(I18nEnvironmentPostProcessor.BASENAME_PROPERTY) ==
'config/i18n/custom,messages'
}
+ void 'declared base names in #format form stay ahead of discovered
bundles'() {
+ given: 'Grails configuration exposes non-empty lists through indexed
properties'
+ NavigableMap config = new NavigableMap()
+ config.merge([spring: [messages: [basename: declared]]], false)
+ MockEnvironment environment = new MockEnvironment()
+ environment.propertySources.addLast(new
NavigableMapPropertySource('application', config))
Review Comment:
Left the test sources at addLast. The processor publishes the composed list
with addFirst, and the tests model application config below that source.
##########
grails-i18n/src/main/groovy/org/grails/plugins/i18n/I18nEnvironmentPostProcessor.java:
##########
@@ -147,12 +149,10 @@ private List<String> compose(ConfigurableEnvironment
environment, List<String> d
}
private List<String> declaredBasenames(ConfigurableEnvironment
environment) {
- String declared = environment.getProperty(BASENAME_PROPERTY);
- if (declared == null || declared.isBlank()) {
- return List.of();
- }
+ List<String> declared =
Binder.get(environment).bind(BASENAME_PROPERTY, Bindable.listOf(String.class))
+ .orElseGet(List::of);
List<String> basenames = new ArrayList<>();
- for (String basename : declared.split(",")) {
+ for (String basename : declared) {
Review Comment:
Left bind failures uncaught. A map binds to an empty list, and a failure
that is real would surface again when Boot binds the same property.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
