jamesfredley commented on code in PR #16736: URL: https://github.com/apache/grails-core/pull/16736#discussion_r4409722063
########## grails-datasource/src/test/groovy/org/grails/plugins/datasource/DataSourceEnvironmentYamlSpec.groovy: ########## @@ -0,0 +1,152 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.grails.plugins.datasource + +import javax.sql.DataSource + +import com.zaxxer.hikari.HikariDataSource + +import org.springframework.core.env.StandardEnvironment +import org.springframework.core.io.ByteArrayResource +import org.springframework.jdbc.datasource.LazyConnectionDataSourceProxy +import org.springframework.jdbc.datasource.TransactionAwareDataSourceProxy + +import grails.util.Environment +import org.grails.config.yaml.YamlPropertySourceLoader +import org.grails.datastore.gorm.jdbc.connections.DataSourceConnectionSourceFactory +import org.grails.datastore.mapping.core.connections.ConnectionSource + +import spock.lang.Specification + +class DataSourceEnvironmentYamlSpec extends Specification { + + private static final String APPLICATION_YML = '''\ +dataSource: + pooled: true + driverClassName: org.h2.Driver + username: sa + password: '' + type: com.zaxxer.hikari.HikariDataSource + properties: + maximumPoolSize: 4 + minimumIdle: 1 + connectionTimeout: 1000 +environments: + development: + dataSource: + dbCreate: create-drop + url: jdbc:h2:mem:devDb;DB_CLOSE_DELAY=-1 + properties: + maximumPoolSize: 16 + minimumIdle: 8 + connectionTimeout: 2000 + test: + dataSource: + dbCreate: update + url: jdbc:h2:mem:testDb;DB_CLOSE_DELAY=-1 + properties: + maximumPoolSize: 2 + minimumIdle: 1 + production: + dataSource: + dbCreate: none + url: jdbc:h2:mem:prodDb;DB_CLOSE_DELAY=-1 + properties: + maximumPoolSize: 32 + minimumIdle: 4 + connectionTimeout: 5000 +''' + + private String previousEnvironment + + def setup() { + previousEnvironment = System.getProperty(Environment.KEY) + } + + def cleanup() { + if (previousEnvironment == null) { + System.clearProperty(Environment.KEY) + } + else { + System.setProperty(Environment.KEY, previousEnvironment) + } + Environment.reset() + } + + void "application.yml #environment block supplies the dataSource properties"() { + given: + System.setProperty(Environment.KEY, environment) + Environment.reset() + def connectionSource = createFromApplicationYaml() + def dataSource = unwrap(connectionSource.source) + + expect: + connectionSource.settings.url == url + dataSource instanceof HikariDataSource + dataSource.maximumPoolSize == maximumPoolSize + dataSource.minimumIdle == minimumIdle + dataSource.connectionTimeout == connectionTimeout + + cleanup: + connectionSource?.close() + + where: + environment | url | maximumPoolSize | minimumIdle | connectionTimeout + 'development' | 'jdbc:h2:mem:devDb;DB_CLOSE_DELAY=-1' | 16 | 8 | 2000 + 'test' | 'jdbc:h2:mem:testDb;DB_CLOSE_DELAY=-1' | 2 | 1 | 1000 + 'production' | 'jdbc:h2:mem:prodDb;DB_CLOSE_DELAY=-1' | 32 | 4 | 5000 + } + + void "unset grails.env uses the development dataSource properties"() { + given: + System.clearProperty(Environment.KEY) + Environment.reset() + def connectionSource = createFromApplicationYaml() + def dataSource = unwrap(connectionSource.source) + + expect: + connectionSource.settings.url == 'jdbc:h2:mem:devDb;DB_CLOSE_DELAY=-1' + dataSource instanceof HikariDataSource + dataSource.maximumPoolSize == 16 + dataSource.minimumIdle == 8 + dataSource.connectionTimeout == 2000 + + cleanup: + connectionSource?.close() + } + + private static createFromApplicationYaml() { Review Comment: The description now shows the dotted `application.groovy` form, which `application.groovy dotted properties configure the connection pool` loads through `GroovyConfigPropertySourceLoader`. It also notes that the block form does not parse under Groovy 5.1 and is separate from this fix. ########## grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/jdbc/connections/DataSourceConnectionSourceFactorySpec.groovy: ########## @@ -18,14 +18,308 @@ */ 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 +import javax.sql.DataSource + + 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"() { Review Comment: Left for a follow-up. The description now has a sentence on `dataSources.dataSource` falling back to the root `dataSource` settings. ########## 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 + } + Object value = readEnvironmentValue(environment, propertySource, name) + if (value == null) { + continue + } + String[] segments = name.substring(prefix.length()).split('\\.') Review Comment: Fixed in 84e8282825. `name[n]` keys build a `List`, copying an existing one instead of mutating it, so `connectionInitSqls` binds as a list. Covered by `application.yml binds a list under dataSource properties` and `create collects indexed pool properties into a list in numeric order`. ########## 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()) { Review Comment: Fixed in 84e8282825. The early return now applies only to a resolver that is not a `ConfigurableEnvironment`. With an Environment the map at the exact path is read per source inside the walk, so a higher flattened `maximumPoolSize: 50` wins over a lower map. Covered by `create merges a lower direct map behind higher flattened pool properties`. -- 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]
