jamesfredley commented on code in PR #16736:
URL: https://github.com/apache/grails-core/pull/16736#discussion_r4353104723
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/ConfigurationBuilder.groovy:
##########
@@ -425,6 +430,152 @@ 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 (directValue != null && !directValue.isEmpty()) {
+ return directValue
+ }
+ if (propertyResolver instanceof ConfigurableEnvironment) {
+ ConfigurableEnvironment environment = (ConfigurableEnvironment)
propertyResolver
+ String prefix = propertyPath + '.'
+ Map<String, Object> result = new LinkedHashMap<>()
+ Set<String> resolvedNames = new HashSet<>()
+ for (PropertySource<?> propertySource :
environment.propertySources) {
+ if (propertySource instanceof EnumerablePropertySource) {
+ for (String name : ((EnumerablePropertySource<?>)
propertySource).propertyNames) {
+ if (!name.startsWith(prefix) ||
!resolvedNames.add(name)) {
+ continue
+ }
Review Comment:
The failure in #16567 is `application.yml` and `application.groovy`. Spring
and Grails already flatten those to dotted keys such as
`dataSource.properties.maximumPoolSize`, and that is what this scan matches.
`SystemEnvironmentPropertySource` can resolve a known dotted name to
`DATASOURCE_PROPERTIES_MAXIMUMPOOLSIZE`, but `getPropertyNames()` returns the
raw env-var form. Reversing that form is ambiguous (`MAXIMUMPOOLSIZE` vs
`MAXIMUM_POOL_SIZE`) and is not how this bug shows up. This module also cannot
take a Spring Boot Binder dependency. Not adding a guesser for that.
##########
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/ConfigurationBuilder.groovy:
##########
@@ -425,6 +430,152 @@ 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 (directValue != null && !directValue.isEmpty()) {
+ return directValue
+ }
+ if (propertyResolver instanceof ConfigurableEnvironment) {
+ ConfigurableEnvironment environment = (ConfigurableEnvironment)
propertyResolver
+ String prefix = propertyPath + '.'
+ Map<String, Object> result = new LinkedHashMap<>()
+ Set<String> resolvedNames = new HashSet<>()
+ for (PropertySource<?> propertySource :
environment.propertySources) {
+ if (propertySource instanceof EnumerablePropertySource) {
+ for (String name : ((EnumerablePropertySource<?>)
propertySource).propertyNames) {
+ if (!name.startsWith(prefix) ||
!resolvedNames.add(name)) {
+ continue
+ }
Review Comment:
The prefix check already skips keys that do not belong to the map. What
remains is one walk per map setting, at startup, when the datasource is created.
`systemProperties` and `systemEnvironment` have to stay in that walk.
`-DdataSource.properties.maximumPoolSize` lives there and has to win
precedence. Caching the rebuilt map on the builder would also alias the map
stored on the settings object. Leaving the scan as-is.
##########
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/jdbc/connections/DataSourceConnectionSourceFactorySpec.groovy:
##########
@@ -18,14 +18,305 @@
*/
package org.grails.datastore.gorm.jdbc.connections
+import com.zaxxer.hikari.HikariDataSource
+import groovy.util.ConfigObject
import org.grails.datastore.mapping.core.DatastoreUtils
+import org.grails.datastore.mapping.core.exceptions.ConfigurationException
import org.grails.datastore.mapping.core.connections.ConnectionSource
+import org.springframework.core.convert.converter.Converter
+import org.springframework.core.convert.support.DefaultConversionService
+import org.springframework.core.env.MapPropertySource
+import org.springframework.core.env.StandardEnvironment
import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy
import org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy
import spock.lang.Specification
class DataSourceConnectionSourceFactorySpec extends Specification {
+ void "create binds flattened default data source pool properties from the
Spring Environment"() {
+ given:
+ def factory = new DataSourceConnectionSourceFactory()
+ def environment = new StandardEnvironment()
+ environment.propertySources.addFirst(new
MapPropertySource('application.yml', [
+ 'dataSource.url': 'jdbc:h2:mem:factoryEnvironmentTest',
+ 'dataSource.driverClassName': 'org.h2.Driver',
+ 'dataSource.username': 'sa',
+ 'dataSource.password': '',
+ 'dataSource.type': 'com.zaxxer.hikari.HikariDataSource',
+ 'dataSource.properties.maximumPoolSize': 16,
+ 'dataSource.properties.minimumIdle': 8,
+ 'dataSource.properties.connectionTimeout': 2000,
+
'dataSource.properties.dataSourceProperties.connectionCollation':
'utf8mb4_unicode_ci'
+ ]))
+
+ when:
+ def connectionSource = factory.create(ConnectionSource.DEFAULT,
environment)
+
+ then:
+ connectionSource.settings.url == 'jdbc:h2:mem:factoryEnvironmentTest'
+ connectionSource.settings.username == 'sa'
+ connectionSource.settings.password == ''
+ connectionSource.source instanceof TransactionAwareDataSourceProxy
+ def lazyDataSource = ((TransactionAwareDataSourceProxy)
connectionSource.source).targetDataSource
+ lazyDataSource instanceof LazyConnectionDataSourceProxy
+ def dataSource = ((LazyConnectionDataSourceProxy)
lazyDataSource).targetDataSource
+ dataSource instanceof HikariDataSource
+ dataSource.maximumPoolSize == 16
+ dataSource.minimumIdle == 8
+ dataSource.connectionTimeout == 2000
+ dataSource.dataSourceProperties.getProperty('connectionCollation') ==
'utf8mb4_unicode_ci'
+
+ cleanup:
+ connectionSource?.close()
+ }
+
+ void "create binds flattened named data source pool properties from the
Spring Environment"() {
+ given:
+ def factory = new DataSourceConnectionSourceFactory()
+ def environment = new StandardEnvironment()
+ environment.propertySources.addFirst(new
MapPropertySource('application.yml', [
+ 'dataSources.secondary.url':
'jdbc:h2:mem:factoryNamedEnvironmentTest',
+ 'dataSources.secondary.type': HikariDataSource.name,
+ 'dataSources.secondary.properties.maximumPoolSize': 24
+ ]))
+
+ when:
+ def connectionSource = factory.create('secondary', environment)
+
+ then:
+ connectionSource.name == 'secondary'
+ def dataSource =
connectionSource.source.targetDataSource.targetDataSource
+ dataSource instanceof HikariDataSource
Review Comment:
Done in 31b481234b. The Hikari assertions now go through `unwrap()`, which
walks any number of `TransactionAwareDataSourceProxy` and
`LazyConnectionDataSourceProxy` wrappers. The first test still checks the
production proxy order explicitly.
--
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]
