[jamesfredley]

Fixes #16567

Expected Behavior

The dataSource.properties block in application.yml is applied to the HikariCP pool.

With the config below, the pool should use maximumPoolSize 16 and minimumIdle 8.

dataSource:
    pooled: true
    jmxExport: true
    driverClassName: org.h2.Driver
    username: sa
    password: ''
    properties:
        maximumPoolSize: 16
        minimumIdle: 8
        connectionTimeout: 2000
environments:
    development:
        dataSource:
            dbCreate: create-drop
            type: com.zaxxer.hikari.HikariDataSource
            url: jdbc:h2:mem:devDb;LOCK_TIMEOUT=10000;DB_CLOSE_ON_EXIT=FALSE
            properties:
                maximumPoolSize: 16
                minimumIdle: 8
                connectionTimeout: 2000

The dotted application.groovy form fails the same way:

dataSource.pooled = true
dataSource.properties.maximumPoolSize = 100
dataSource.properties.minimumIdle = 10

Actual Behaviour

The pool uses Hikari's defaults: maximumPoolSize 10 and minimumIdle equal to maximumPoolSize.

URL, username, and password from the same block are applied. Only the properties map is dropped.

Cause

The dataSource bean is created from the Spring Environment:

#{dataSourceConnectionSourceFactory.create('default', environment).source}

Environment.getProperty("dataSource.properties", Map) does not return the nested YAML map. The keys exist only as flattened entries such as dataSource.properties.maximumPoolSize. Because that map is empty, the Hikari binder never runs. The unconfigured pool is then cached and reused by GORM.

Fix

A raw Spring Environment binds a map setting from flattened names only, highest source first. The first source to define a key wins.

• dataSource.properties.maximumPoolSize and the other flattened pool keys bind, including from an environments block.
• name[n] builds a list, so a YAML list under dataSource.properties binds as a list.
• A higher scalar blocks a lower descendant, so flush.mode=COMMIT is not overwritten by a lower flush.mode.something.
• String values go through Environment.getProperty, so the unresolvable-placeholder setting is kept.
• A map stored at the exact path is used only when no flattened name exists. It is not merged behind flattened keys. This matches Spring Boot's map binder. The loaders Grails ships flatten, so a map at the path is the application.groovy or DatastoreUtils case, not YAML.

DatastoreEnvironment is prepared configuration. It returns its map unchanged and does not walk, so JVM systemProperties and systemEnvironment are not merged in, and a lower configuration map is not merged either. That is the base-branch result. A resolver that is not a ConfigurableEnvironment still returns its map unchanged. A malformed map value with no flattened children still fails.

@Builder(builderStrategy = SimpleStrategy) settings, including HibernateSettings, use the same binding, so a Spring Environment picks up their flattened keys.

No application change is required. The generated application.yml works as written.

The environments.development, test, and production blocks override the root dataSource for the active Grails environment. An unset grails.env uses development. A key the active block does not set, such as connectionTimeout under test, stays at the root value.

Not in this PR

• The block form dataSource { properties { ... } } in application.groovy does not parse under Groovy 5.1. ConfigSlurper throws ReadOnlyPropertyException for properties, so only the dotted form above loads.
• A default datasource declared under dataSources.dataSource is still read with a direct map lookup in DataSourceConnectionSourceFactory, so it falls back to the root dataSource settings. The guide documents only the root dataSource: form.
• A YAML list that reaches a datastore through DatastoreUtils.preparePropertyResolver is still kept as literal name[n] keys. This PR does not change that parse. The datasource YAML path does bind connectionInitSqls as a list.

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

3 open issues

