fix(config): bind nested settings maps under Spring 7

[jamesfredley]

Fixes #16159.

The problem

Spring Framework 7 no longer converts a configuration Map into a type annotated @Builder(builderStrategy = SimpleStrategy). ConfigurationBuilder depends on that conversion to bind nested settings from application.yml / application.groovy, so those settings now fail with ConverterNotFoundException at configuration-binding time - that is, at application startup.

Grails 8 is the first affected version, because it is the first on Spring 7: 8.0.x pins Spring Boot 4.1.0, which resolves spring-core 7.0.8.

This targets 8.0.x so the fix flows up the maintenance chain (8.0.x -> 8.1.x -> 9.0.x) instead of landing only on the newest line. Both branches pin the same Spring Boot and Groovy versions, and ConfigurationBuilder.groovy plus its spec are byte-identical on each, so one fix covers both.

The gap is demonstrable rather than theoretical: with the previous ConfigurationBuilder and only this PR's spec applied, six scenarios fail with

Expected exception of type 'ConfigurationException',
  but got 'org.springframework.core.convert.ConverterNotFoundException'

The fix

ConfigurationBuilder now instantiates the target type and populates it from the Map when, and only when, Spring genuinely has no converter.

Two independent reviewers examined the handling over five rounds and found nine distinct defects. Every guard below exists because removing it produced an observable failure, and each has a regression test:

Guard	Failure without it
Engage only when the cause chain contains ConverterNotFoundException	A converter that deliberately rejects a Map could be bypassed
Never suppress ConfigurationException	Unknown-key and malformed-value failures were masked by the original exception
Throw on raw-lookup failure instead of falling back	Configuration whose lookup had failed was silently accepted
Inherit from the fallback before applying overrides	Overriding one field discarded every unspecified field
Pass each property's fallback child into nested conversion	Inheritance worked only at the first level; deeper children were reset
Convert values to the target property type	multiTenancy.mode: database was rejected where DATABASE worked
Resolve Class entries via the thread context class loader	hibernate.configClass and other application classes were left unbound, because that converter resolves against the framework class loader
Let Map-backed types keep arbitrary entries	HibernateSettings extends LinkedHashMap exists to carry keys such as hibernate.hbm2ddl.auto; strict binding rejected them
Bind flattened descendant keys through their parent	The resolver flattens nested config, so anything nested more than one level failed to build
Invoke setters with an explicit single-element argument array	The Java null-varargs pitfall meant an explicit null could not clear an inherited value

Strictness is preserved where it belongs: a dotted key whose first segment is unknown is still rejected, and non-Map types still reject unknown keys. Both are covered by guard specs so a future change cannot quietly relax them.

Testing

ConfigurationBuilderSpec grows from 10 to 22 specs; the module total goes from 108 to 120, all passing.

:grails-datastore-core:test --rerun-tasks   BUILD SUCCESSFUL, 46 tasks executed
tests=120 failures=0 errors=0
ConfigurationBuilderSpec tests=22 failures=0

Verified on the 8.0.x base, not only on the branch this was originally written against.

Some specs use a narrow PropertyResolver proxy to reproduce the Spring 7 failure boundary, because the real resolver auto-converts the top-level bean and would bypass the handler entirely. Raw-map shape and dotted lookups still go through the real DatastoreUtils.createPropertyResolver.

Known limitation

A PropertyResolver that exposes only an aggregate map, without its entries addressable as dotted properties, can still yield null for a configured scalar. Grails' own DatastoreUtils.createPropertyResolver flattens and is unaffected. Binding the raw value unconditionally was considered and rejected because it would bypass the type conversion and case-insensitive enum handling above; both reviewers agreed that would be a net regression. A follow-up could use the raw value only when it is already assignable.

Provenance

