Skip to content

Commit 7d5e684

Browse files
committed
fix(config): bind nested settings maps under Spring 7
Spring Framework 7 no longer converts a configuration Map into a type annotated @builder(builderStrategy = SimpleStrategy), so nested settings failed to bind with ConverterNotFoundException. ConfigurationBuilder now instantiates the target type and populates it from the Map. The gap is demonstrable on this branch: with the previous ConfigurationBuilder and only the new spec applied, six scenarios fail with "Expected exception of type 'ConfigurationException', but got 'ConverterNotFoundException'". 9.0.x resolves spring-core 7.0.8 via Spring Boot 4.1.0. The fallback is deliberately narrow, and every guard below exists because removing it produced an observable failure: - It engages only when the cause chain contains ConverterNotFoundException, so a converter that deliberately rejects a Map is not bypassed. - ConfigurationException is never suppressed, so unknown-key and malformed-value failures still surface instead of being masked by the original conversion exception. - A failure while resolving the raw value throws rather than silently falling back, so configuration whose lookup failed is not quietly accepted. - The instance inherits from the fallback before overrides are applied, and each nested level receives its own fallback child, so overriding one field does not discard the rest. - Values are converted to the target property type, including the case-insensitive enum path, so multiTenancy.mode: database still binds. - Class-typed entries resolve through the thread context class loader, the same route the top-level Class handling uses, because the resolver's converter resolves against the framework class loader and would leave an application class such as hibernate.configClass unbound. - Types that are themselves a Map keep arbitrary entries. HibernateSettings extends LinkedHashMap precisely to carry keys like hibernate.hbm2ddl.auto, which strict property-only binding would have rejected. - Flattened descendant keys are bound once through their parent rather than rejected, since the resolver flattens nested configuration; a dotted key whose first segment is unknown is still rejected. - Setters are invoked with an explicit single-element argument array so an explicit null clears an inherited value. ConfigurationBuilderSpec grows from 10 to 22 specs covering each of the above. 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 rejected as a fix because it would bypass the type conversion above. Assisted-by: claude-code:claude-opus-5
1 parent 54ea208 commit 7d5e684

2 files changed

Lines changed: 644 additions & 17 deletions

File tree

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

Lines changed: 202 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
*/
1919
package org.grails.datastore.mapping.config
2020

21+
import java.beans.Introspector
22+
import java.beans.PropertyDescriptor
23+
import java.lang.reflect.InvocationTargetException
2124
import java.lang.reflect.Method
2225
import java.lang.reflect.Modifier
2326

@@ -28,6 +31,7 @@ import groovy.transform.builder.SimpleStrategy
2831
import groovy.util.logging.Slf4j
2932

