sbglasius commented on code in PR #16547:
URL: https://github.com/apache/grails-core/pull/16547#discussion_r4209451381
##########
grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy:
##########
@@ -148,10 +148,6 @@ class NavigableMap implements Map<String, Object>,
Cloneable {
Map sourceMap,
boolean parseFlatKeys) {
Review Comment:
Removing the profile check here also removes it for non-YAML sources.
`GroovyConfigPropertySourceLoader` calls `NavigableMap.merge(configObject,
false)`, so an `application.groovy` with `spring.profiles = 'prod'` or
`spring.config.activate.on-profile = 'prod'` used to be skipped when `prod` was
inactive. It is now merged in every environment, because only the YAML loader
filters by profile.
If dropping this is intentional, it would help to say in the docs that
Groovy config doesn't support document-level profile selectors, and to add a
test for it.
##########
grails-core/src/main/groovy/org/grails/config/yaml/YamlPropertySourceLoader.java:
##########
@@ -58,9 +61,17 @@ public List<PropertySource<?>> load(String name, Resource
resource) throws IOExc
public List<PropertySource<?>> load(String name, Resource resource,
List<String> filteredKeys) throws IOException {
setResources(resource);
+ // Select source documents once; merging resolved configuration must
not re-evaluate JVM profiles.
+ final List<String> activeProfiles = Arrays.asList(
+
StringUtils.tokenizeToStringArray(System.getProperty("spring.profiles.active",
""), ","));
setDocumentMatchers((DocumentMatcher) properties -> {
- final String profile = properties.getProperty("spring.profiles");
- return profile == null ||
profile.equalsIgnoreCase(System.getProperty("spring.profiles.active")) ?
MatchStatus.FOUND : MatchStatus.NOT_FOUND;
+ final String profile =
properties.getProperty("spring.config.activate.on-profile");
+ final String legacyProfile =
properties.getProperty("spring.profiles");
+ final boolean matchesProfile = profile == null ||
profile.isEmpty() ||
Review Comment:
A whitespace-only selector (e.g. `spring.profiles: ' '`) passes the
`isEmpty()` guard, and `Profiles.of(" ")` throws `IllegalArgumentException:
Invalid profile expression [ ]: must contain text` (checked against spring-core
7.0.8), which aborts config loading. Trimming the values before the empty check
avoids it.
Also, a comma list such as `spring.profiles: 'dev,test'` is parsed as one
profile named `dev,test` and never matches. The old `equalsIgnoreCase` check
didn't match it either, so this isn't a regression, but the docs only mention
commas for the active profiles.
--
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]
