jamesfredley opened a new issue, #16159:
URL: https://github.com/apache/grails-core/issues/16159
`ConfigurationBuilder` gained a Spring 7 compatibility path: when Spring
cannot convert a configuration `Map` into a type annotated
`@Builder(builderStrategy = SimpleStrategy)`, the builder instantiates the
target and populates it from the Map. That path currently ships on the Groovy 6
canary branch (#15558), where it is 261 of that branch's insertions despite its
own docstring stating it is *"independent of the Groovy version"*.
An attempt to extract it onto `9.0.x`, where it belongs, ran it through two
independent code reviewers over four rounds. The gap on trunk is real - with
trunk's original `ConfigurationBuilder` and only the new spec applied, six
scenarios fail with:
```
Expected exception of type 'ConfigurationException',
but got 'org.springframework.core.convert.ConverterNotFoundException'
```
`9.0.x` resolves `org.springframework:spring-core:7.0.8` via Spring Boot
4.1.0, so the affected Spring version is present.
However, the review surfaced nine distinct defects in the implementation.
Seven were fixed and covered with tests; two remain open and need someone with
Hibernate configuration-binding knowledge.
## Fixed, with regression tests
| # | Defect | Consequence |
|---|---|---|
| 1 | Every non-enum `ConversionFailedException` triggered the Map fallback
without checking the cause chain contained `ConverterNotFoundException` | A
converter that deliberately rejects a Map could be bypassed |
| 2 | A surrounding `catch (Throwable)` swallowed `ConfigurationException` |
Unknown-key and malformed-value failures were masked by the original exception |
| 3 | Raw-value resolution failures were logged and discarded, leaving the
population-failure flag null | A non-null fallback was returned, silently
accepting configuration whose lookup had failed |
| 4 | Population built a fresh instance without copying the fallback | Every
field the caller did not specify was lost, breaking `buildRecurse` per-field
inheritance for named connection settings |
| 5 | Map entries were assigned raw | Converters and case-insensitive enum
handling were bypassed, so `multiTenancy.mode: database` was rejected where
`DATABASE` worked |
| 6 | Any key that was not a declared bean property was rejected |
`HibernateSettings extends LinkedHashMap<String, String>` exists to carry
arbitrary keys, so `hibernate.hbm2ddl.auto` would be rejected as an unknown
setting |
| 7 | `Method.invoke(instance, value)` hit the Java null-varargs pitfall |
An explicit null could not clear an inherited value |
Coverage went from 10 to 19 specs in `ConfigurationBuilderSpec`, module
total 108 to 117, all passing on Groovy 5.
## Open, not fixed
**A. Flattened descendant keys are rejected.**
`DatastoreUtils.createPropertyResolver` flattens nested configuration, so a
parent map contains both immediate children and flattened descendants -
`hibernate.jpa` holds both `compliance` and `compliance.cascade`. Since
`JpaSettings` is not map-backed, the flattened key reaches the strict
unknown-key branch and is rejected, so configuration nested more than one level
deep fails to build.
**B. Fallback inheritance does not recurse.** When a fallback object has a
nested child and the configuration overrides only part of that child, the
nested resolve runs with a null fallback. The previously copied child is
replaced, silently losing its unspecified fallback fields. Inheritance
therefore works only at the first manually populated level.
Both need validation against real Hibernate and multi-datasource
configuration rather than unit-level mocks, which is why they are being
reported rather than patched speculatively.
## Why this matters now
Defects 1-7 and both open items are **live on the canary branch today**,
since that is where this code currently resides. Items A and B in particular
can reject valid configuration or silently reset inherited settings at
application startup.
## Suggested handling
1. Fix A and B with integration coverage over genuinely nested Hibernate
configuration.
2. Land the whole thing on `9.0.x` as a Spring 7 compatibility fix.
3. Drop it from #15558, which then carries only genuine Groovy 6 workarounds
and roughly halves in size.
The seven fixes plus the 9 added specs exist on the local branch
`fix/spring7-nested-map-conversion` and can be pushed if wanted as a starting
point.
--
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]
