matrei commented on code in PR #16736:
URL: https://github.com/apache/grails-core/pull/16736#discussion_r4413307755
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/ConfigurationBuilder.groovy:
##########
@@ -425,6 +431,387 @@ abstract class ConfigurationBuilder<B, C> {
}
+ private Map resolveMapProperty(String propertyPath, Map fallBackValue) {
+ Map directValue
+ RuntimeException conversionFailure = null
+ try {
+ directValue = propertyResolver.getProperty(propertyPath, Map)
+ } catch (ConversionFailedException | ConverterNotFoundException e) {
+ conversionFailure = e
+ directValue = null
+ }
+ if (!(propertyResolver instanceof ConfigurableEnvironment) &&
directValue != null && !directValue.isEmpty()) {
Review Comment:
`DatastoreUtils.createPropertyResolver` returns a `DatastoreEnvironment`.
That class extends `StandardEnvironment`, so this resolver skips the early
return and goes through the walk too. The description says it still returns its
map unchanged. That only holds while no other source in it adds a key under the
path. `DatastoreEnvironment` also includes the JVM system properties, so
`-DdataSource.properties.minimumIdle=3` is enough to change the result:
```groovy
DatastoreUtils.createPropertyResolver([
'dataSource.properties.maximumPoolSize': 16,
'dataSource.properties.dataSourceProperties.cachePrepStmts': true,
])
```
```
c8b26a41: properties=[maximumPoolSize:16,
dataSourceProperties.cachePrepStmts:true]
84e82828: properties=[maximumPoolSize:16,
dataSourceProperties.cachePrepStmts:true, minimumIdle:3]
```
This resolver backs every datastore built from a `Map`, and also
`HibernateDatastore` when it gets an `Environment`, so the change now reaches
those paths. `preparePropertyResolver` already treats a `DatastoreEnvironment`
as prepared. Adding `|| propertyResolver instanceof DatastoreEnvironment` to
the first condition here restores the c8b26a41 result. With that change, the
specs above still pass. If merging system properties is intended, the
description and `Test DatastoreUtils resolver retains existing map-backed
builder binding` should say so.
--
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]
