jamesfredley opened a new pull request, #16160:
URL: https://github.com/apache/grails-core/pull/16160
<!-- markdownlint-disable MD033 MD041 -->
Fixes #16159.
## The problem
Spring Framework 7 no longer converts a configuration `Map` into a type
annotated `@Builder(builderStrategy = SimpleStrategy)`, so nested settings fail
to bind with `ConverterNotFoundException`. `9.0.x` resolves `spring-core`
**7.0.8** via Spring Boot 4.1.0, so the affected version is already here.
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'
```
`ConfigurationBuilder` now instantiates the target type and populates it
from the Map when, and only when, Spring genuinely has no converter.
## Why the fallback is this defensive
This code originated on the Groovy 6 canary branch (#15558), where it is not
a Groovy 6 workaround at all - its own docstring says it is "independent of the
Groovy version". Extracting it here removes 261 insertions from that branch and
puts the fix where it belongs.
Extraction put it through two independent reviewers over five rounds, which
surfaced **nine** 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 |
| Let `Map`-backed types keep arbitrary entries | `HibernateSettings extends
LinkedHashMap` exists to carry keys like `hibernate.hbm2ddl.auto`; strict
binding rejected them |
| Bind flattened descendant keys through their parent | The resolver
flattens 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 on Groovy 5.
```
:grails-datastore-core:compileGroovy BUILD SUCCESSFUL
:grails-datastore-core:test --rerun-tasks BUILD SUCCESSFUL, 46 tasks
executed
tests=120 failures=0 errors=0
ConfigurationBuilderSpec tests=22 failures=0
```
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, and not its entries
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 listed above - the
reviewers agreed that would be a net regression. Worth a follow-up that uses
the raw value only when it is already assignable.
## Follow-up
Once this lands, the same change should be dropped from #15558, which then
carries only genuine Groovy 6 workarounds.
--
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]