This was first written on the Groovy 6 canary branch (#15558), which is simply where the failure was noticed. It is not Groovy-version-specific - both branches are on Groovy 5.0.8 and the code carries its own note to that effect - so it belongs here. Once this reaches 9.0.x through the merge chain, the same change is dropped from #15558.

[Copilot]

Pull request overview

This PR fixes configuration binding for nested settings maps under Spring Framework 7, where Spring no longer auto-converts Map values into Groovy @Builder(builderStrategy = SimpleStrategy) types (leading to ConverterNotFoundException during settings binding).

Changes:

• Adds a defensive fallback in ConfigurationBuilder to instantiate and populate target types from Map when (and only when) Spring genuinely has no converter.
• Preserves strictness and inheritance semantics in the fallback path (unknown keys, malformed values, explicit nulls, map-backed settings, flattened descendant keys).
• Expands ConfigurationBuilderSpec with targeted regression coverage for Spring 7 conversion failure boundaries and the new fallback behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/ConfigurationBuilder.groovy	Implements Spring 7 Map-to-builder-type fallback binding and related conversion/strictness handling.
grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/config/ConfigurationBuilderSpec.groovy	Adds extensive regression tests covering nested-map fallback binding, strictness, inheritance, and flattened key behavior.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[bito-code-review]

The observation is correct. In the handleConverterNotFoundException method, the code iterates over the raw map entries and uses the original key object directly when putting it into the map-backed instance:

((Map) instance).put(key, val)

If the key is not a String (e.g., a GString), this can lead to issues where subsequent lookups using String keys fail, even though the code earlier in the loop correctly normalizes the key to a String for property matching (String propertyName = key.toString()). To ensure consistency and compatibility with String-based lookups, the code should use the normalized propertyName instead of the original key when populating the map.

grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/config/ConfigurationBuilder.groovy

if (mapBacked) {
                        ((Map) instance).put(propertyName, val)
                        return
                    }

[codecov]

Codecov Report

❌ Patch coverage is 59.00000% with 41 lines in your changes missing coverage. Please review.
✅ Project coverage is 53.1571%. Comparing base (54ea208) to head (7d5e684).

Files with missing lines	Patch %	Lines
...tastore/mapping/config/ConfigurationBuilder.groovy	59.0000%	30 Missing and 11 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #16160        +/-   ##
==================================================
+ Coverage     52.3252%   53.1571%   +0.8320%     
- Complexity      18536      19390       +854     
==================================================
  Files            2039       2080        +41     
  Lines           97498      99093      +1595     
  Branches        17138      17387       +249     
==================================================
+ Hits            51016      52675      +1659     
+ Misses          38998      38850       -148     
- Partials         7484       7568        +84     

Files with missing lines	Coverage Δ
...tastore/mapping/config/ConfigurationBuilder.groovy	65.2027% <59.0000%> (-3.2702%)	⬇️

... and 85 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.

[testlens-app]

✅ All tests passed ✅

🏷️ Commit: 7d5e684
▶️ Tests: 63102 executed
⚪️ Checks: 78/78 completed

---

Learn more about TestLens at testlens.app.

[jdaugherty]

@Builder is RUNTIME-retained on Groovy 5.0.8, so an argType annotated with @Builder(builderStrategy = SimpleStrategy) is intercepted by the argType.getAnnotation(Builder) branch earlier in buildRecurse and never reaches this getProperty call. The types that actually land here are ones with no runtime-visible @Builder (like the spec's synthetic settings classes). Two asks:

1. Reword this comment (and the handleConverterNotFoundException javadoc) so it doesn't name @Builder(SimpleStrategy) types as the case being handled here.
2. ConfigurationBuilder fails to bind nested settings under Spring 7 #16159 states the Hibernate and connection-source settings trees are real consumers, but HibernateConnectionSourceSettings, its nested types, ConnectionSourceSettings, and MultiTenancySettings are all annotated and bind through the recursion branch (the pre-existing specs in this file exercise that path and passed on 8.0.x before this change). Which shipped configuration reproduces the startup failure on 8.0.x? Worth capturing in the issue/PR so the affected surface is clear.

[jdaugherty]

This branch and resolveClassValue aren't exercised by the new specs: MapBackedSettings.configClass is declared as String, and no test settings type has a Class-typed property, so the guard listed in the PR table ("Resolve Class entries via the thread context class loader") has no regression test. Could you add coverage for a nested Class-typed property — a Class literal value, a String class name, and an invalid class name?

While adding that, worth pinning the fallback behavior: the top-level argType == Class handling falls back to fallBackValue when no usable value is found, but resolveClassValue returns null, so a value that is neither a Class nor a CharSequence would silently clear an inherited fallback through the setter instead of retaining it (or erroring).

[jdaugherty]

The raw lookup runs even when the typed lookup on the previous line already returned a non-null value, where its result is discarded. It's only needed in the value == null case, so it can move inside that branch and save a resolver pass per nested scalar.