apache / apache/grails-core

ConfigurationBuilder fails to bind nested settings under Spring 7

Open
#16,159 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Groovy
Stars
2.9k
Forks
975
Avg merge
1d 22h
Merged PRs (30d)
92

Description

Spring Framework 7 no longer converts a configuration `Map` into a type annotated `@Builder(builderStrategy = SimpleStrategy)`. `ConfigurationBuilder` relies on that conversion to bind nested settings from `application.yml` / `application.groovy`, so those settings now fail with:

```
org.springframework.core.convert.ConverterNotFoundException
```

**Grails 8 is the first version affected**, because it is the first to ship Spring 7: `8.0.x` pins Spring Boot `4.1.0`, which resolves `spring-core` **7.0.8**. `9.0.x` inherits the same versions, and `ConfigurationBuilder.groovy` plus its spec are byte-identical on both branches, so a fix landed on `8.0.x` flows up the maintenance chain (`8.0.x` -> `8.1.x` -> `9.0.x`) and covers both lines.

This has nothing to do with the Groovy version. Both branches are on Groovy `5.0.8`, and the affected code carries its own note that it is "independent of the Groovy version".

## Reproducing

With the current `ConfigurationBuilder` and the spec added in #16160, six scenarios fail with:

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

## Impact

Nested configuration that binds through a `SimpleStrategy` builder does not apply. Real consumers include the Hibernate and connection-source settings trees. Because the failure surfaces during configuration binding, it shows up at application startup rather than as a test failure.

## Fix

#16160 targets `8.0.x`. `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 it over five rounds and found nine distinct defects in the handling, all of which are fixed and covered by regression tests in that PR:

| # | Defect | Consequence if unguarded |
|---|---|---|
| 1 | Fallback engaged without checking the cause chain for `ConverterNotFoundException` | A converter that deliberately rejects a Map could be bypassed |
| 2 | `catch (Throwable)` swallowed `ConfigurationException` | Unknown-key and malformed-value failures were masked |
| 3 | Raw-lookup failures logged and discarded | Configuration whose lookup failed was silently accepted via the fallback |
| 4 | Fresh instance built without copying the fallback | Every unspecified field was lost, breaking per-field inheritance for named connection settings |
| 5 | Nested resolution passed a null fallback | Inheritance worked only at the first level; deeper children were reset |
| 6 | Map entries assigned without conversion | `multiTenancy.mode: database` was rejected where `DATABASE` worked |
| 7 | `Class` entries resolved via the property resolver | `hibernate.configClass` and other application classes were left unbound, because that converter resolves against the framework class loader |
| 8 | Every key required to be a declared bean property | `HibernateSettings extends LinkedHashMap` exists to carry arbitrary keys such as `hibernate.hbm2ddl.auto`; these were rejected |
| 9 | Flattened descendant keys rejected as unknown | The resolver flattens nested config, so anything nested more than one level failed to build |

Plus a null-varargs fix so an explicit null can 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 covered by guard specs.

## Status

Fixed in #16160 (targets `8.0.x`). `ConfigurationBuilderSpec` goes from 10 to 22 specs and the module total from 108 to 120, all passing on the `8.0.x` base.

One known limitation is documented in that PR: a `PropertyResolver` exposing 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.

## Provenance

The fix was originally written on the Groovy 6 canary branch (#15558), which is simply where the failure was first noticed. It does not belong there, and #16160 moves it to `8.0.x` so both the Grails 8 and Grails 9 lines get it.

Contributor guide

Open the contributing guide

Research direction

Read ConfigurationBuilder.groovy and ConfigurationBuilderSpec, then review the fix and regression coverage in #16160. Run the ConfigurationBuilderSpec and the module suite on the 8.0.x branch; done means the six failing scenarios and the expanded 120-spec module suite pass while the documented strictness guards remain covered.

Written by the indexing model from the issue text.

Assessment

Tech stack
groovy, spring, spring-boot
Domain
backend, testing
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
20/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.