• The rebuild logic only considers raw enumerable property names that literally start with… · New
• When direct map binding is null/empty, this scans every enumerable key of every PropertySource.… · New
• These tests assume a specific proxy layering (TransactionAwareDataSourceProxy →… · New

What changed in this PR

Ensures dataSource.properties.* entries defined as flattened Spring Environment properties (e.g., from application.yml) are correctly bound into the datasource configuration map so downstream HikariCP settings are applied.

Changes:

• Add resolveMapProperty to reconstruct nested Map values from flattened Environment keys when direct map binding is empty/missing.
• Use the new map-resolution logic when binding map-typed configuration values.
• Add Spock specs covering flattened binding, property-source precedence, and mixed map/flattened scenarios.

File	Description
grails-datastore-core/​src/​main/​groovy/​org/​grails/​datastore/​mapping/​config/​ConfigurationBuilder.groovy	Rebuilds map config values from flattened Spring Environment property keys when direct map binding yields null/empty.
grails-datamapping-core/​src/​test/​groovy/​org/​grails/​datastore/​gorm/​jdbc/​connections/​DataSourceConnectionSourceFactorySpec.groovy	Adds tests validating flattened datasource pool property binding and precedence behavior for Spring Environment.

_{Review effort: Lite}

[codecov]

Codecov Report

❌ Patch coverage is 76.66667% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.2391%. Comparing base (a348ef9) to head (c8b26a4).
⚠️ Report is 15 commits behind head on 8.0.x.

Files with missing lines	Patch %	Lines
...tastore/mapping/config/ConfigurationBuilder.groovy	76.6667%	5 Missing and 16 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #16736        +/-   ##
==================================================
+ Coverage     62.1821%   62.2391%   +0.0570%     
- Complexity      26900      26967        +67     
==================================================
  Files            2241       2241                
  Lines          110736     110829        +93     
  Branches        20218      20246        +28     
==================================================
+ Hits            68858      68979       +121     
+ Misses          32615      32573        -42     
- Partials         9263       9277        +14     

Files with missing lines	Coverage Δ
...tastore/mapping/config/ConfigurationBuilder.groovy	70.4082% <76.6667%> (+1.6582%)	⬆️

... and 8 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[jdaugherty]

Verified at c8b26a4:

• The new specs reproduce #16567. Against the base ConfigurationBuilder, all 4 DataSourceEnvironmentYamlSpec features and 10 of the 23 DataSourceConnectionSourceFactorySpec features fail. At the PR head, grails-datastore-core (326), the factory spec (23) and the YAML spec (4) pass locally.
• Cause confirmed: NavigableMapPropertySource.getProperty returns null for a nested map and leaves it out of getPropertyNames, so the direct Map read on the Environment is empty. The 7.0.x source hides the map the same way; what changed on 8.0.x is which definition builds the pool. DataSourceGrailsPlugin leaves the dataSource bean to HibernateDatastoreConnectionSourcesRegistrar when the hibernate plugin is present, and that SpEL passes environment.
• The Groovy Snapshot Canary failures on this head are mongo:8.0 container startups, unrelated. The CI workflow is green at c8b26a4.

Inline: one precedence bug when a nested key meets a scalar across property sources, a coverage ask for the @Builder map branch that HibernateSettings takes, and the application.groovy claim in the description, which the Groovy 5 ConfigSlurper never reaches. The dataSources.dataSource note is a follow-up.

[jdaugherty]

@jamesfredley reminder this is ready for another pass

[matrei]

I checked this locally at c8b26a4 alongside jdaugherty's review, and I agree with the points there. I have one design question and two more edge cases.

The registrar is the only caller that reads the raw Environment. HibernateDatastore already works around the hidden maps. Its constructors pass the configuration through DatastoreUtils.preparePropertyResolver(configuration, 'dataSource', 'hibernate', 'grails'), which rebuilds nested maps from the flattened keys of a ConfigurableEnvironment. In a Grails app it gets grailsApplication.config instead, which that method returns as is. HibernateDatastoreConnectionSourcesRegistrar is the only caller that passes the raw environment to dataSourceConnectionSourceFactory.create. CachedDataSourceConnectionSourceFactory caches the pool by name, so whichever of the two runs first decides the pool for both.

I reverted this PR's ConfigurationBuilder change and ran the issue's YAML through create('default', DatastoreUtils.preparePropertyResolver(environment, 'dataSource')). That gives maximumPoolSize 16, connectionTimeout 2000 and the dataSourceProperties collation, and a higher-precedence dataSource.properties.minimumIdle still wins. It also resolves the dataSources: dataSource: form from the follow-up comment. On this PR's head, that form still falls back to the built-in H2 URL with empty properties.

Building the registrar's bean through the same call HibernateDatastore makes would be a one-line change in the hibernate5 and hibernate7 registrars. It would leave ConfigurationBuilder unchanged. Every datastore shares that class, including the @Builder map branch flagged above. The trade-off is that this PR also fixes callers that pass a raw Environment to a ConfigurationBuilder directly. Was this approach considered? Even if the generic fix stays, I think the registrar and the datastore should resolve the default data source the same way, because the cache hands the pool to whichever one runs first.

[matrei]

I rechecked at 84e8282. Locally, grails-datastore-core (354), DataSourceConnectionSourceFactorySpec (23) and DataSourceEnvironmentYamlSpec (6) all pass. Both points from my last review are fixed. A YAML list under dataSource.properties now binds as a list, including on a bean with DBCP2's connectionInitSqls getter and setter. A higher flattened key now wins over a lower map stored at the exact path. Also, a higher list that is shorter than a lower one replaces it whole, as in Spring Boot.

The design question from my last review is still open. resolveMapProperty and its helpers now take about 380 lines of ConfigurationBuilder, which every datastore shares. Most of the helpers resolve conflicts between maps, literal dotted keys, strings that convert to maps, and scalars from different sources. The config loaders Grails ships never put a map at a path. Both the YAML and the Groovy loader produce a NavigableMapPropertySource, which returns null for map and list values and lists their contents as flattened names. Properties files, system properties and the command line are flat. Map values at a path come from a MapPropertySource built in code, as most of the new specs do, and from the ConfigObjectPropertySource in DatastoreUtils.

My earlier comment on the early return offered handling that case as one option, so part of this growth comes from my own suggestion. Now that I see the cost, I'd prefer a smaller fix. One way is to limit the Environment walk to flattened names, including name[n] lists, and document the direct-map case as out of scope. The other is to build the registrar's bean through DatastoreUtils.preparePropertyResolver, the way HibernateDatastore resolves its configuration. Was that considered?

[jamesfredley]

Fixed in 0b35b1c. A DatastoreEnvironment still walks its prepared configuration sources, so a key that exists only on a lower map is kept. The walk skips the systemProperties and systemEnvironment sources that StandardEnvironment adds, so -Dgrails.gorm.hibernate.minimumIdle=3 is not a setting.

Returning the highest getProperty(..., Map) alone drops those lower keys. createPropertyResolvers stores each map as its own source, and the later map only wins keys it actually sets. Test DatastoreUtils resolver keeps keys from a lower configuration map covers shared=high, override=new, base=retained, and minimumIdle=null. The existing single-map feature still asserts the system property is absent.

The generic walk stays for a raw ConfigurableEnvironment. Callers that pass a raw environment still need it, so this does not replace that walk with the registrar one-liner. dataSources.dataSource remains a follow-up.

[jdaugherty]

Verified at 0b35b1c:

• Locally, grails-datastore-core (355), grails-datamapping-core (1815) and grails-datasource (13) pass, including ConfigurationBuilderSpec (58), DataSourceConnectionSourceFactorySpec (23) and DataSourceEnvironmentYamlSpec (6).
• The five asks from my first review are addressed: a higher scalar now blocks a lower flattened descendant (the flush.mode case), the @Builder map branch is covered with a StandardEnvironment and with a DatastoreUtils resolver, the Object-typed read is gone, the description shows the dotted application.groovy form that GroovyConfigPropertySourceLoader loads and names the block form as separate, and dataSources.dataSource is noted as a follow-up.

On the design question matrei left open, I support the narrowing option, for the reasons in the inline comment on resolveMapProperty: Spring Boot's MapBinder binds a Map from flattened descendants and reads a value stored at the exact path only when no source has a descendant, every loader Grails ships flattens, and 19 of the 24 features this PR adds to ConfigurationBuilderSpec exist to pin map-at-path cases those loaders cannot produce. "Callers that pass a raw environment still need it" holds, and the walk stays for them; what goes is the per-source map merging around it. My reply on the DatastoreEnvironment thread has the base-branch measurement behind the early return.

One pre-existing gap on the datastore path is noted on the YAML list feature as a follow-up, not for this PR.

[testlens-app]

🚨 TestLens detected 4 failed tests 🚨

Failed Jobs without Test Failures

❌ Code Style / Core Projects

Test Summary

Tip

Inspect all details with the TestLens CLI: testlens pr 16736

CI / Build Grails Forge (Java 21, indy=false) > :grails-forge-core:test [grails-forge]

Test	Runs	Flakiness
GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation	❌	0% 🟢

CI / Build Grails Forge (Java 21, indy=true) > :grails-forge-core:test [grails-forge]

Test	Runs	Flakiness
GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation	❌	0% 🟢

CI / Build Grails Forge (Java 25, indy=false) > :grails-forge-core:test [grails-forge]

Test	Runs	Flakiness
GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation	❌	0% 🟢

CI / Functional Tests (Java 25, indy=false) > :grails-test-examples-geb:integrationTest

Test	Runs	Flakiness
PerTestRecordingSpec > the recordings of the previous two tests are different	❌	1% 🟡

🏷️ Commit: bc3f653
▶️ Tests: 93305 executed
⚪️ Checks: 96/96 completed

Test Failures

GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation (:grails-forge-core:test [grails-forge] in CI / Build Grails Forge (Java 21, indy=false))

Condition not satisfied:

readme.contains("[Grails GORM for GraphQL documentation](https://grails.apache.org/docs/${VersionInfo.documentationVersion}/grails-data/graphql/manual/)")
|      |                                                                                  |           |
|      false                                                                              |           snapshot
|                                                                                         class org.grails.forge.util.VersionInfo
## Grails 8.0.1-SNAPSHOT Documentation
 
- [User Guide](https://grails.apache.org/docs/snapshot/guide/index.html)
- [API Reference](https://grails.apache.org/docs/snapshot/api/index.html)
- [Grails Guides](https://guides.grails.org/index.html)
---
 
## Feature gorm-graphql documentation
 
- [Grails Data for GraphQL documentation](https://grails.apache.org/docs/snapshot/grails-data/graphql/manual/)
 
- [https://graphql.org](https://graphql.org)
 
## Feature spring-boot-devtools documentation
 
- [Grails SpringBoot Developer Tools documentation](https://docs.spring.io/spring-boot/reference/using/devtools.html)
 
## Feature asset-pipeline-grails documentation
 
- [Grails Asset Pipeline documentation](https://github.com/wondrify/asset-pipeline#readme)
 
## Feature scaffolding documentation
 
- [Grails Scaffolding documentation](https://grails.apache.org/docs/snapshot/guide/scaffolding.html)
 
## Feature geb-with-testcontainers documentation
 
- [Grails Geb Functional Testing for Grails with Testcontainers documentation](https://github.com/apache/grails-geb#readme)
 
- [https://groovy.apache.org/geb/manual/current/](https://groovy.apache.org/geb/manual/current/)

	at org.grails.forge.feature.database.GraphqlGormSpec.test readme.md with feature gorm-graphql contains links to documentation(GraphqlGormSpec.groovy:158)

GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation (:grails-forge-core:test [grails-forge] in CI / Build Grails Forge (Java 21, indy=true))

Condition not satisfied:

readme.contains("[Grails GORM for GraphQL documentation](https://grails.apache.org/docs/${VersionInfo.documentationVersion}/grails-data/graphql/manual/)")
|      |                                                                                  |           |
|      false                                                                              |           snapshot
|                                                                                         class org.grails.forge.util.VersionInfo
## Grails 8.0.1-SNAPSHOT Documentation
 
- [User Guide](https://grails.apache.org/docs/snapshot/guide/index.html)
- [API Reference](https://grails.apache.org/docs/snapshot/api/index.html)
- [Grails Guides](https://guides.grails.org/index.html)
---
 
## Feature gorm-graphql documentation
 
- [Grails Data for GraphQL documentation](https://grails.apache.org/docs/snapshot/grails-data/graphql/manual/)
 
- [https://graphql.org](https://graphql.org)
 
## Feature scaffolding documentation
 
- [Grails Scaffolding documentation](https://grails.apache.org/docs/snapshot/guide/scaffolding.html)
 
## Feature asset-pipeline-grails documentation
 
- [Grails Asset Pipeline documentation](https://github.com/wondrify/asset-pipeline#readme)
 
## Feature mockito documentation
 
- [https://site.mockito.org](https://site.mockito.org)
 
## Feature geb-with-testcontainers documentation
 
- [Grails Geb Functional Testing for Grails with Testcontainers documentation](https://github.com/apache/grails-geb#readme)
 
- [https://groovy.apache.org/geb/manual/current/](https://groovy.apache.org/geb/manual/current/)
 
## Feature spring-boot-devtools documentation
 
- [Grails SpringBoot Developer Tools documentation](https://docs.spring.io/spring-boot/reference/using/devtools.html)

	at org.grails.forge.feature.database.GraphqlGormSpec.test readme.md with feature gorm-graphql contains links to documentation(GraphqlGormSpec.groovy:158)

GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation (:grails-forge-core:test [grails-forge] in CI / Build Grails Forge (Java 25, indy=false))

Condition not satisfied:

readme.contains("[Grails GORM for GraphQL documentation](https://grails.apache.org/docs/${VersionInfo.documentationVersion}/grails-data/graphql/manual/)")
|      |                                                                                  |           |
|      false                                                                              |           snapshot
|                                                                                         class org.grails.forge.util.VersionInfo
## Grails 8.0.1-SNAPSHOT Documentation
 
- [User Guide](https://grails.apache.org/docs/snapshot/guide/index.html)
- [API Reference](https://grails.apache.org/docs/snapshot/api/index.html)
- [Grails Guides](https://guides.grails.org/index.html)
---
 
## Feature spring-boot-devtools documentation
 
- [Grails SpringBoot Developer Tools documentation](https://docs.spring.io/spring-boot/reference/using/devtools.html)
 
## Feature asset-pipeline-grails documentation
 
- [Grails Asset Pipeline documentation](https://github.com/wondrify/asset-pipeline#readme)
 
## Feature gorm-graphql documentation
 
- [Grails Data for GraphQL documentation](https://grails.apache.org/docs/snapshot/grails-data/graphql/manual/)
 
- [https://graphql.org](https://graphql.org)
 
## Feature geb-with-testcontainers documentation
 
- [Grails Geb Functional Testing for Grails with Testcontainers documentation](https://github.com/apache/grails-geb#readme)
 
- [https://groovy.apache.org/geb/manual/current/](https://groovy.apache.org/geb/manual/current/)
 
## Feature scaffolding documentation
 
- [Grails Scaffolding documentation](https://grails.apache.org/docs/snapshot/guide/scaffolding.html)

	at org.grails.forge.feature.database.GraphqlGormSpec.test readme.md with feature gorm-graphql contains links to documentation(GraphqlGormSpec.groovy:158)

PerTestRecordingSpec > the recordings of the previous two tests are different (:grails-test-examples-geb:integrationTest in CI / Functional Tests (Java 25, indy=false))

Condition not satisfied:

secondRecording.length() > MIN_MEANINGFUL_RECORDING_BYTES
|               |        | |
|               2790     | 5000
|                        false
build/gebContainer/recordings/20261031_010304/PASSED-PerTestRecordingSpec_setup_running_a_second_test_to_create_another_recording-20261031-010657.mp4

	at org.demo.spock.PerTestRecordingSpec.the recordings of the previous two tests are different(PerTestRecordingSpec.groovy:105)

Rerun Controls

Select tests to mute in this pull request:

• GraphqlGormSpec > test readme.md with feature gorm-graphql contains links to documentation
• PerTestRecordingSpec > the recordings of the previous two tests are different

Reuse successful test results:

• ♻️ Only rerun the tests that failed or were muted before

Click the checkbox to trigger a rerun:

• Rerun jobs

---

Learn more about TestLens at testlens.app/docs.