3033
import org.springframework.core.convert.ConversionFailedException
34+
import org.springframework.core.convert.ConverterNotFoundException
3135
import org.springframework.core.env.PropertyResolver
3236
import org.springframework.util.ClassUtils
3337
import org.springframework.util.ReflectionUtils
@@ -393,23 +397,12 @@ abstract class ConfigurationBuilder<B, C> {
393397
try {
394398
value = propertyResolver.getProperty(propertyPathForArg, argType, fallBackValue)
395399
} catch (ConversionFailedException e) {
396-
if (argType.isEnum()) {
397-
value = propertyResolver.getProperty(propertyPathForArg, String)
398-
if (value != null) {
399-
try {
400-
value = Enum.valueOf((Class) argType, value.toUpperCase())
401-
} catch (Throwable e2) {
402-
// ignore e2 and throw original
403-
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
404-
}
405-
}
406-
else {
407-
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
408-
}
409-
}
410-
else {
411-
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
412-
}
400+
value = handleConversionException(e, argType, propertyPathForArg, fallBackValue)
401+
} catch (ConverterNotFoundException e) {
402+
// Spring 7 nested-map conversion fallback: handle types with
403+
// @Builder(builderStrategy = SimpleStrategy) where Spring cannot
404+
// auto-convert from Map. Independent of the Groovy version.
405+
value = handleConverterNotFoundException(e, argType, propertyPathForArg, fallBackValue)
413406
}
414407
if (value != null) {
415408
log.debug('Resolved value [{}] for setting [{}]', value, propertyPathForArg)
@@ -463,4 +456,196 @@ abstract class ConfigurationBuilder<B, C> {
463456
protected void startBuild(Object builder, String configurationPath) {
464457
// no-op
465458
}
459+
/**
460+
* Handle ConversionFailedException - for enums, try case-insensitive conversion
461+
*/
462+
private Object handleConversionException(ConversionFailedException e, Class argType, String propertyPathForArg, Object fallBackValue) {
463+
if (argType.isEnum()) {
464+
def value = propertyResolver.getProperty(propertyPathForArg, String)
465+
if (value != null) {
466+
try {
467+
return Enum.valueOf((Class) argType, value.toUpperCase())
468+
} catch (IllegalArgumentException e2) {
469+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
470+
}
471+
}
472+
else {
473+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
474+
}
475+
}
476+
else {
477+
ConverterNotFoundException converterNotFoundException = findConverterNotFoundException(e)
478+
if (converterNotFoundException != null) {
479+
return handleConverterNotFoundException(converterNotFoundException, argType, propertyPathForArg, fallBackValue)
480+
}
481+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
482+
}
483+
}
484+
485+
private static ConverterNotFoundException findConverterNotFoundException(Throwable exception) {
486+
Throwable cause = exception
487+
while (cause != null) {
488+
if (cause instanceof ConverterNotFoundException) {
489+
return (ConverterNotFoundException) cause
490+
}
491+
cause = cause.getCause()
492+
}
493+
return null
494+
}
495+
496+
/**
497+
* Handle ConverterNotFoundException - for nested configuration types,
498+
* try to instantiate and populate from Map. This handles Spring 7 compatibility where
499+
* Spring can't auto-convert from LinkedHashMap to these types. This is independent of the
500+
* Groovy version and is required regardless of @Builder annotation retention.
501+
*/
502+
@CompileDynamic
503+
private Object handleConverterNotFoundException(ConverterNotFoundException e, Class argType, String propertyPathForArg, Object fallBackValue, Object rawValue = null) {
504+
if (rawValue == null) {
505+
try {
506+
// Use Object.class to prevent Spring's MapToMapConverter from deep-converting values
507+
rawValue = propertyResolver.getProperty(propertyPathForArg, Object)
508+
} catch (ConfigurationException e2) {
509+
throw e2
510+
} catch (Exception e2) {
511+
throw new ConfigurationException("Cannot read configuration for path [$propertyPathForArg]: $e2.message", e2)
512+
}
513+
}
514+
515+
if (rawValue instanceof Map) {
516+
try {
517+
Map<String, PropertyDescriptor> writableProperties = [:]
518+
Introspector.getBeanInfo(argType).propertyDescriptors.each { PropertyDescriptor property ->
519+
if (property.name != 'metaClass' && property.writeMethod != null) {
520+
writableProperties[property.name] = property
521+
}
522+
}
523+
524+
def instance = argType.getDeclaredConstructor().newInstance()
525+
if (fallBackValue != null && argType.isInstance(fallBackValue)) {
526+
// A map-backed settings type carries arbitrary entries as well as declared
527+
// properties, so the inherited entries have to come across too or overriding
528+
// one nested value would silently drop the rest.
529+
if (instance instanceof Map && fallBackValue instanceof Map) {
530+
((Map) instance).putAll((Map) fallBackValue)
531+
}
532+
writableProperties.values().each { PropertyDescriptor property ->
533+
if (property.readMethod != null && property.readMethod.parameterCount == 0) {
534+
Object fallbackPropertyValue = property.readMethod.invoke(fallBackValue)
535+
property.writeMethod.invoke(instance, [fallbackPropertyValue] as Object[])
536+
}
537+
}
538+
}
539+
540+
boolean mapBacked = instance instanceof Map
541+
Set<String> resolvedProperties = [] as Set<String>
542+
((Map) rawValue).each { key, val ->
543+
String propertyName = key.toString()
544+
PropertyDescriptor property = writableProperties[propertyName]
545+
if (property != null) {
546+
Object fallBackPropertyValue = getFallBackValue(fallBackValue, propertyName)
547+
Object value = resolveMapValue(property.propertyType, "$propertyPathForArg.$propertyName", fallBackPropertyValue, val)
548+
property.writeMethod.invoke(instance, [value] as Object[])
549+
resolvedProperties.add(propertyName)
550+
return
551+
}
552+
int nestedPropertySeparator = propertyName.indexOf('.')
553+
if (nestedPropertySeparator > 0) {
554+
String nestedPropertyName = propertyName.substring(0, nestedPropertySeparator)
555+
PropertyDescriptor nestedProperty = writableProperties[nestedPropertyName]
556+
if (nestedProperty != null) {
557+
if (resolvedProperties.add(nestedPropertyName)) {
558+
Object fallBackPropertyValue = getFallBackValue(fallBackValue, nestedPropertyName)
559+
Object value = resolveMapValue(nestedProperty.propertyType, "$propertyPathForArg.$nestedPropertyName", fallBackPropertyValue, val)
560+
nestedProperty.writeMethod.invoke(instance, [value] as Object[])
561+
}
562+
return
563+
}
564+
}
565+
// Types that are themselves a Map (HibernateSettings extends LinkedHashMap, for
566+
// example) exist precisely to carry arbitrary keys such as hibernate.hbm2ddl.auto,
567+
// so an entry that is not a declared bean property belongs in the map rather than
568+
// being rejected. Only types with a fixed set of properties reject unknown keys.
569+
if (mapBacked) {
570+
((Map) instance).put(key, val)
571+
return
572+
}
573+
throw new ConfigurationException("Unknown setting [$propertyPathForArg.$propertyName]")
574+
}
575+
return instance
576+
} catch (ConfigurationException e2) {
577+
throw e2
578+
} catch (InvocationTargetException e2) {
579+
Throwable cause = e2.targetException
580+
if (cause instanceof Error) {
581+
throw (Error) cause
582+
}
583+
if (cause instanceof ConfigurationException) {
584+
throw (ConfigurationException) cause
585+
}
586+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $cause.message", cause)
587+
} catch (Exception e2) {
588+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e2.message", e2)
589+
}
590+
}
591+
592+
if (rawValue != null) {
593+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: cannot convert value [$rawValue] to required type [$argType.name]", e)
594+
}
595+
596+
// If we have a fallback value, return it
597+
if (fallBackValue != null) {
598+
return fallBackValue
599+
}
600+
601+
if (e != null) {
602+
throw new ConfigurationException("Invalid value for setting [$propertyPathForArg]: $e.message", e)
603+
}
604+
return null
605+
}
606+
607+
private Object resolveClassValue(String propertyPath) {
608+
Object rawValue = propertyResolver.getProperty(propertyPath, Object)
609+
if (rawValue instanceof Class) {
610+
return rawValue
611+
}
612+
String className = rawValue instanceof CharSequence ? rawValue.toString().trim() : null
613+
if (!className) {
614+
return null
615+
}
616+
ClassLoader classLoader = Thread.currentThread().contextClassLoader ?: getClass().classLoader
617+
try {
618+
return ClassUtils.forName(className, classLoader)
619+
} catch (ClassNotFoundException | LinkageError e) {
620+
throw new ConfigurationException("Invalid class name [$className] for setting [$propertyPath]: ${e.message}", e)
621+
}
622+
}
623+
624+
private Object resolveMapValue(Class propertyType, String propertyPath, Object fallBackValue, Object rawValue) {
625+
// Class-typed entries must use the same thread context class loader route as the
626+
// top-level Class handling above, because the resolver's String->Class converter
627+
// resolves against the framework class loader and silently leaves an
628+
// application-defined class (hibernate.configClass, for example) unbound.
629+
if (propertyType == Class) {
630+
return resolveClassValue(propertyPath)
631+
}
632+
if (rawValue instanceof Map && !propertyType.isInstance(rawValue)) {
633+
return handleConverterNotFoundException(null, propertyType, propertyPath, fallBackValue, rawValue)
634+
}
635+
try {
636+
Object value = propertyResolver.getProperty(propertyPath, propertyType)
637+
Object rawPropertyValue = propertyResolver.getProperty(propertyPath, Object)
638+
if (value == null && rawPropertyValue instanceof Map) {
639+
if (propertyType.isInstance(rawPropertyValue)) {
640+
return rawPropertyValue
641+
}
642+
return handleConverterNotFoundException(null, propertyType, propertyPath, fallBackValue, rawPropertyValue)
643+
}
644+
return value
645+
} catch (ConversionFailedException e) {
646+
return handleConversionException(e, propertyType, propertyPath, fallBackValue)
647+
} catch (ConverterNotFoundException e) {
648+
return handleConverterNotFoundException(e, propertyType, propertyPath, fallBackValue)
649+
}
650+
}
466651
}

0 commit comments

Comments
 (0)