From 5ed809415757ac829727b2663b14229af58012e4 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Mon, 10 Aug 2026 18:59:35 -0500 Subject: [PATCH 01/23] Memoize system environment resolution in NavigableMapConfig Every Config.getProperty call resolved the key against the process environment from scratch. findInSystemEnvironment asked resolvePropertyName for the environment spelling of the key, and checkPropertyName probed System.getenv up to four times per casing - as-is, dots replaced, hyphens replaced, both replaced - then repeated the whole sequence against the uppercased key. That is up to eight getenv probes and six intermediate strings per lookup, and the answer never changes: the process environment is fixed for the lifetime of a config. Resolution is now memoized per config instance, as is the token list a dotted key splits into. Property values themselves are not cached, so configuration that changes at runtime is still observed. The cache is deliberately per-instance rather than static. Tests install environment variables reflectively and then build a fresh config, which must see the environment as it stands at that point. Measured on the grails-core config specs: getProperty drops from 355.3 to 91.6 ns/op over two million lookups. On a scaffolded page rendering 100 rows, NavigableMapConfig falls from 7.01% to 1.16% of JFR execution samples, the fields plugin being the caller that resolves configuration per rendered property. --- .../org/grails/config/NavigableMapConfig.java | 49 ++++++++++++++++-- .../config/SystemEnvironmentConfigSpec.groovy | 50 +++++++++++++++++++ 2 files changed, 94 insertions(+), 5 deletions(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index 42b909cf70f..ef15bb86d9a 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -20,14 +20,17 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Properties; import java.util.Set; import java.util.StringTokenizer; +import java.util.concurrent.ConcurrentHashMap; import groovy.util.ConfigObject; import org.codehaus.groovy.runtime.DefaultGroovyMethods; @@ -53,8 +56,17 @@ @Deprecated public abstract class NavigableMapConfig implements Config { protected static final Logger LOG = LoggerFactory.getLogger(NavigableMapConfig.class); + + /** + * Upper bound on each key-derived cache. Configuration keys come from a small fixed set of source + * literals; the bound only guards against a caller synthesising unbounded key strings at runtime. + */ + private static final int MAX_CACHED_KEYS = 2048; + protected ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); protected ConfigurableConversionService conversionService = new DefaultConversionService(); + private final ConcurrentHashMap> systemEnvironmentCache = new ConcurrentHashMap<>(); + private final ConcurrentHashMap> dotNotatedKeyCache = new ConcurrentHashMap<>(); protected NavigableMap configMap = new NavigableMap() { @Override protected Object mergeMapEntry(NavigableMap targetMap, String sourceKey, Object newValue) { @@ -231,8 +243,26 @@ public T getProperty(String key, Class targetType, T defaultValue) { } private Object findInSystemEnvironment(String key) { + if (key == null) { + return null; + } + Optional cached = systemEnvironmentCache.get(key); + if (cached != null) { + return cached.orElse(null); + } String propertyName = resolvePropertyName(key); - return propertyName != null ? System.getenv(propertyName) : null; + String value = propertyName != null ? System.getenv(propertyName) : null; + // Resolving a key to its environment value depends only on the key and on the process + // environment, which is fixed for the lifetime of this config, so the result is memoized per + // instance. Without this, every getProperty() call spends up to eight System.getenv() probes + // and six string allocations in resolvePropertyName()/checkPropertyName() re-deriving a + // constant answer. The cache is deliberately per-instance rather than static: tests that + // install environment variables reflectively build a fresh config afterwards and must observe + // the environment as it stands then. + if (systemEnvironmentCache.size() < MAX_CACHED_KEYS) { + systemEnvironmentCache.put(key, Optional.ofNullable(value)); + } + return value; } private String resolvePropertyName(String name) { @@ -404,8 +434,17 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k return null; } - List keys = convertTokensIntoArrayList(new StringTokenizer(key, ".")); - if (keys.size() == 0) { + List keys = dotNotatedKeyCache.get(key); + if (keys == null) { + keys = convertTokensIntoArrayList(new StringTokenizer(key, ".")); + // Splitting a dotted key is a pure function of the key, and configuration keys come from a + // small fixed set of source literals, so the token list is cached rather than rebuilt (with + // a fresh StringTokenizer and ArrayList) on every lookup. + if (dotNotatedKeyCache.size() < MAX_CACHED_KEYS) { + dotNotatedKeyCache.put(key, keys); + } + } + if (keys.isEmpty()) { return null; } @@ -421,10 +460,10 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k } private List convertTokensIntoArrayList(StringTokenizer st) { - List elements = new ArrayList<>(); + List elements = new ArrayList<>(st.countTokens()); while (st.hasMoreTokens()) { elements.add(st.nextToken()); } - return elements; + return Collections.unmodifiableList(elements); } } diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index 2adb3591f88..a163973f73c 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -73,6 +73,56 @@ property-with_mixed.symbols: from-yml 'property-with_mixed.symbols' | 'PROPERTY_WITH_MIXED_SYMBOLS' | 'from-env' } + void 'a property read more than once keeps resolving to the system environment value'() { + given: 'configuration that is overridden by the environment' + def config = configFor('property.with.period: from-yml') + modifiableSystemEnvironment.put('PROPERTY_WITH_PERIOD', 'from-env') + + expect: 'every read resolves to the environment value, not just the first' + config.getProperty('property.with.period') == 'from-env' + config.getProperty('property.with.period') == 'from-env' + config.getProperty('property.with.period') == 'from-env' + + cleanup: + modifiableSystemEnvironment.remove('PROPERTY_WITH_PERIOD') + } + + void 'a property that has already been read still reflects later configuration changes'() { + given: 'a property that has been read once' + def config = configFor('some.nested.value: original') + assert config.getProperty('some.nested.value') == 'original' + + when: 'the configuration is changed' + config.merge(['some.nested.value': 'updated']) + + then: 'the new value is returned rather than the previously resolved one' + config.getProperty('some.nested.value') == 'updated' + } + + void 'a config created after an environment variable is installed observes it'() { + given: 'a config created and read before the variable exists' + def before = configFor('late.bound.property: from-yml') + assert before.getProperty('late.bound.property') == 'from-yml' + + when: 'the variable is installed and a new config is created' + modifiableSystemEnvironment.put('LATE_BOUND_PROPERTY', 'from-env') + def after = configFor('late.bound.property: from-yml') + + then: 'the new config resolves to the environment value' + after.getProperty('late.bound.property') == 'from-env' + + cleanup: + modifiableSystemEnvironment.remove('LATE_BOUND_PROPERTY') + } + + private static PropertySourcesConfig configFor(String yaml) { + def yamlPropertiesSource = new YamlPropertySourceLoader() + .load('application.yml', new ByteArrayResource(yaml.bytes, 'test.yml'), null) + def propertySources = new MutablePropertySources() + propertySources.addFirst(yamlPropertiesSource.first()) + new PropertySourcesConfig(propertySources) + } + // From https://github.com/spring-projects/spring-framework/blob/4.3.x/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java#L492 @SuppressWarnings("unchecked") static Map getModifiableSystemEnvironment() { From a332f7693208543c3c7ceaf0616acbff943b3da4 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Wed, 21 Oct 2026 20:31:38 -0400 Subject: [PATCH 02/23] Index the process environment once per config instead of probing it per lookup Config.getProperty resolved every key against the process environment first, on every call, by probing System.getenv with up to eight spellings of the key and allocating six intermediate strings along the way. The answer cannot change: the environment of a running JVM is fixed, and nearly every key is not an environment variable at all, so most of the cost went into proving a miss. Each config now snapshots the environment when it is created and indexes the variable names by their canonical form (upper-cased, with dots and hyphens folded to underscores). Every spelling the lookup probes shares that canonical form, so a key whose canonical form belongs to no variable is rejected with one hash lookup. A key whose canonical form is present runs the same spelling-by-spelling resolution as before against the snapshot, so which variable wins when several match is unchanged. This is how Spring Boot's binder treats the system environment: as an immutable source copied once. The memory this takes is proportional to the number of environment variables, not to the size of the configuration or to the number of distinct keys looked up, and there is nothing to evict. The per-key memoization added earlier on this branch is removed, as is the token-list cache: the dotted-key walk now splits the key with indexOf instead of a StringTokenizer and a list. Upper-casing uses Locale.ROOT, as Spring's SystemEnvironmentPropertySource does, so the resolution no longer depends on the default locale. The environment spec installs each variable before building the config it reads it through, since a config observes the environment it was created with; a running JVM cannot gain variables, only the reflective test helper can. --- .../org/grails/config/NavigableMapConfig.java | 202 +++++++++--------- .../config/PropertySourcesConfigSpec.groovy | 16 ++ .../config/SystemEnvironmentConfigSpec.groovy | 74 +++++-- 3 files changed, 173 insertions(+), 119 deletions(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index ef15bb86d9a..f35f14e2371 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -20,17 +20,16 @@ import java.util.ArrayList; import java.util.Collection; -import java.util.Collections; +import java.util.HashMap; +import java.util.HashSet; import java.util.IdentityHashMap; import java.util.Iterator; import java.util.LinkedHashMap; import java.util.List; +import java.util.Locale; import java.util.Map; -import java.util.Optional; import java.util.Properties; import java.util.Set; -import java.util.StringTokenizer; -import java.util.concurrent.ConcurrentHashMap; import groovy.util.ConfigObject; import org.codehaus.groovy.runtime.DefaultGroovyMethods; @@ -56,17 +55,9 @@ @Deprecated public abstract class NavigableMapConfig implements Config { protected static final Logger LOG = LoggerFactory.getLogger(NavigableMapConfig.class); - - /** - * Upper bound on each key-derived cache. Configuration keys come from a small fixed set of source - * literals; the bound only guards against a caller synthesising unbounded key strings at runtime. - */ - private static final int MAX_CACHED_KEYS = 2048; - protected ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); protected ConfigurableConversionService conversionService = new DefaultConversionService(); - private final ConcurrentHashMap> systemEnvironmentCache = new ConcurrentHashMap<>(); - private final ConcurrentHashMap> dotNotatedKeyCache = new ConcurrentHashMap<>(); + private final SystemEnvironmentIndex systemEnvironment = new SystemEnvironmentIndex(System.getenv()); protected NavigableMap configMap = new NavigableMap() { @Override protected Object mergeMapEntry(NavigableMap targetMap, String sourceKey, Object newValue) { @@ -231,7 +222,7 @@ public T getProperty(String key, Class targetType, T defaultValue, List T getProperty(String key, Class targetType, T defaultValue) { - Object value = findInSystemEnvironment(key); + Object value = systemEnvironment.get(key); if (value == null) { value = getValueWithDotNotatedKeySupport(configMap, key); } @@ -242,72 +233,6 @@ public T getProperty(String key, Class targetType, T defaultValue) { return convertValueIfNecessary(value, targetType, defaultValue); } - private Object findInSystemEnvironment(String key) { - if (key == null) { - return null; - } - Optional cached = systemEnvironmentCache.get(key); - if (cached != null) { - return cached.orElse(null); - } - String propertyName = resolvePropertyName(key); - String value = propertyName != null ? System.getenv(propertyName) : null; - // Resolving a key to its environment value depends only on the key and on the process - // environment, which is fixed for the lifetime of this config, so the result is memoized per - // instance. Without this, every getProperty() call spends up to eight System.getenv() probes - // and six string allocations in resolvePropertyName()/checkPropertyName() re-deriving a - // constant answer. The cache is deliberately per-instance rather than static: tests that - // install environment variables reflectively build a fresh config afterwards and must observe - // the environment as it stands then. - if (systemEnvironmentCache.size() < MAX_CACHED_KEYS) { - systemEnvironmentCache.put(key, Optional.ofNullable(value)); - } - return value; - } - - private String resolvePropertyName(String name) { - String resolvedName = checkPropertyName(name); - if (resolvedName != null) { - return resolvedName; - } - String uppercasedName = name.toUpperCase(); - if (!name.equals(uppercasedName)) { - resolvedName = checkPropertyName(uppercasedName); - if (resolvedName != null) { - return resolvedName; - } - } - return name; - } - - private String checkPropertyName(String name) { - // Check name as-is - if (containsKey(name)) { - return name; - } - // Check name with just dots replaced - String noDotName = name.replace('.', '_'); - if (!name.equals(noDotName) && containsKey(noDotName)) { - return noDotName; - } - // Check name with just hyphens replaced - String noHyphenName = name.replace('-', '_'); - if (!name.equals(noHyphenName) && containsKey(noHyphenName)) { - return noHyphenName; - } - // Check name with dots and hyphens replaced - String noDotNoHyphenName = noDotName.replace('-', '_'); - if (!noDotName.equals(noDotNoHyphenName) && containsKey(noDotNoHyphenName)) { - return noDotNoHyphenName; - } - // Give up - return null; - } - - private boolean containsKey(String name) { - return System.getenv(name) != null; - } - private Map convertToMap(NavigableMap from, IdentityHashMap> cache) { if (cache.containsKey(from)) { return cache.get(from); @@ -434,36 +359,107 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k return null; } - List keys = dotNotatedKeyCache.get(key); - if (keys == null) { - keys = convertTokensIntoArrayList(new StringTokenizer(key, ".")); - // Splitting a dotted key is a pure function of the key, and configuration keys come from a - // small fixed set of source literals, so the token list is cached rather than rebuilt (with - // a fresh StringTokenizer and ArrayList) on every lookup. - if (dotNotatedKeyCache.size() < MAX_CACHED_KEYS) { - dotNotatedKeyCache.put(key, keys); + Object value = null; + boolean first = true; + int length = key.length(); + int start = 0; + while (start < length) { + int end = key.indexOf('.', start); + if (end < 0) { + end = length; } + if (end > start) { + String segment = key.substring(start, end); + if (first) { + value = configMap.get(segment); + first = false; + } else if (value instanceof Map) { + value = ((Map) value).get(segment); + } else { + break; + } + } + start = end + 1; } - if (keys.isEmpty()) { - return null; + return value; + } + + /** + * The process environment as it stood when this config was created, indexed so that a key no + * variable can match is answered with one hash lookup. + *

+ * A key matches a variable in any of the spellings Spring's {@code SystemEnvironmentPropertySource} + * accepts: as written, with dots replaced by underscores, with hyphens replaced by underscores, + * with both replaced, and the upper-cased form of each. Every one of those spellings has the same + * canonical name, so a key whose canonical name belongs to no variable cannot match and is + * rejected without probing. Only a key whose canonical name is present runs the spelling-by-spelling + * resolution, which keeps the precedence between spellings as it was. + *

+ * The environment of a running JVM cannot change, so one snapshot per config instance is exact. + * The memory it takes is proportional to the number of environment variables, not to the size of + * the configuration or to the number of keys looked up. + */ + private static final class SystemEnvironmentIndex { + + private final Map variables; + private final Set canonicalNames; + + SystemEnvironmentIndex(Map environment) { + this.variables = new HashMap<>(environment); + this.canonicalNames = new HashSet<>(); + for (String name : variables.keySet()) { + canonicalNames.add(canonicalName(name)); + } } - Object value = null; - for (int i = 0; i < keys.size(); i++) { - if (i == 0) { - value = configMap.get(keys.get(i)); - } else if (value instanceof Map) { - value = ((Map) value).get(keys.get(i)); + String get(String key) { + if (key == null || !canonicalNames.contains(canonicalName(key))) { + return null; } + return variables.get(resolvePropertyName(key)); } - return value; - } - private List convertTokensIntoArrayList(StringTokenizer st) { - List elements = new ArrayList<>(st.countTokens()); - while (st.hasMoreTokens()) { - elements.add(st.nextToken()); + private static String canonicalName(String name) { + return name.toUpperCase(Locale.ROOT).replace('.', '_').replace('-', '_'); + } + + private String resolvePropertyName(String name) { + String resolvedName = checkPropertyName(name); + if (resolvedName != null) { + return resolvedName; + } + String uppercasedName = name.toUpperCase(Locale.ROOT); + if (!name.equals(uppercasedName)) { + resolvedName = checkPropertyName(uppercasedName); + if (resolvedName != null) { + return resolvedName; + } + } + return name; + } + + private String checkPropertyName(String name) { + // Check name as-is + if (variables.containsKey(name)) { + return name; + } + // Check name with just dots replaced + String noDotName = name.replace('.', '_'); + if (!name.equals(noDotName) && variables.containsKey(noDotName)) { + return noDotName; + } + // Check name with just hyphens replaced + String noHyphenName = name.replace('-', '_'); + if (!name.equals(noHyphenName) && variables.containsKey(noHyphenName)) { + return noHyphenName; + } + // Check name with dots and hyphens replaced + String noDotNoHyphenName = noDotName.replace('-', '_'); + if (!noDotName.equals(noDotNoHyphenName) && variables.containsKey(noDotNoHyphenName)) { + return noDotNoHyphenName; + } + // Give up + return null; } - return Collections.unmodifiableList(elements); } } diff --git a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy index 747532b6f22..e7ab1291f8b 100644 --- a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy @@ -116,4 +116,20 @@ class PropertySourcesConfigSpec extends Specification { then: 'the second value is not merged with the first value' cfg.foo.bar == ['two': '2'] } + + void 'a dotted key walks plain nested maps and skips empty segments'() { + given: "a plain map value that was put without flattened keys" + def config = new PropertySourcesConfig() + config.put('plain', [nested: [value: 'found']]) + + expect: "each segment of the key is resolved against the map it reaches" + config.getProperty('plain.nested.value') == 'found' + config.getProperty('plain.nested', Map) == [value: 'found'] + config.getProperty('plain.missing.value') == null + + and: "empty segments are ignored as they always were" + config.getProperty('plain..nested.value') == 'found' + config.getProperty('.plain.nested.value.') == 'found' + config.getProperty('...') == null + } } diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index a163973f73c..83c6cedbb3d 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -21,7 +21,6 @@ package org.grails.config import org.grails.config.yaml.YamlPropertySourceLoader import org.springframework.core.env.MutablePropertySources import org.springframework.core.io.ByteArrayResource -import org.springframework.core.io.Resource import spock.lang.Issue import spock.lang.Specification @@ -32,25 +31,21 @@ import java.lang.reflect.Field */ class SystemEnvironmentConfigSpec extends Specification { - @Issue('#10670') - void 'configuration properties defined in SystemEnvironment take precedence'() { - given: 'default configuration in application-yml' - Resource resource1 = new ByteArrayResource(''' + private static final String YAML = ''' property.with.period: from-yml property_with_underscore: from-yml property-with-hyphen: from-yml property-with_mixed.symbols: from-yml -'''.bytes, 'test.yml') +''' - def propertySourceLoader = new YamlPropertySourceLoader() - def yamlPropertiesSource = propertySourceLoader.load('application.yml', resource1, null) - def propertySources = new MutablePropertySources() - propertySources.addFirst(yamlPropertiesSource.first()) - def config = new PropertySourcesConfig(propertySources) - - and: 'overriding value with system environment variable' + @Issue('#10670') + void 'configuration properties defined in SystemEnvironment take precedence'() { + given: 'a value in the system environment' modifiableSystemEnvironment.put(systemProperty, value) + and: 'default configuration in application-yml' + def config = configFor(YAML) + expect: config.getProperty(propertyToFind) == value @@ -73,10 +68,54 @@ property-with_mixed.symbols: from-yml 'property-with_mixed.symbols' | 'PROPERTY_WITH_MIXED_SYMBOLS' | 'from-env' } + void 'a key that no environment variable can match resolves from configuration'() { + given: 'configuration with no matching variable in the environment' + def config = configFor(YAML) + + expect: 'the configured value is returned' + config.getProperty('property.with.period') == 'from-yml' + config.getProperty('property-with-hyphen') == 'from-yml' + + and: 'a key that is configured nowhere falls back to the default' + config.getProperty('property.nobody.set', String, 'fallback') == 'fallback' + } + + void 'a variable that matches the key in none of the probed spellings does not override it'() { + given: 'a lower-case variable for a mixed-case key' + modifiableSystemEnvironment.put('property_with_period', 'from-env') + def config = configFor('Property.With.Period: from-yml') + + expect: 'only the key as written and its upper-cased form are probed, as Spring does' + config.getProperty('Property.With.Period') == 'from-yml' + + cleanup: + modifiableSystemEnvironment.remove('property_with_period') + } + + void 'when several variables match a key the first probed spelling wins'() { + given: 'two variables that both match the key' + modifiableSystemEnvironment.put(winner, 'from-winner') + modifiableSystemEnvironment.put(loser, 'from-loser') + def config = configFor(YAML) + + expect: 'the value of the spelling that is probed first is returned' + config.getProperty('property.with.period') == 'from-winner' + + cleanup: + modifiableSystemEnvironment.remove(winner) + modifiableSystemEnvironment.remove(loser) + + where: + winner | loser + 'property.with.period' | 'PROPERTY_WITH_PERIOD' + 'property_with_period' | 'PROPERTY_WITH_PERIOD' + 'property_with_period' | 'PROPERTY.WITH.PERIOD' + } + void 'a property read more than once keeps resolving to the system environment value'() { given: 'configuration that is overridden by the environment' - def config = configFor('property.with.period: from-yml') modifiableSystemEnvironment.put('PROPERTY_WITH_PERIOD', 'from-env') + def config = configFor(YAML) expect: 'every read resolves to the environment value, not just the first' config.getProperty('property.with.period') == 'from-env' @@ -99,8 +138,8 @@ property-with_mixed.symbols: from-yml config.getProperty('some.nested.value') == 'updated' } - void 'a config created after an environment variable is installed observes it'() { - given: 'a config created and read before the variable exists' + void 'each config observes the environment as it stands when the config is created'() { + given: 'a config created before the variable exists' def before = configFor('late.bound.property: from-yml') assert before.getProperty('late.bound.property') == 'from-yml' @@ -111,6 +150,9 @@ property-with_mixed.symbols: from-yml then: 'the new config resolves to the environment value' after.getProperty('late.bound.property') == 'from-env' + and: 'the earlier config keeps the environment it was created with' + before.getProperty('late.bound.property') == 'from-yml' + cleanup: modifiableSystemEnvironment.remove('LATE_BOUND_PROPERTY') } From 9adc51487ee860bc8e941938a9651f3367460822 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Wed, 21 Oct 2026 20:39:07 -0400 Subject: [PATCH 03/23] Treat a dotted key that runs past a scalar as absent getProperty('server.url.port') returned the value of server.url when that value was a string: the dotted walk stopped descending at the first value that is not a map but kept whatever it had reached, so a key naming a child of a scalar resolved to the scalar itself. Dot navigation fails on such a key and Spring's Environment resolves it to nothing, so the walk now reports it as absent and the default applies. --- .../org/grails/config/NavigableMapConfig.java | 3 ++- .../grails/config/PropertySourcesConfigSpec.groovy | 13 +++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index f35f14e2371..b22f6e19175 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -376,7 +376,8 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k } else if (value instanceof Map) { value = ((Map) value).get(segment); } else { - break; + // The key names a child of a scalar, which does not exist whatever the scalar is. + return null; } } start = end + 1; diff --git a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy index e7ab1291f8b..c49cd7db2c4 100644 --- a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy @@ -132,4 +132,17 @@ class PropertySourcesConfigSpec extends Specification { config.getProperty('.plain.nested.value.') == 'found' config.getProperty('...') == null } + + void 'a dotted key that runs past a scalar value is absent'() { + given: "a scalar value" + def config = new PropertySourcesConfig([server: [url: 'http://localhost']]) + + expect: "the scalar itself resolves" + config.getProperty('server.url') == 'http://localhost' + + and: "a key that names a child of the scalar is absent rather than the scalar" + config.getProperty('server.url.port') == null + config.getProperty('server.url.port', String, 'none') == 'none' + !config.containsProperty('server.url.port') + } } From 0bcc737c2fdf77d283ef6a9c170873c7ec74f279 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Wed, 21 Oct 2026 20:39:07 -0400 Subject: [PATCH 04/23] Compile NavigableMap.get statically get carried @CompileDynamic since the 2018 subscript-access change, when its body matched the key against a regex and indexed the matcher dynamically. That logic moved out the same month, leaving a one-line delegateMap.get(key) under dynamic dispatch. Every segment of a dotted getProperty walk goes through this method, so the dispatch cost was paid several times per lookup. --- .../src/main/groovy/org/grails/config/NavigableMap.groovy | 2 -- 1 file changed, 2 deletions(-) diff --git a/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy b/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy index ebed2abc449..3a4b3b73774 100644 --- a/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy +++ b/grails-bootstrap/src/main/groovy/org/grails/config/NavigableMap.groovy @@ -18,7 +18,6 @@ */ package org.grails.config -import groovy.transform.CompileDynamic import groovy.transform.CompileStatic import groovy.transform.EqualsAndHashCode import groovy.util.logging.Slf4j @@ -89,7 +88,6 @@ class NavigableMap implements Map, Cloneable { delegateMap.containsValue(value) } - @CompileDynamic @Override Object get(Object key) { Object result = delegateMap.get(key) From e8f4a78e17d85e4f96697492ca8dbf0a703a535c Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Wed, 21 Oct 2026 20:59:09 -0400 Subject: [PATCH 05/23] Keep Windows matching environment variable names regardless of case System.getenv(String) answers from a case-insensitive copy of the environment on Windows, where the operating system treats variable names that way, while the System.getenv() map the index snapshots keeps the names as given. The snapshot now goes into a case-insensitive map on Windows, so a variable spelled Grails_Foo still satisfies grails.foo there, as it did when every lookup asked System.getenv directly. The environment spec's Windows helper installed a variable only into the case-insensitive copy, which the probes used to read; it now installs into both, since the index reads the other. The cases that depend on exact-case matching run only where names are case-sensitive, and a Windows-only case covers the case-insensitive match. --- .../org/grails/config/NavigableMapConfig.java | 10 ++- .../config/SystemEnvironmentConfigSpec.groovy | 77 ++++++++++++++----- 2 files changed, 67 insertions(+), 20 deletions(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index b22f6e19175..0bc239b5be3 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -30,6 +30,7 @@ import java.util.Map; import java.util.Properties; import java.util.Set; +import java.util.TreeMap; import groovy.util.ConfigObject; import org.codehaus.groovy.runtime.DefaultGroovyMethods; @@ -402,11 +403,18 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k */ private static final class SystemEnvironmentIndex { + /** + * {@code System.getenv(String)} matches a variable name case-insensitively on Windows, where + * the operating system does, and exactly everywhere else. The snapshot keeps that contract. + */ + private static final boolean IS_WINDOWS = System.getProperty("os.name", "").startsWith("Windows"); + private final Map variables; private final Set canonicalNames; SystemEnvironmentIndex(Map environment) { - this.variables = new HashMap<>(environment); + this.variables = IS_WINDOWS ? new TreeMap<>(String.CASE_INSENSITIVE_ORDER) : new HashMap<>(); + this.variables.putAll(environment); this.canonicalNames = new HashSet<>(); for (String name : variables.keySet()) { canonicalNames.add(canonicalName(name)); diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index 83c6cedbb3d..8bb1b31a0e0 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -21,7 +21,9 @@ package org.grails.config import org.grails.config.yaml.YamlPropertySourceLoader import org.springframework.core.env.MutablePropertySources import org.springframework.core.io.ByteArrayResource +import spock.lang.IgnoreIf import spock.lang.Issue +import spock.lang.Requires import spock.lang.Specification import java.lang.reflect.Field @@ -80,6 +82,7 @@ property-with_mixed.symbols: from-yml config.getProperty('property.nobody.set', String, 'fallback') == 'fallback' } + @IgnoreIf(value = { os.windows }, reason = 'Windows matches variable names regardless of case') void 'a variable that matches the key in none of the probed spellings does not override it'() { given: 'a lower-case variable for a mixed-case key' modifiableSystemEnvironment.put('property_with_period', 'from-env') @@ -92,6 +95,20 @@ property-with_mixed.symbols: from-yml modifiableSystemEnvironment.remove('property_with_period') } + @Requires({ os.windows }) + void 'on Windows a variable matches the key in any case, as System.getenv does there'() { + given: 'a mixed-case variable for a lower-case key' + modifiableSystemEnvironment.put('Property_With_Period', 'from-env') + def config = configFor(YAML) + + expect: 'the variable overrides the configured value' + config.getProperty('property.with.period') == 'from-env' + + cleanup: + modifiableSystemEnvironment.remove('Property_With_Period') + } + + @IgnoreIf(value = { os.windows }, reason = 'the spellings that differ only in case are one variable on Windows') void 'when several variables match a key the first probed spelling wins'() { given: 'two variables that both match the key' modifiableSystemEnvironment.put(winner, 'from-winner') @@ -187,7 +204,8 @@ property-with_mixed.symbols: from-yml } } - // for windows + // for windows: System.getenv(String) reads a case-insensitive copy of the environment while + // System.getenv() exposes the original, so a variable installed for a test has to go into both Class processEnvironmentClass; try { processEnvironmentClass = Class.forName("java.lang.ProcessEnvironment"); @@ -196,32 +214,53 @@ property-with_mixed.symbols: from-yml throw new IllegalStateException(ex); } - try { - Field theCaseInsensitiveEnvironmentField = processEnvironmentClass.getDeclaredField("theCaseInsensitiveEnvironment"); - theCaseInsensitiveEnvironmentField.setAccessible(true); - Object obj = theCaseInsensitiveEnvironmentField.get(null); - return (Map) obj; - } - catch (NoSuchFieldException ex) { - // do nothing + Map caseInsensitiveEnvironment = staticMapField(processEnvironmentClass, 'theCaseInsensitiveEnvironment') + Map environment = staticMapField(processEnvironmentClass, 'theEnvironment') + if (caseInsensitiveEnvironment != null && environment != null) { + return new BothEnvironments(caseInsensitiveEnvironment, environment) } - catch (Exception ex) { - throw new IllegalStateException(ex); + if (environment != null) { + return environment } + throw new IllegalStateException() + } + @SuppressWarnings("unchecked") + private static Map staticMapField(Class owner, String name) { try { - Field theEnvironmentField = processEnvironmentClass.getDeclaredField("theEnvironment"); - theEnvironmentField.setAccessible(true); - Object obj = theEnvironmentField.get(null); - return (Map) obj; + Field field = owner.getDeclaredField(name) + field.setAccessible(true) + return (Map) field.get(null) } - catch (NoSuchFieldException ex) { - // do nothing + catch (NoSuchFieldException ignored) { + return null } catch (Exception ex) { - throw new IllegalStateException(ex); + throw new IllegalStateException(ex) } + } + + private static class BothEnvironments implements Map { + + @Delegate + final Map primary + final Map secondary - throw new IllegalStateException(); + BothEnvironments(Map primary, Map secondary) { + this.primary = primary + this.secondary = secondary + } + + @Override + String put(String key, String value) { + secondary.put(key, value) + primary.put(key, value) + } + + @Override + String remove(Object key) { + secondary.remove(key) + primary.remove(key) + } } } From 1e79f8f3fd32489454611b0b3e2aaa20301f9720 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 11:49:20 -0400 Subject: [PATCH 06/23] Revert "Treat a dotted key that runs past a scalar as absent" This reverts commit 9adc51487e. A key that runs past a non-Map value has been answered with that value on every released version, and containsProperty of it has been true. Changing that answer is a change for a major release, so it moves to a 9.0.x pull request together with the List and closure cases, the flat-key states on which the two representations disagree, and the navigate paths that still return the parent. The indexOf walk stays; it leaves the value untouched when a segment lands on a non-Map, as the StringTokenizer walk did. --- .../org/grails/config/NavigableMapConfig.java | 3 +-- .../grails/config/PropertySourcesConfigSpec.groovy | 13 ------------- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index 0bc239b5be3..9b99468d31a 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -377,8 +377,7 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k } else if (value instanceof Map) { value = ((Map) value).get(segment); } else { - // The key names a child of a scalar, which does not exist whatever the scalar is. - return null; + break; } } start = end + 1; diff --git a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy index c49cd7db2c4..e7ab1291f8b 100644 --- a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy @@ -132,17 +132,4 @@ class PropertySourcesConfigSpec extends Specification { config.getProperty('.plain.nested.value.') == 'found' config.getProperty('...') == null } - - void 'a dotted key that runs past a scalar value is absent'() { - given: "a scalar value" - def config = new PropertySourcesConfig([server: [url: 'http://localhost']]) - - expect: "the scalar itself resolves" - config.getProperty('server.url') == 'http://localhost' - - and: "a key that names a child of the scalar is absent rather than the scalar" - config.getProperty('server.url.port') == null - config.getProperty('server.url.port', String, 'none') == 'none' - !config.containsProperty('server.url.port') - } } From dc3e5d19e8da4c87ebe191104c32a1b1e0414d1a Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 11:53:14 -0400 Subject: [PATCH 07/23] Read the environment again when the config is refreshed PropertySourcesConfig.refresh() merges the property sources again, which brings in Spring's live systemEnvironment source under the raw variable names, but the environment index was built once with the config, so after a refresh the dotted spelling of a variable installed since still resolved against the old snapshot. The index is now a volatile field that refreshSystemEnvironment() re-captures, and refresh() calls it before it merges. That also gives a test which installs a variable after the context exists a public way to see it through grailsApplication.config. --- .../org/grails/config/NavigableMapConfig.java | 17 ++++++++++++---- .../grails/config/PropertySourcesConfig.java | 4 ++++ .../config/SystemEnvironmentConfigSpec.groovy | 20 +++++++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index 9b99468d31a..7c84d2124a5 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -58,7 +58,7 @@ public abstract class NavigableMapConfig implements Config { protected static final Logger LOG = LoggerFactory.getLogger(NavigableMapConfig.class); protected ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); protected ConfigurableConversionService conversionService = new DefaultConversionService(); - private final SystemEnvironmentIndex systemEnvironment = new SystemEnvironmentIndex(System.getenv()); + private volatile SystemEnvironmentIndex systemEnvironment = new SystemEnvironmentIndex(System.getenv()); protected NavigableMap configMap = new NavigableMap() { @Override protected Object mergeMapEntry(NavigableMap targetMap, String sourceKey, Object newValue) { @@ -221,6 +221,14 @@ public T getProperty(String key, Class targetType, T defaultValue, List T getProperty(String key, Class targetType, T defaultValue) { Object value = systemEnvironment.get(key); @@ -396,9 +404,10 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k * rejected without probing. Only a key whose canonical name is present runs the spelling-by-spelling * resolution, which keeps the precedence between spellings as it was. *

- * The environment of a running JVM cannot change, so one snapshot per config instance is exact. - * The memory it takes is proportional to the number of environment variables, not to the size of - * the configuration or to the number of keys looked up. + * The environment of a running JVM cannot change, so one snapshot per config instance is exact; + * {@link #refreshSystemEnvironment()} takes a new one for a test that installs variables + * reflectively. The memory it takes is proportional to the number of environment variables, not + * to the size of the configuration or to the number of keys looked up. */ private static final class SystemEnvironmentIndex { diff --git a/grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java b/grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java index 2b941ef5b22..2add2fe7d14 100644 --- a/grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/PropertySourcesConfig.java @@ -80,7 +80,11 @@ public PropertySources getPropertySources() { return propertySources; } + /** + * Reads the process environment again and merges the property sources again. + */ public void refresh() { + refreshSystemEnvironment(); initializeFromPropertySources(propertySources); } diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index 8bb1b31a0e0..ef9dfdd2e1f 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -174,6 +174,26 @@ property-with_mixed.symbols: from-yml modifiableSystemEnvironment.remove('LATE_BOUND_PROPERTY') } + void 'a variable installed after the config exists is seen once the config is refreshed'() { + given: 'a config created before the variable exists' + def config = configFor('late.bound.property: from-yml') + + when: 'the variable is installed' + modifiableSystemEnvironment.put('LATE_BOUND_PROPERTY', 'from-env') + + then: 'the config keeps the environment it was created with' + config.getProperty('late.bound.property') == 'from-yml' + + when: 'the config is refreshed' + config.refresh() + + then: 'the variable overrides the configured value' + config.getProperty('late.bound.property') == 'from-env' + + cleanup: + modifiableSystemEnvironment.remove('LATE_BOUND_PROPERTY') + } + private static PropertySourcesConfig configFor(String yaml) { def yamlPropertiesSource = new YamlPropertySourceLoader() .load('application.yml', new ByteArrayResource(yaml.bytes, 'test.yml'), null) From c58e746a1898f18844a48b31fd793774e79b6e87 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 11:55:25 -0400 Subject: [PATCH 08/23] Reject a key no variable can match without allocating MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The pre-filter built the canonical name of the key with toUpperCase and two replace calls, which allocate for a typical lower-case dotted key, and then hashed the fresh string. It now folds each character to its canonical form while accumulating String.hashCode over them and looks the hash up in the sorted hashes of the variables' canonical names; only a key whose hash is present builds the name for the exact check before the spelling-by-spelling resolution. A character above ASCII can upper-case to more than one character, as ß does, so a key that contains one takes the exact check directly. The two collections are sized up front. On the description's harness a lookup goes from 141 to 104 ns here. --- .../org/grails/config/NavigableMapConfig.java | 40 ++++++++++++++++--- 1 file changed, 34 insertions(+), 6 deletions(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index 7c84d2124a5..4449453a8fd 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -19,6 +19,7 @@ package org.grails.config; import java.util.ArrayList; +import java.util.Arrays; import java.util.Collection; import java.util.HashMap; import java.util.HashSet; @@ -395,14 +396,16 @@ private Object getValueWithDotNotatedKeySupport(NavigableMap configMap, String k /** * The process environment as it stood when this config was created, indexed so that a key no - * variable can match is answered with one hash lookup. + * variable can match is answered without allocating. *

* A key matches a variable in any of the spellings Spring's {@code SystemEnvironmentPropertySource} * accepts: as written, with dots replaced by underscores, with hyphens replaced by underscores, * with both replaced, and the upper-cased form of each. Every one of those spellings has the same * canonical name, so a key whose canonical name belongs to no variable cannot match and is - * rejected without probing. Only a key whose canonical name is present runs the spelling-by-spelling - * resolution, which keeps the precedence between spellings as it was. + * rejected without probing: the hash of its canonical name is computed over its characters and + * looked up among the hashes of the variables' canonical names. Only a key whose hash is present + * builds the canonical name for the exact check and then runs the spelling-by-spelling resolution, + * which keeps the precedence between spellings as it was. *

* The environment of a running JVM cannot change, so one snapshot per config instance is exact; * {@link #refreshSystemEnvironment()} takes a new one for a test that installs variables @@ -419,23 +422,48 @@ private static final class SystemEnvironmentIndex { private final Map variables; private final Set canonicalNames; + private final int[] canonicalHashes; SystemEnvironmentIndex(Map environment) { - this.variables = IS_WINDOWS ? new TreeMap<>(String.CASE_INSENSITIVE_ORDER) : new HashMap<>(); + this.variables = IS_WINDOWS ? new TreeMap<>(String.CASE_INSENSITIVE_ORDER) : HashMap.newHashMap(environment.size()); this.variables.putAll(environment); - this.canonicalNames = new HashSet<>(); + this.canonicalNames = HashSet.newHashSet(variables.size()); for (String name : variables.keySet()) { canonicalNames.add(canonicalName(name)); } + this.canonicalHashes = canonicalNames.stream().mapToInt(String::hashCode).sorted().toArray(); } String get(String key) { - if (key == null || !canonicalNames.contains(canonicalName(key))) { + if (key == null || !mayMatch(key)) { return null; } return variables.get(resolvePropertyName(key)); } + /** + * Whether the canonical name of the key belongs to a variable. The name's hash is computed + * over the key's characters without building the name, and only a key whose hash is present + * builds it for the exact check. A character above ASCII can upper-case to more than one + * character, as {@code ß} does, so a key that contains one takes the exact check directly. + */ + private boolean mayMatch(String key) { + int hash = 0; + for (int i = 0, length = key.length(); i < length; i++) { + char c = key.charAt(i); + if (c > 127) { + return canonicalNames.contains(canonicalName(key)); + } + if (c == '.' || c == '-') { + c = '_'; + } else if (c >= 'a' && c <= 'z') { + c = (char) (c - ('a' - 'A')); + } + hash = 31 * hash + c; + } + return Arrays.binarySearch(canonicalHashes, hash) >= 0 && canonicalNames.contains(canonicalName(key)); + } + private static String canonicalName(String name) { return name.toUpperCase(Locale.ROOT).replace('.', '_').replace('-', '_'); } From 9677aec218d3e31a9ff9e1745c06d47718870146 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 11:56:52 -0400 Subject: [PATCH 09/23] Pin the null key, the root locale and each spelling in the environment spec Two behaviour changes the description lists had no feature: a null key resolves to the default value rather than the NullPointerException that System.getenv(null) raised, and variable names are upper-cased with Locale.ROOT, so a key containing i matches its upper-cased variable under a Turkish default locale. The precedence table also gave every row the same value, so a row could not tell its own spelling from a sibling's or from a variable leaked by an earlier feature; each row now carries its own. --- .../config/SystemEnvironmentConfigSpec.groovy | 54 ++++++++++++++----- 1 file changed, 41 insertions(+), 13 deletions(-) diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index ef9dfdd2e1f..9d4a5225a2c 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -55,19 +55,47 @@ property-with_mixed.symbols: from-yml modifiableSystemEnvironment.remove(systemProperty) where: - propertyToFind | systemProperty | value - 'property.with.period' | 'property.with.period' | 'from-env' - 'property.with.period' | 'property_with_period' | 'from-env' - 'property.with.period' | 'PROPERTY_WITH_PERIOD' | 'from-env' - 'property_with_underscore' | 'property_with_underscore' | 'from-env' - 'property_with_underscore' | 'PROPERTY_WITH_UNDERSCORE' | 'from-env' - 'property-with-hyphen' | 'property-with-hyphen' | 'from-env' - 'property-with-hyphen' | 'PROPERTY-WITH-HYPHEN' | 'from-env' - 'property-with-hyphen' | 'PROPERTY_WITH_HYPHEN' | 'from-env' - 'property-with_mixed.symbols' | 'property-with_mixed.symbols' | 'from-env' - 'property-with_mixed.symbols' | 'property_with_mixed_symbols' | 'from-env' - 'property-with_mixed.symbols' | 'PROPERTY-WITH_MIXED.SYMBOLS' | 'from-env' - 'property-with_mixed.symbols' | 'PROPERTY_WITH_MIXED_SYMBOLS' | 'from-env' + propertyToFind | systemProperty + 'property.with.period' | 'property.with.period' + 'property.with.period' | 'property_with_period' + 'property.with.period' | 'PROPERTY_WITH_PERIOD' + 'property_with_underscore' | 'property_with_underscore' + 'property_with_underscore' | 'PROPERTY_WITH_UNDERSCORE' + 'property-with-hyphen' | 'property-with-hyphen' + 'property-with-hyphen' | 'PROPERTY-WITH-HYPHEN' + 'property-with-hyphen' | 'PROPERTY_WITH_HYPHEN' + 'property-with_mixed.symbols' | 'property-with_mixed.symbols' + 'property-with_mixed.symbols' | 'property_with_mixed_symbols' + 'property-with_mixed.symbols' | 'PROPERTY-WITH_MIXED.SYMBOLS' + 'property-with_mixed.symbols' | 'PROPERTY_WITH_MIXED_SYMBOLS' + value = 'from-' + systemProperty + } + + void 'a key is matched to its upper-cased variable under any default locale'() { + given: 'a default locale whose upper-case i is not I' + Locale defaultLocale = Locale.getDefault() + Locale.setDefault(Locale.forLanguageTag('tr-TR')) + + and: 'a variable that is the upper-cased key' + modifiableSystemEnvironment.put('PROPERTY_WITH_ID', 'from-env') + def config = configFor('property.with.id: from-yml') + + expect: 'the variable overrides the configured value' + config.getProperty('property.with.id') == 'from-env' + + cleanup: + modifiableSystemEnvironment.remove('PROPERTY_WITH_ID') + Locale.setDefault(defaultLocale) + } + + void 'a null key resolves to the default value'() { + given: 'any configuration' + def config = configFor(YAML) + + expect: 'a null key is absent rather than an error from the environment probe' + config.getProperty(null) == null + config.getProperty(null, String, 'dflt') == 'dflt' + !config.containsProperty(null) } void 'a key that no environment variable can match resolves from configuration'() { From 5eb46c740d520cf6d1cb5ae360f8502ccd17dc9f Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 11:57:17 -0400 Subject: [PATCH 10/23] State the preconditions of two environment features in expect blocks --- .../grails/config/SystemEnvironmentConfigSpec.groovy | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index 9d4a5225a2c..1d13eb02639 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -172,9 +172,11 @@ property-with_mixed.symbols: from-yml } void 'a property that has already been read still reflects later configuration changes'() { - given: 'a property that has been read once' + given: 'a configured property' def config = configFor('some.nested.value: original') - assert config.getProperty('some.nested.value') == 'original' + + expect: 'it resolves to the configured value' + config.getProperty('some.nested.value') == 'original' when: 'the configuration is changed' config.merge(['some.nested.value': 'updated']) @@ -186,7 +188,9 @@ property-with_mixed.symbols: from-yml void 'each config observes the environment as it stands when the config is created'() { given: 'a config created before the variable exists' def before = configFor('late.bound.property: from-yml') - assert before.getProperty('late.bound.property') == 'from-yml' + + expect: 'it resolves to the configured value' + before.getProperty('late.bound.property') == 'from-yml' when: 'the variable is installed and a new config is created' modifiableSystemEnvironment.put('LATE_BOUND_PROPERTY', 'from-env') From 777c0ca9e99b9a02e5f4034cc333c1b92d8da165 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 11:57:50 -0400 Subject: [PATCH 11/23] Document the environment snapshot in the 8.1 upgrade guide and what's new The snapshot is visible to a test that installs environment variables after the application context exists, so the upgrade guide says to install them before the context or to call refresh() on the configuration, and notes the Locale.ROOT upper-casing and the null key resolving to the default. The what's new page gets a short entry for the faster lookup. --- .../src/en/guide/introduction/whatsNew.adoc | 4 ++++ .../src/en/guide/upgrading/upgrading81x.adoc | 17 +++++++++++++++++ 2 files changed, 21 insertions(+) diff --git a/grails-doc/src/en/guide/introduction/whatsNew.adoc b/grails-doc/src/en/guide/introduction/whatsNew.adoc index a864d817b9a..9065c125c77 100644 --- a/grails-doc/src/en/guide/introduction/whatsNew.adoc +++ b/grails-doc/src/en/guide/introduction/whatsNew.adoc @@ -691,6 +691,10 @@ scheduler does not know about explains why it is not registered — neither ends any more. See <> for what a job that outlives its trigger's interval needs, and <> for the details. +==== Faster Configuration Lookups + +`grailsApplication.config.getProperty` no longer probes the process environment on every call. The environment is read once, when the configuration is created, and its variable names are indexed, so a property that no variable can match is answered without touching `System.getenv`, and the walk through a dotted key no longer tokenises it. A test that installs environment variables after the application has started calls `refresh()` on the configuration to have them seen; see xref:upgrading#upgrading81x[the upgrade guide]. + ==== Compiled Tag Resolution Tag libraries are now described when they are compiled, and that description resolves tag calls in diff --git a/grails-doc/src/en/guide/upgrading/upgrading81x.adoc b/grails-doc/src/en/guide/upgrading/upgrading81x.adoc index c125715b722..e37da2d01d3 100644 --- a/grails-doc/src/en/guide/upgrading/upgrading81x.adoc +++ b/grails-doc/src/en/guide/upgrading/upgrading81x.adoc @@ -51,3 +51,20 @@ The new `trimLogicLines` setting is opt-in. See link:#pageDirectives[Page Direct ==== 3. Forge Project Generation Default projects consistently include Mockito with Spock. When both Grails Data for MongoDB and the synchronous MongoDB driver are selected, Grails Data owns the MongoDB URLs: development and test use embedded databases, while production uses the configured external server. + +==== 4. Environment Variables Are Read Once Per Configuration + +`grailsApplication.config` resolves every property against the process environment before it consults the configuration, so that a variable such as `GRAILS_SERVERURL` overrides `grails.serverURL`. Grails 8.0 probed `System.getenv` with up to eight spellings of the key on every lookup. Grails 8.1 reads the environment once, when the configuration is created, and indexes the variable names, so a lookup that no variable can match is answered without touching the environment. + +===== Required Actions + +For applications **no changes are required**: the environment of a running JVM cannot change, so every lookup has the same answer as before. + +A test that installs environment variables reflectively, with a library such as System Stubs or JUnit Pioneer, must install them before the application context is created, or ask the configuration to read the environment again afterwards: + +[source,groovy] +---- +grailsApplication.config.refresh() +---- + +Two smaller differences: variable names are upper-cased with `Locale.ROOT`, as Spring does, so a key containing `i` matches its upper-cased variable under a Turkish default locale too; and `getProperty(null)` returns the default value instead of throwing a `NullPointerException`. From 3c61a8142f3983809c4f59fe451945f2839139eb Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 12:09:59 -0400 Subject: [PATCH 12/23] Cover the non-ASCII fallback of the environment pre-filter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A key with a character above ASCII, such as ß, cannot be folded per character because its upper-case form has two letters, so the pre-filter builds the canonical name for such a key instead. Nothing exercised that path, so deleting it would have kept every spec green. straße.x now has to match STRASSE_X. --- .../grails/config/SystemEnvironmentConfigSpec.groovy | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index 1d13eb02639..a83d64a7d15 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -88,6 +88,18 @@ property-with_mixed.symbols: from-yml Locale.setDefault(defaultLocale) } + void 'a key with a character above ASCII is matched through its upper-cased form'() { + given: 'a variable that is the upper-cased key, in which the sharp s has become two letters' + modifiableSystemEnvironment.put('STRASSE_X', 'from-env') + def config = configFor('stra\u00dfe.x: from-yml') + + expect: 'the variable overrides the configured value' + config.getProperty('stra\u00dfe.x') == 'from-env' + + cleanup: + modifiableSystemEnvironment.remove('STRASSE_X') + } + void 'a null key resolves to the default value'() { given: 'any configuration' def config = configFor(YAML) From 66e9bb2abea433cd4e96bbafba8520b0ea82f214 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 12:10:22 -0400 Subject: [PATCH 13/23] Pin the answer for a key that runs past a scalar The revert restored the answer but nothing asserted it, so the walk could have changed it again without a failing test. The 9.0.x change that makes such a key absent will flip this assertion on purpose. --- .../groovy/org/grails/config/PropertySourcesConfigSpec.groovy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy index e7ab1291f8b..417557ae42c 100644 --- a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy @@ -131,5 +131,9 @@ class PropertySourcesConfigSpec extends Specification { config.getProperty('plain..nested.value') == 'found' config.getProperty('.plain.nested.value.') == 'found' config.getProperty('...') == null + + and: "a key that runs past a scalar answers with that scalar, as it always has" + config.getProperty('plain.nested.value.deeper') == 'found' + config.containsProperty('plain.nested.value.deeper') } } From 85a942591faa87913b1a0f1fa11b79fd6285942b Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 12:10:37 -0400 Subject: [PATCH 14/23] Change the default locale only after the variable is installed The locale feature set tr-TR before it touched the reflective environment, so a failure to open the environment would have left tr-TR as the default for the rest of the test JVM. The locale is now the last thing the feature changes and the first thing its cleanup restores. --- .../org/grails/config/SystemEnvironmentConfigSpec.groovy | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index a83d64a7d15..57da02a8502 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -72,20 +72,22 @@ property-with_mixed.symbols: from-yml } void 'a key is matched to its upper-cased variable under any default locale'() { - given: 'a default locale whose upper-case i is not I' + given: 'the default locale, restored afterwards' Locale defaultLocale = Locale.getDefault() - Locale.setDefault(Locale.forLanguageTag('tr-TR')) and: 'a variable that is the upper-cased key' modifiableSystemEnvironment.put('PROPERTY_WITH_ID', 'from-env') def config = configFor('property.with.id: from-yml') + and: 'a default locale whose upper-case i is not I' + Locale.setDefault(Locale.forLanguageTag('tr-TR')) + expect: 'the variable overrides the configured value' config.getProperty('property.with.id') == 'from-env' cleanup: - modifiableSystemEnvironment.remove('PROPERTY_WITH_ID') Locale.setDefault(defaultLocale) + modifiableSystemEnvironment.remove('PROPERTY_WITH_ID') } void 'a key with a character above ASCII is matched through its upper-cased form'() { From e9fc57f9409bfffe557d9549b2f24902165f357c Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 12:10:37 -0400 Subject: [PATCH 15/23] Say what refresh() does and does not do in the upgrade guide Only getProperty consults the environment, so the section no longer says the configuration resolves every property against it. refresh() is on the configuration implementation rather than on the Config interface, so it is a call for dynamically compiled test code; it merges the property sources again, which replaces a value a test set directly wherever a source defines the key; and it has to be called again once the variables are removed, because the configuration keeps the environment it last read. --- grails-doc/src/en/guide/upgrading/upgrading81x.adoc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/grails-doc/src/en/guide/upgrading/upgrading81x.adoc b/grails-doc/src/en/guide/upgrading/upgrading81x.adoc index e37da2d01d3..ce16d5f1960 100644 --- a/grails-doc/src/en/guide/upgrading/upgrading81x.adoc +++ b/grails-doc/src/en/guide/upgrading/upgrading81x.adoc @@ -54,7 +54,7 @@ Default projects consistently include Mockito with Spock. When both Grails Data ==== 4. Environment Variables Are Read Once Per Configuration -`grailsApplication.config` resolves every property against the process environment before it consults the configuration, so that a variable such as `GRAILS_SERVERURL` overrides `grails.serverURL`. Grails 8.0 probed `System.getenv` with up to eight spellings of the key on every lookup. Grails 8.1 reads the environment once, when the configuration is created, and indexes the variable names, so a lookup that no variable can match is answered without touching the environment. +`getProperty` on `grailsApplication.config` resolves a property against the process environment before it consults the configuration, so that a variable such as `GRAILS_SERVERURL` overrides `grails.serverURL`. Grails 8.0 probed `System.getenv` with up to eight spellings of the key on every lookup. Grails 8.1 reads the environment once, when the configuration is created, and indexes the variable names, so a lookup that no variable can match is answered without touching the environment. ===== Required Actions @@ -67,4 +67,6 @@ A test that installs environment variables reflectively, with a library such as grailsApplication.config.refresh() ---- +`refresh()` is a method of the configuration implementation rather than of the `Config` interface, so it is a call for dynamically compiled test code. It reads the environment again and merges the property sources again, which replaces a value the test set on the configuration directly wherever a property source defines the same key. Call it again once the variables are removed: the configuration keeps the environment it last read. + Two smaller differences: variable names are upper-cased with `Locale.ROOT`, as Spring does, so a key containing `i` matches its upper-cased variable under a Turkish default locale too; and `getProperty(null)` returns the default value instead of throwing a `NullPointerException`. From 83b6c5d9218fe191dae4afeff83970737171689a Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Thu, 22 Oct 2026 10:53:38 -0500 Subject: [PATCH 16/23] fix(hibernate7): leave the criteria's orders out of a count query A count has one row whatever the order, and an ORDER BY on a column that is not aggregated fails on H2 and most other databases, so `count()` on a DetachedCriteria or a criteria builder with an `order` threw a SQLGrammarException ("Column ... must be in the GROUP BY list"). Only the paged-list total escaped it, because it clears the orders first. JpaCriteriaQueryCreator now skips the orders when every projection of the query is a count, as `count()` and `countDistinct()` set it up. A query that groups and counts keeps its orders. (cherry picked from commit ebc4b2a4e3bfbb4147513a627575fad7a1639a41) --- .../query/JpaCriteriaQueryCreator.java | 16 ++++++++++++++ .../HibernateCriteriaBuilderDirectSpec.groovy | 22 +++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java index 6f632533799..879c0016b3c 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java @@ -272,6 +272,11 @@ private Class resolveProjectionType(Query.PropertyProjection projection) { @SuppressWarnings("unchecked") private void assignOrderBy(CriteriaQuery cq, JpaQueryContext context) { + if (isCountOnly()) { + // a count returns one row whatever the order, and an ORDER BY on a column that is not aggregated fails + // on H2 and most other databases, so a count query leaves the criteria's orders out + return; + } List orders = detachedCriteria.getOrders(); if (!orders.isEmpty()) { var jpaOrders = orders.stream() @@ -296,6 +301,17 @@ private void assignOrderBy(CriteriaQuery cq, JpaQueryContext context) { } } + /** + * Whether the query selects nothing but counts, as {@code count()} and {@code countDistinct()} do: such a query + * has one row, so the criteria's orders do not apply to it. A query that groups and counts keeps its orders. + */ + private boolean isCountOnly() { + List projectionList = projections.getProjectionList(); + return !projectionList.isEmpty() && projectionList.stream() + .allMatch(projection -> projection instanceof Query.CountProjection || + projection instanceof Query.CountDistinctProjection); + } + private void discoverAliases(List criteria, JpaQueryContext context) { if (criteria == null) return; for (Query.Criterion criterion : criteria) { diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy index 4943cd042c8..01b510191ce 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy @@ -389,6 +389,28 @@ class HibernateCriteriaBuilderDirectSpec extends HibernateGormDatastoreSpec { cleanup: PredicateGenerator.clearCustomCriterionHandlers() } + + void "a count query leaves the criteria's orders out"() { + expect: "a count with an order counts every row instead of failing on the order by" + new DetachedCriteria(CriteriaTestEntity).build { order('amount', 'desc') }.count() == 4L + new DetachedCriteria(CriteriaTestEntity).build { + gt('amount', 15) + order('name') + }.count() == 3L + + and: "a row count projection with an order returns the count" + c.list { + projections { rowCount() } + order('amount') + } == [4L] + + and: "the builder's count with an order counts every row" + new HibernateCriteriaBuilder(CriteriaTestEntity, manager.hibernateDatastore.sessionFactory, manager.hibernateDatastore) + .count { order('amount', 'desc') } == 4L + + and: "the order still applies to the list" + new DetachedCriteria(CriteriaTestEntity).build { order('amount', 'desc') }.list()*.name == ['D', 'C', 'B', 'A'] + } } @Entity From 66f1f26e20776b349f0addfe41b5a08c618ba39c Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Thu, 22 Oct 2026 11:56:02 -0500 Subject: [PATCH 17/23] Hibernate 7: give criterion handlers the query context and let a handler order by an expression Resolves #16563 on 8.x. A plugin or an application can extend the Hibernate 7 criteria builder with its own criteria and orders. - A criterion handler can take the query's QueryPathContext, a small interface that JpaQueryContext implements (getRoot, getFullyQualifiedPath, getFullyQualifiedExpression). Through it a handler resolves alias.property or association.property to the join GORM created for the association, so a custom criterion and a built-in one on the same association restrict the same row instead of each joining it again. The four-argument handler keeps working; ContextualCriterionHandler is the five-argument form, and a closure with five parameters is coerced to it. - An OrderHandler turns a Query.Order subclass into a JPA order by any expression, in its place among the query's orders, registered in code or through a META-INF/services OrderHandlerProvider. - Handler discovery waits for the providers: loadServiceProviders loads under a lock and publishes its flag only after both ServiceLoader loops, so a query that arrives during the discovery no longer decides, with the handlers still unregistered, that an order has no handler. A handler registered in code stays ahead of a discovered one. - A count query leaves its orders out, handled ones too (the fix of the previous commit), so no handler runs for a count. Covered by HibernateCriteriaBuilderDirectSpec (a contextual handler on an association path before and after the association block, an order handler through the builder and DetachedCriteria.list(), a count with a handled order), OrderHandlerDiscoverySpec (a lookup during a blocked discovery waits for the handler), PredicateGeneratorSpec and JpaCriteriaQueryCreatorSpec. Documented in the Hibernate 7 guide and the what's new; the criteria-extension example exercises both handler kinds through providers. This is the 8.1.x version of #16696, which becomes its merge-up onto the Java-to-Groovy Hibernate 7 branch. --- .../query/JpaCriteriaQueryCreator.java | 51 +++-- .../orm/hibernate/query/JpaQueryContext.java | 5 +- .../hibernate/query/PredicateGenerator.java | 189 +++++++++++++++++- .../orm/hibernate/query/QueryPathContext.java | 49 +++++ .../JpaCriteriaQueryCreatorSpec.groovy | 58 ++++++ .../OrderHandlerDiscoverySpec.groovy | 115 +++++++++++ .../PredicateGeneratorSpec.groovy | 42 ++++ .../HibernateCriteriaBuilderDirectSpec.groovy | 102 ++++++++++ ...ry.PredicateGenerator$OrderHandlerProvider | 19 ++ .../src/docs/asciidoc/querying/criteria.adoc | 88 ++++++++ .../src/en/guide/introduction/whatsNew.adoc | 10 + .../example/ProductSearchService.groovy | 11 + .../example/CriteriaExtensionSpec.groovy | 36 ++++ .../HibernateCriteriaBuilderExtensions.groovy | 16 ++ .../NumberLikeCriterionHandlerProvider.groovy | 10 +- .../example/query/PriceDistanceOrder.groovy | 40 ++++ .../PriceDistanceOrderHandlerProvider.groovy | 49 +++++ ...ry.PredicateGenerator$OrderHandlerProvider | 1 + 18 files changed, 859 insertions(+), 32 deletions(-) create mode 100644 grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java create mode 100644 grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy create mode 100644 grails-data-hibernate7/core/src/test/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider create mode 100644 grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrder.groovy create mode 100644 grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrderHandlerProvider.groovy create mode 100644 grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java index 879c0016b3c..c5c9cbbd08c 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaCriteriaQueryCreator.java @@ -274,30 +274,41 @@ private Class resolveProjectionType(Query.PropertyProjection projection) { private void assignOrderBy(CriteriaQuery cq, JpaQueryContext context) { if (isCountOnly()) { // a count returns one row whatever the order, and an ORDER BY on a column that is not aggregated fails - // on H2 and most other databases, so a count query leaves the criteria's orders out + // on H2 and most other databases, so a count query leaves the criteria's orders out, handled ones too return; } List orders = detachedCriteria.getOrders(); if (!orders.isEmpty()) { - var jpaOrders = orders.stream() - .map(order -> { - var propertyName = order.getProperty(); - SqlProjection sqlProjection = findSqlProjection(propertyName, false); - Expression expression = sqlProjection != null ? - context.getSelectionAlias(sqlProjection.getColumnAlias()) : - context.getFullyQualifiedExpression(propertyName); - if (order.isIgnoreCase() && expression.getJavaType().equals(String.class)) { - return order.getDirection().equals(Query.Order.Direction.ASC) ? - criteriaBuilder.asc(criteriaBuilder.lower((Expression) expression)) : - criteriaBuilder.desc(criteriaBuilder.lower((Expression) expression)); - } else { - return order.getDirection().equals(Query.Order.Direction.ASC) ? - criteriaBuilder.asc(expression) : - criteriaBuilder.desc(expression); - } - }) - .toArray(jakarta.persistence.criteria.Order[]::new); - cq.orderBy(jpaOrders); + List jpaOrders = new ArrayList<>(); + for (Query.Order order : orders) { + PredicateGenerator.OrderHandler customHandler = PredicateGenerator.orderHandler(order.getClass()); + if (customHandler != null) { + // the handler of a Query.Order subclass builds its JPA order, or leaves the order out + jakarta.persistence.criteria.Order customOrder = + customHandler.handle(cq, context.getRoot(), criteriaBuilder, order, context); + if (customOrder != null) { + jpaOrders.add(customOrder); + } + continue; + } + String propertyName = order.getProperty(); + SqlProjection sqlProjection = findSqlProjection(propertyName, false); + Expression expression = sqlProjection != null ? + context.getSelectionAlias(sqlProjection.getColumnAlias()) : + context.getFullyQualifiedExpression(propertyName); + jakarta.persistence.criteria.Order jpaOrder; + if (order.isIgnoreCase() && expression.getJavaType().equals(String.class)) { + jpaOrder = order.getDirection().equals(Query.Order.Direction.ASC) ? + criteriaBuilder.asc(criteriaBuilder.lower((Expression) expression)) : + criteriaBuilder.desc(criteriaBuilder.lower((Expression) expression)); + } else { + jpaOrder = order.getDirection().equals(Query.Order.Direction.ASC) ? + criteriaBuilder.asc(expression) : + criteriaBuilder.desc(expression); + } + jpaOrders.add(jpaOrder); + } + cq.orderBy(jpaOrders.toArray(new jakarta.persistence.criteria.Order[0])); } } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java index 81061df2960..bf180c3c55d 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java @@ -34,7 +34,7 @@ * * @since 8.0 */ -public class JpaQueryContext implements Cloneable { +public class JpaQueryContext implements Cloneable, QueryPathContext { private final AliasRegistry aliasRegistry; private final JoinTracker joinTracker; @@ -110,6 +110,7 @@ public void setParent(JpaQueryContext parent) { this.parent = parent; } + @Override public From getRoot() { return joinTracker.getRoot(); } @@ -173,6 +174,7 @@ public Expression getAliasedExpression(String alias) { return expr; } + @Override public Expression getFullyQualifiedExpression(String path) { if (parent != null) { if ("{alias}".equals(path)) { @@ -185,6 +187,7 @@ public Expression getFullyQualifiedExpression(String path) { return resolver.resolve(path); } + @Override public Path getFullyQualifiedPath(String path) { if (parent != null) { if ("{alias}".equals(path)) { diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java index 163c18a79aa..717e3b35d1a 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java @@ -27,11 +27,11 @@ import java.util.Objects; import java.util.ServiceLoader; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.atomic.AtomicBoolean; import java.util.stream.Collectors; import jakarta.persistence.criteria.AbstractQuery; import jakarta.persistence.criteria.CriteriaBuilder; +import jakarta.persistence.criteria.CriteriaQuery; import jakarta.persistence.criteria.Expression; import jakarta.persistence.criteria.From; import jakarta.persistence.criteria.JoinType; @@ -90,6 +90,122 @@ Predicate handle( HibernateCriteriaBuilder criteriaBuilder, Query.Criterion criterion ); + + /** + * Converts a criterion with the context of the query it belongs to. The context resolves a property path + * the way the built-in criteria do: {@link QueryPathContext#getFullyQualifiedPath(String)} turns + * {@code alias.property}, for an alias created with {@code createAlias}, or {@code association.property} + * into a path on the join GORM created for that association, so a custom criterion and a GORM criterion on + * the same association restrict the same row instead of each joining the association again. This default + * ignores the context and calls {@link #handle(AbstractQuery, From, HibernateCriteriaBuilder, Query.Criterion)}; + * a handler that needs the context implements {@link ContextualCriterionHandler}. + * + * @param criteriaQuery the query or subquery the predicate is for + * @param root the root of that query + * @param criteriaBuilder the criteria builder + * @param criterion the criterion to convert + * @param context the context of the query: its root, its joins and its aliases + * @return the predicate, or {@code null} to add none + * @since 8.1 + */ + default Predicate handle( + AbstractQuery criteriaQuery, + From root, + HibernateCriteriaBuilder criteriaBuilder, + Query.Criterion criterion, + QueryPathContext context + ) { + return handle(criteriaQuery, root, criteriaBuilder, criterion); + } + } + + /** + * A {@link CriterionHandler} that is given the context of the query, through which it resolves property paths + * with {@link QueryPathContext#getFullyQualifiedPath(String)}. A closure with five parameters is coerced to it: + *

+     *     PredicateGenerator.registerCriterionHandler(MyCriterion, { query, root, cb, criterion, context ->
+     *         def c = criterion as MyCriterion
+     *         cb.like(context.getFullyQualifiedPath(c.property) as Expression<String>, c.value as String)
+     *     } as PredicateGenerator.ContextualCriterionHandler)
+     * 
+ * + * @since 8.1 + */ + @FunctionalInterface + public interface ContextualCriterionHandler extends CriterionHandler { + + @Override + Predicate handle( + AbstractQuery criteriaQuery, + From root, + HibernateCriteriaBuilder criteriaBuilder, + Query.Criterion criterion, + QueryPathContext context + ); + + /** + * Called by code that has no context to give; the handler then receives {@code null} for it. + */ + @Override + default Predicate handle( + AbstractQuery criteriaQuery, + From root, + HibernateCriteriaBuilder criteriaBuilder, + Query.Criterion criterion + ) { + return handle(criteriaQuery, root, criteriaBuilder, criterion, null); + } + } + + /** + * Extension point for ordering a criteria query by something other than a property. Register a handler for a + * {@link Query.Order} subclass to have it converted to a JPA {@link jakarta.persistence.criteria.Order}, in + * the place the order has among the query's orders. The order reaches the query through + * {@code order(Query.Order)} of the criteria builder or of a detached criteria. + * + *
+     *     class ByRandom extends Query.Order {
+     *         ByRandom() { super('random') }
+     *     }
+     *     PredicateGenerator.registerOrderHandler(ByRandom) { query, root, cb, order, context ->
+     *         cb.asc(cb.function('random', Double))
+     *     }
+     * 
+ * + * @since 8.1 + */ + @FunctionalInterface + public interface OrderHandler { + + /** + * @param criteriaQuery the query, whose result type tells a query for the entity from a projection + * @param root the root of the query + * @param criteriaBuilder the criteria builder + * @param order the order to convert + * @param context the context of the query: its root, its joins and its aliases + * @return the JPA order, or {@code null} to leave this order out of the query + */ + jakarta.persistence.criteria.Order handle( + CriteriaQuery criteriaQuery, + From root, + HibernateCriteriaBuilder criteriaBuilder, + Query.Order order, + QueryPathContext context + ); + } + + /** + * SPI for contributing an {@link OrderHandler} via {@link ServiceLoader}. Register an implementation in + * {@code META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider}; it is + * discovered on first query execution, like a {@link CriterionHandlerProvider}. + * + * @since 8.1 + */ + public interface OrderHandlerProvider { + + Class orderType(); + + OrderHandler orderHandler(); } /** @@ -119,13 +235,68 @@ public interface CriterionHandlerProvider { public static final Map, CriterionHandler> CUSTOM_HANDLERS = new ConcurrentHashMap<>(); - private static final AtomicBoolean SERVICE_LOADERS_INITIALIZED = new AtomicBoolean(false); + public static final Map, OrderHandler> CUSTOM_ORDER_HANDLERS = + new ConcurrentHashMap<>(); + + private static final Object SERVICE_LOADER_LOCK = new Object(); + private static volatile boolean serviceLoadersInitialized = false; + + /** + * Loads the handler providers once. A caller that arrives while another thread is loading them waits until the + * loading is complete, so no query decides on a handler before the discovered ones are registered. A discovered + * handler never replaces one registered in code: the maps keep the first registration. + */ private static void loadServiceProviders() { - if (SERVICE_LOADERS_INITIALIZED.compareAndSet(false, true)) { - ServiceLoader.load(CriterionHandlerProvider.class, - Thread.currentThread().getContextClassLoader()) + if (serviceLoadersInitialized) { + return; + } + synchronized (SERVICE_LOADER_LOCK) { + if (serviceLoadersInitialized) { + return; + } + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + ServiceLoader.load(CriterionHandlerProvider.class, classLoader) .forEach(p -> CUSTOM_HANDLERS.putIfAbsent(p.criterionType(), p.criterionHandler())); + ServiceLoader.load(OrderHandlerProvider.class, classLoader) + .forEach(p -> CUSTOM_ORDER_HANDLERS.putIfAbsent(p.orderType(), p.orderHandler())); + serviceLoadersInitialized = true; + } + } + + /** + * Registers an {@link OrderHandler} for the given order type. The handler converts every order of exactly + * that type; programmatic registration takes precedence over a {@link ServiceLoader}-discovered handler. + * + * @param type the exact order class to handle + * @param handler the handler that converts the order to a JPA order + * @since 8.1 + */ + public static void registerOrderHandler(Class type, OrderHandler handler) { + CUSTOM_ORDER_HANDLERS.put(type, handler); + } + + /** + * Returns the handler registered for an order type, loading the {@link ServiceLoader} providers first. + * + * @param type the exact order class + * @return the handler, or {@code null} when the type is ordered by its property as usual + * @since 8.1 + */ + public static OrderHandler orderHandler(Class type) { + loadServiceProviders(); + return CUSTOM_ORDER_HANDLERS.get(type); + } + + /** + * Removes all registered custom order handlers and resets the ServiceLoader flag. Useful for test cleanup. + * + * @since 8.1 + */ + public static void clearCustomOrderHandlers() { + synchronized (SERVICE_LOADER_LOCK) { + CUSTOM_ORDER_HANDLERS.clear(); + serviceLoadersInitialized = false; } } @@ -146,8 +317,10 @@ public static void registerCriterionHandler(Class typ * Useful for test cleanup. */ public static void clearCustomCriterionHandlers() { - CUSTOM_HANDLERS.clear(); - SERVICE_LOADERS_INITIALIZED.set(false); + synchronized (SERVICE_LOADER_LOCK) { + CUSTOM_HANDLERS.clear(); + serviceLoadersInitialized = false; + } } private final HibernateCriteriaBuilder criteriaBuilder; @@ -190,7 +363,7 @@ public Predicate handleCriterion( if (criterion instanceof Query.Criterion c) { CriterionHandler customHandler = CUSTOM_HANDLERS.get(c.getClass()); if (customHandler != null) { - return customHandler.handle(criteriaQuery, root, criteriaBuilder, c); + return customHandler.handle(criteriaQuery, root, criteriaBuilder, c, fromsByProvider); } } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java new file mode 100644 index 00000000000..5758615bac7 --- /dev/null +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java @@ -0,0 +1,49 @@ +/* + * Copyright 2024-2025 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.grails.orm.hibernate.query; + +import jakarta.persistence.criteria.Expression; +import jakarta.persistence.criteria.From; +import jakarta.persistence.criteria.Path; + +/** + * What a criterion or order handler sees of the query it converts for: the root of the query and the resolution + * of property paths the way the built-in criteria resolve them. {@code alias.property}, for an alias created with + * {@code createAlias}, or {@code association.property} becomes a path on the join GORM created for that + * association, so a custom criterion and a built-in one on the same association restrict the same row rather + * than each joining the association again. + * + * @since 8.1 + */ +public interface QueryPathContext { + + /** + * @return the root of the query or subquery the handler converts for + */ + From getRoot(); + + /** + * @param path a property path: {@code property}, {@code association.property} or {@code alias.property} + * @return the path, or {@code null} when the property path resolves to an expression that is not a path + */ + Path getFullyQualifiedPath(String path); + + /** + * @param path a property path, or the alias of a projection + * @return the expression for it: a path, or the expression an alias or projection stands for + */ + Expression getFullyQualifiedExpression(String path); +} diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/JpaCriteriaQueryCreatorSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/JpaCriteriaQueryCreatorSpec.groovy index 27c30b58a6e..8eef5106197 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/JpaCriteriaQueryCreatorSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/JpaCriteriaQueryCreatorSpec.groovy @@ -29,6 +29,7 @@ import org.grails.datastore.mapping.query.Query import org.hibernate.query.criteria.JpaCriteriaQuery import org.grails.orm.hibernate.query.JpaCriteriaQueryCreator import org.grails.orm.hibernate.query.JpaQueryContext +import org.grails.orm.hibernate.query.PredicateGenerator import org.grails.orm.hibernate.query.SqlGroupProjection import org.grails.orm.hibernate.query.SqlProjection import org.springframework.core.convert.support.DefaultConversionService @@ -130,6 +131,55 @@ class JpaCriteriaQueryCreatorSpec extends HibernateGormDatastoreSpec { query != null } + def "test createQuery orders by what the handler of a Query.Order subclass returns, in its place"() { + given: + var entity = getPersistentEntity(JpaCriteriaQueryCreatorSpecPerson) + var detachedCriteria = new DetachedCriteria(JpaCriteriaQueryCreatorSpecPerson) + detachedCriteria.order(new JpaCriteriaQueryCreatorSpecLengthOrder()) + detachedCriteria.order(Query.Order.asc("firstName")) + JpaQueryContext seen = null + PredicateGenerator.registerOrderHandler(JpaCriteriaQueryCreatorSpecLengthOrder) { cq, root, cb, order, context -> + seen = context + cb.desc(cb.length(context.getFullyQualifiedPath('firstName') as jakarta.persistence.criteria.Expression)) + } + var creator = new JpaCriteriaQueryCreator(new Query.ProjectionList(), criteriaBuilder, entity, detachedCriteria, new DefaultConversionService()) + + when: + JpaCriteriaQuery query = creator.createQuery() + + then: "the handler's order comes first and the property order after it" + query.orderList.size() == 2 + !query.orderList[0].ascending + query.orderList[1].ascending + + and: "the handler was given the context of the query" + seen != null + seen.root.is(query.roots.first()) + + cleanup: + PredicateGenerator.clearCustomOrderHandlers() + } + + def "test createQuery leaves out an order whose handler returns null"() { + given: + var entity = getPersistentEntity(JpaCriteriaQueryCreatorSpecPerson) + var detachedCriteria = new DetachedCriteria(JpaCriteriaQueryCreatorSpecPerson) + detachedCriteria.order(new JpaCriteriaQueryCreatorSpecLengthOrder()) + detachedCriteria.order(Query.Order.asc("firstName")) + PredicateGenerator.registerOrderHandler(JpaCriteriaQueryCreatorSpecLengthOrder) { cq, root, cb, order, context -> null } + var creator = new JpaCriteriaQueryCreator(new Query.ProjectionList(), criteriaBuilder, entity, detachedCriteria, new DefaultConversionService()) + + when: + JpaCriteriaQuery query = creator.createQuery() + + then: + query.orderList.size() == 1 + query.orderList[0].ascending + + cleanup: + PredicateGenerator.clearCustomOrderHandlers() + } + def "test createQuery with group by"() { given: var entity = getPersistentEntity(JpaCriteriaQueryCreatorSpecPerson) @@ -461,6 +511,14 @@ class JpaCriteriaQueryCreatorSpecPet implements GormEntity { Long id diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy new file mode 100644 index 00000000000..8ee49ae3176 --- /dev/null +++ b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy @@ -0,0 +1,115 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package grails.gorm.tests.hibernatequery + +import java.util.concurrent.Callable +import java.util.concurrent.CountDownLatch +import java.util.concurrent.ExecutorService +import java.util.concurrent.Executors +import java.util.concurrent.Future +import java.util.concurrent.TimeUnit +import java.util.concurrent.TimeoutException + +import spock.lang.Specification + +import org.grails.datastore.mapping.query.Query +import org.grails.orm.hibernate.query.PredicateGenerator + +/** + * The handler providers are discovered on first use. A query that arrives while another thread discovers them must + * wait for the discovery instead of deciding, with the handlers still unregistered, that an order has no handler. + * {@link OrderHandlerDiscoveryProvider} is on the test classpath through {@code META-INF/services} and blocks inside + * its discovery while the spec holds its gate. + */ +class OrderHandlerDiscoverySpec extends Specification { + + void cleanup() { + OrderHandlerDiscoveryProvider.gate.countDown() + OrderHandlerDiscoveryProvider.gate = new CountDownLatch(0) + PredicateGenerator.clearCustomOrderHandlers() + } + + void "a query that arrives while the providers load waits for them and gets the discovered order handler"() { + given: "discovery has not run, and the provider blocks until the spec releases it" + PredicateGenerator.clearCustomOrderHandlers() + OrderHandlerDiscoveryProvider.gate = new CountDownLatch(1) + OrderHandlerDiscoveryProvider.entered = new CountDownLatch(1) + ExecutorService executor = Executors.newFixedThreadPool(2) + Callable lookup = { PredicateGenerator.orderHandler(OrderHandlerDiscoveryOrder) } + + when: "one thread starts the discovery and blocks inside the provider, and a second asks for the handler meanwhile" + Future first = executor.submit(lookup) + boolean providerEntered = OrderHandlerDiscoveryProvider.entered.await(10, TimeUnit.SECONDS) + Future second = executor.submit(lookup) + boolean secondAnsweredDuringDiscovery = true + try { + second.get(500, TimeUnit.MILLISECONDS) + } catch (TimeoutException ignored) { + secondAnsweredDuringDiscovery = false + } + OrderHandlerDiscoveryProvider.gate.countDown() + + then: "the second caller waited for the discovery, and both callers get the discovered handler" + providerEntered + !secondAnsweredDuringDiscovery + first.get(10, TimeUnit.SECONDS) != null + second.get(10, TimeUnit.SECONDS) != null + + cleanup: + executor.shutdownNow() + } + + void "a handler registered in code is kept over the discovered one"() { + given: + PredicateGenerator.clearCustomOrderHandlers() + PredicateGenerator.OrderHandler registered = { query, root, cb, order, context -> null } + PredicateGenerator.registerOrderHandler(OrderHandlerDiscoveryOrder, registered) + + expect: + PredicateGenerator.orderHandler(OrderHandlerDiscoveryOrder).is(registered) + } + +} + +class OrderHandlerDiscoveryOrder extends Query.Order { + + OrderHandlerDiscoveryOrder() { + super('discovered') + } + +} + +class OrderHandlerDiscoveryProvider implements PredicateGenerator.OrderHandlerProvider { + + static volatile CountDownLatch gate = new CountDownLatch(0) + static volatile CountDownLatch entered = new CountDownLatch(0) + + @Override + Class orderType() { + OrderHandlerDiscoveryOrder + } + + @Override + PredicateGenerator.OrderHandler orderHandler() { + entered.countDown() + gate.await() + return { query, root, cb, order, context -> null } as PredicateGenerator.OrderHandler + } + +} diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/PredicateGeneratorSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/PredicateGeneratorSpec.groovy index f09b8d817a0..b03c93f3167 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/PredicateGeneratorSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/PredicateGeneratorSpec.groovy @@ -59,6 +59,44 @@ class PredicateGeneratorSpec extends HibernateGormDatastoreSpec { predicateGenerator = new PredicateGenerator(cb, session.datastore.mappingContext.conversionService) } + def "test getPredicates gives a contextual criterion handler the query context"() { + given: + JpaQueryContext seen = null + PredicateGenerator.registerCriterionHandler(PredicateGeneratorSpecMarker, { q, r, cb, criterion, context -> + seen = context + cb.conjunction() + } as PredicateGenerator.ContextualCriterionHandler) + + when: + def predicates = predicateGenerator.getPredicates(query, root, [new PredicateGeneratorSpecMarker()], fromProvider, personEntity) + + then: + predicates.length == 1 + seen.is(fromProvider) + + cleanup: + PredicateGenerator.clearCustomCriterionHandlers() + } + + def "test getPredicates still calls a criterion handler that takes no context"() { + given: + boolean called = false + PredicateGenerator.registerCriterionHandler(PredicateGeneratorSpecMarker) { q, r, cb, criterion -> + called = true + cb.conjunction() + } + + when: + def predicates = predicateGenerator.getPredicates(query, root, [new PredicateGeneratorSpecMarker()], fromProvider, personEntity) + + then: + predicates.length == 1 + called + + cleanup: + PredicateGenerator.clearCustomCriterionHandlers() + } + def "test getPredicates with Equals criterion"() { given: List criteria = [new Query.Equals("firstName", "Bob")] @@ -573,6 +611,10 @@ class PredicateGeneratorSpec extends HibernateGormDatastoreSpec { } } +/** A criterion with no built-in handling, so only a registered handler converts it. */ +class PredicateGeneratorSpecMarker implements Query.Criterion { +} + @Entity class PredicateGeneratorSpecPerson implements GormEntity { Long id diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy index 01b510191ce..ed9f672380f 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy @@ -21,6 +21,7 @@ package grails.orm import grails.gorm.DetachedCriteria import grails.gorm.annotation.Entity import grails.gorm.tests.HibernateGormDatastoreSpec +import jakarta.persistence.criteria.Expression import org.grails.datastore.mapping.query.Query import org.grails.datastore.mapping.query.api.BuildableCriteria import org.grails.orm.hibernate.query.PredicateGenerator @@ -411,6 +412,107 @@ class HibernateCriteriaBuilderDirectSpec extends HibernateGormDatastoreSpec { and: "the order still applies to the list" new DetachedCriteria(CriteriaTestEntity).build { order('amount', 'desc') }.list()*.name == ['D', 'C', 'B', 'A'] } + + void "a contextual criterion handler resolves an association path to the join GORM created for it"() { + given: "a parent with two children, and a handler that resolves its criterion's path through the context" + var parent = CriteriaTestEntity.findByName('A') + parent.addToChildren(new CriteriaTestChild(name: 'Kid')) + parent.addToChildren(new CriteriaTestChild(name: 'Other')).save(flush: true) + PredicateGenerator.registerCriterionHandler(CriteriaTestPathLike, { query, root, cb, criterion, context -> + def c = criterion as CriteriaTestPathLike + cb.like(context.getFullyQualifiedPath(c.property) as Expression, c.value as String) + } as PredicateGenerator.ContextualCriterionHandler) + + when: "a GORM criterion and the custom criterion restrict the same association" + List both = new DetachedCriteria(CriteriaTestEntity).build { + children { eq('name', 'Kid') } + add(new CriteriaTestPathLike('children.name', 'K%')) + }.list() + List different = new DetachedCriteria(CriteriaTestEntity).build { + children { eq('name', 'Kid') } + add(new CriteriaTestPathLike('children.name', 'O%')) + }.list() + + then: "a child matching both criteria is found" + both*.name == ['A'] + + and: "two children matching one criterion each are not: the handler used the join GORM created, not a second one" + different.empty + + when: "the custom criterion comes before the association block" + List bothReversed = new DetachedCriteria(CriteriaTestEntity).build { + add(new CriteriaTestPathLike('children.name', 'K%')) + children { eq('name', 'Kid') } + }.list() + List differentReversed = new DetachedCriteria(CriteriaTestEntity).build { + add(new CriteriaTestPathLike('children.name', 'O%')) + children { eq('name', 'Kid') } + }.list() + + then: "the handler creates the join from the discovered alias and the association block reuses it" + bothReversed*.name == ['A'] + differentReversed.empty + + cleanup: + PredicateGenerator.clearCustomCriterionHandlers() + } + + void "an order handler converts a Query.Order subclass in its place among the orders"() { + given: "a handler that orders by the remainder of the amount modulo 15, descending" + PredicateGenerator.registerOrderHandler(CriteriaTestRemainderOrder) { query, root, cb, order, context -> + cb.desc(cb.mod(root.get('amount') as Expression, 15)) + } + + when: "the custom order comes before an order by name" + List names = c.list { + order(new CriteriaTestRemainderOrder()) + order('name', 'asc') + }*.name + + then: "amounts 10, 20, 30 and 40 give remainders 10, 5, 0 and 10, and the name breaks the tie" + names == ['A', 'D', 'B', 'C'] + + when: "the same orders go through a detached criteria" + List detached = new DetachedCriteria(CriteriaTestEntity).build { + order(new CriteriaTestRemainderOrder()) + order('name', 'asc') + }.list()*.name + + then: "the handler converts the order there too" + detached == ['A', 'D', 'B', 'C'] + + and: "a count with the handled order counts every row, because a count query leaves its orders out" + new DetachedCriteria(CriteriaTestEntity).build { order(new CriteriaTestRemainderOrder()) }.count() == 4L + + when: "the handler returns null" + PredicateGenerator.registerOrderHandler(CriteriaTestRemainderOrder) { query, root, cb, order, context -> null } + names = c.list { + order(new CriteriaTestRemainderOrder()) + order('name', 'desc') + }*.name + + then: "its order is left out and the others still apply" + names == ['D', 'C', 'B', 'A'] + + cleanup: + PredicateGenerator.clearCustomOrderHandlers() + } +} + +/** A criterion whose handler resolves the property through the query context. */ +class CriteriaTestPathLike extends Query.PropertyCriterion { + + CriteriaTestPathLike(String property, String value) { + super(property, value) + } +} + +/** An order that a registered handler converts. */ +class CriteriaTestRemainderOrder extends Query.Order { + + CriteriaTestRemainderOrder() { + super('amount') + } } @Entity diff --git a/grails-data-hibernate7/core/src/test/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider b/grails-data-hibernate7/core/src/test/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider new file mode 100644 index 00000000000..59a91c3266d --- /dev/null +++ b/grails-data-hibernate7/core/src/test/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider @@ -0,0 +1,19 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# +grails.gorm.tests.hibernatequery.OrderHandlerDiscoveryProvider diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc index 3dbdb5ab36d..6c6d315da19 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc @@ -387,6 +387,94 @@ def results = Airport.withCriteria { the `flights` collection would be loaded eagerly via a join even though the fetch mode has not been explicitly set. +==== Extending the Criteria Builder + +A plugin or an application can add its own criteria and orders to the Hibernate 7 criteria builder. A custom +criterion is a `org.grails.datastore.mapping.query.Query.Criterion`, usually a `Query.PropertyCriterion` subclass, +that a registered _criterion handler_ turns into a JPA `Predicate` when the query runs. A custom order is a +`Query.Order` subclass that a registered _order handler_ turns into a JPA `Order`, which can order by any +expression, not only by a property. Both are usually added to the builder through a Groovy extension module, so +that they read like the built-in ones: + +[source,groovy] +---- +class NumberLikeCriterion extends Query.PropertyCriterion { + NumberLikeCriterion(String property, String pattern) { super(property, pattern) } +} + +class ByRandom extends Query.Order { + ByRandom() { super('random') } +} + +@CompileStatic +class MyCriteriaExtensions { + static AbstractDetachedCriteria numberLike(AbstractDetachedCriteria self, String property, String pattern) { + self.add(new NumberLikeCriterion(property, pattern)) + self + } + + static AbstractDetachedCriteria orderByRandom(AbstractDetachedCriteria self) { + self.order(new ByRandom()) + self + } +} +---- + +The handlers convert them. A criterion handler is given the query, its root, the Hibernate `CriteriaBuilder`, the +criterion and the query's `QueryPathContext`. Through the context, `getFullyQualifiedPath` resolves a property path +the way the built-in criteria do: `alias.property`, for an alias created with `createAlias`, or `association.property` +becomes a path on the join GORM created for that association, so a custom criterion and a built-in one on the +same association restrict the same row rather than each joining the association again. An order handler is given +the query, its root, the criteria builder, the order and the context, and returns the JPA order, or `null` to leave +the order out. A query whose result is a count leaves every order out by itself, so no handler runs for it: + +[source,groovy] +---- +PredicateGenerator.registerCriterionHandler(NumberLikeCriterion, { query, root, cb, criterion, context -> + def c = criterion as NumberLikeCriterion + cb.like(cb.cast(context.getFullyQualifiedPath(c.property) as JpaExpression, String), c.value as String) +} as PredicateGenerator.ContextualCriterionHandler) + +PredicateGenerator.registerOrderHandler(ByRandom) { query, root, cb, order, context -> + cb.asc(cb.function('random', Double)) +} +---- + +A criterion handler that does not need the context takes four parameters and is coerced to +`PredicateGenerator.CriterionHandler`. Handlers are registered for the exact class of the criterion or order, and +a registered handler wins over the built-in handling, so a handler for `Query.Like` replaces how `like` is built. + +A plugin registers its handlers without any code in the application by implementing +`PredicateGenerator.CriterionHandlerProvider` or `PredicateGenerator.OrderHandlerProvider` and naming the class in +`META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$CriterionHandlerProvider` or +`META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider`. The providers are +loaded when the first criteria query runs; a handler registered in code takes precedence over a provider's. + +[source,groovy] +---- +class ByRandomOrderHandlerProvider implements PredicateGenerator.OrderHandlerProvider { + + Class orderType() { ByRandom } + + PredicateGenerator.OrderHandler orderHandler() { + { query, root, cb, order, context -> + cb.asc(cb.function('random', Double)) + } as PredicateGenerator.OrderHandler + } +} +---- + +With the extension module and the providers on the classpath, a query uses them like any other criterion or order: + +[source,groovy] +---- +new DetachedCriteria(Book).build { + numberLike 'price', '4%' + orderByRandom() +}.list() +---- + + ==== Method Reference diff --git a/grails-doc/src/en/guide/introduction/whatsNew.adoc b/grails-doc/src/en/guide/introduction/whatsNew.adoc index a864d817b9a..ed50151331e 100644 --- a/grails-doc/src/en/guide/introduction/whatsNew.adoc +++ b/grails-doc/src/en/guide/introduction/whatsNew.adoc @@ -59,6 +59,16 @@ Jackson `JsonMapper`, so a Grails application and a Spring Boot application retu render. See the Date and Time Rendering sections of the REST and JSON Views guides, and the upgrade guide for the types that render differently than in Grails 7. +==== Hibernate 7 Criteria Queries Can Be Extended + +A plugin or an application can add its own criteria and orders to the Hibernate 7 criteria builder. A criterion +handler now receives the query context, through which it resolves a property path, whether through an alias created +with `createAlias` or an association, to the join GORM created for it, so a custom criterion and a built-in one on +the same association restrict the same row. A new order handler turns a `Query.Order` subclass into a JPA order by +any expression, such as `random()` or a SQL formula, in its place among the query's orders. Both kinds of handler +are registered in code, or by a plugin through `META-INF/services` providers that are loaded when the first query +runs. See the section on extending the criteria builder in the GORM for Hibernate 7 guide. + ==== Hibernate 7 Application Generation Grails Forge can now generate applications configured for Grails Data with Hibernate 7. diff --git a/grails-test-examples/hibernate7/criteria-extension/grails-app/services/example/ProductSearchService.groovy b/grails-test-examples/hibernate7/criteria-extension/grails-app/services/example/ProductSearchService.groovy index d561a27213f..cc5bd85663c 100644 --- a/grails-test-examples/hibernate7/criteria-extension/grails-app/services/example/ProductSearchService.groovy +++ b/grails-test-examples/hibernate7/criteria-extension/grails-app/services/example/ProductSearchService.groovy @@ -62,4 +62,15 @@ class ProductSearchService { numberLike 'price', pricePattern }.list() as List } + + /** + * Lists every product, the one priced nearest to {@code target} first, through the + * {@code orderByPriceDistance} extension and the order handler behind it. + */ + @ReadOnly + List findNearestPrice(BigDecimal target) { + new DetachedCriteria(Product).build { + orderByPriceDistance target + }.list() as List + } } diff --git a/grails-test-examples/hibernate7/criteria-extension/src/integration-test/groovy/example/CriteriaExtensionSpec.groovy b/grails-test-examples/hibernate7/criteria-extension/src/integration-test/groovy/example/CriteriaExtensionSpec.groovy index 8ff1028e7b0..88e75e5c108 100644 --- a/grails-test-examples/hibernate7/criteria-extension/src/integration-test/groovy/example/CriteriaExtensionSpec.groovy +++ b/grails-test-examples/hibernate7/criteria-extension/src/integration-test/groovy/example/CriteriaExtensionSpec.groovy @@ -244,4 +244,40 @@ class CriteriaExtensionSpec extends Specification { results.size() == 2 results*.name.sort() == ['Doohickey', 'Gadget'] } + + void "orderByPriceDistance lists the products nearest to the target price first"() { + given: + createProducts() + + when: 'prices 9.99, 49.99 and 4.99 are measured from 10' + List results = new DetachedCriteria(Product).build { + orderByPriceDistance 10.00 + }.list() + + then: + results*.name == ['Widget', 'Doohickey', 'Gadget'] + } + + void "orderByPriceDistance keeps its place among the orders of the query"() { + given: + createProducts() + new Product(name: 'Gizmo', price: 10.01).save(flush: true) + + when: 'Widget and Gizmo are both 0.01 from the target, and the name breaks the tie' + List results = new DetachedCriteria(Product).build { + orderByPriceDistance 10.00 + order 'name', 'desc' + }.list() + + then: + results*.name == ['Widget', 'Gizmo', 'Doohickey', 'Gadget'] + } + + void "ProductSearchService.findNearestPrice orders through the order handler"() { + given: + createProducts() + + expect: + productSearchService.findNearestPrice(50.00)*.name == ['Gadget', 'Widget', 'Doohickey'] + } } diff --git a/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/extensions/HibernateCriteriaBuilderExtensions.groovy b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/extensions/HibernateCriteriaBuilderExtensions.groovy index e5564090763..8b8ba9de0bd 100644 --- a/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/extensions/HibernateCriteriaBuilderExtensions.groovy +++ b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/extensions/HibernateCriteriaBuilderExtensions.groovy @@ -20,6 +20,7 @@ package example.extensions import example.query.NumberLikeCriterion +import example.query.PriceDistanceOrder import groovy.transform.CompileStatic import org.grails.datastore.gorm.query.criteria.AbstractDetachedCriteria @@ -59,4 +60,19 @@ class HibernateCriteriaBuilderExtensions { self.add(new NumberLikeCriterion(propertyName, propertyValue as String)) self } + + /** + * Orders the results by how far their price is from {@code target}, nearest first. + * + *

Adds a {@link PriceDistanceOrder}, which the registered + * {@code OrderHandler} converts to an order by {@code abs(price - target)} at query execution time. + * + * @param self the detached criteria + * @param target the price to measure the distance from + * @return the criteria for chaining + */ + static AbstractDetachedCriteria orderByPriceDistance(AbstractDetachedCriteria self, BigDecimal target) { + self.order(new PriceDistanceOrder(target)) + self + } } diff --git a/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/NumberLikeCriterionHandlerProvider.groovy b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/NumberLikeCriterionHandlerProvider.groovy index 94fe21f32b9..924ca6cd7eb 100644 --- a/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/NumberLikeCriterionHandlerProvider.groovy +++ b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/NumberLikeCriterionHandlerProvider.groovy @@ -37,12 +37,16 @@ class NumberLikeCriterionHandlerProvider implements PredicateGenerator.Criterion NumberLikeCriterion } + /** + * A contextual handler: the property is resolved through the query context, so {@code numberLike} also + * works on an association path such as {@code 'supplier.rating'}, on the join GORM created for it. + */ @Override PredicateGenerator.CriterionHandler criterionHandler() { - { query, root, cb, criterion -> + { query, root, cb, criterion, context -> NumberLikeCriterion nlc = criterion as NumberLikeCriterion - def cast = cb.cast(root.get(nlc.property) as JpaExpression, String) + def cast = cb.cast(context.getFullyQualifiedPath(nlc.property) as JpaExpression, String) cb.like(cast, nlc.value as String) - } as PredicateGenerator.CriterionHandler + } as PredicateGenerator.ContextualCriterionHandler } } diff --git a/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrder.groovy b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrder.groovy new file mode 100644 index 00000000000..3409121c134 --- /dev/null +++ b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrder.groovy @@ -0,0 +1,40 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package example.query + +import groovy.transform.CompileStatic +import org.grails.datastore.mapping.query.Query + +/** + * An order by how far a product's price is from a target price, nearest first. It is a + * {@link Query.Order} subclass, so it is given to a criteria query with {@code order(...)}, and the + * {@link org.grails.orm.hibernate.query.PredicateGenerator.OrderHandler} that + * {@link PriceDistanceOrderHandlerProvider} registers turns it into a JPA order by an expression, + * which an order by a property name cannot express. + */ +@CompileStatic +class PriceDistanceOrder extends Query.Order { + + final BigDecimal target + + PriceDistanceOrder(BigDecimal target) { + super('price') + this.target = target + } +} diff --git a/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrderHandlerProvider.groovy b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrderHandlerProvider.groovy new file mode 100644 index 00000000000..f47f6b958cd --- /dev/null +++ b/grails-test-examples/hibernate7/criteria-extension/src/main/groovy/example/query/PriceDistanceOrderHandlerProvider.groovy @@ -0,0 +1,49 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package example.query + +import jakarta.persistence.criteria.Expression +import org.grails.datastore.mapping.query.Query +import org.grails.orm.hibernate.query.PredicateGenerator + +/** + * ServiceLoader provider that registers the handler of {@link PriceDistanceOrder} with + * {@link PredicateGenerator} at first query execution. The handler orders by the absolute difference + * between the price and the target, resolving the {@code price} path through the query context the + * way the built-in orders do. + * + *

Discovered via + * {@code META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider}. + */ +class PriceDistanceOrderHandlerProvider implements PredicateGenerator.OrderHandlerProvider { + + @Override + Class orderType() { + PriceDistanceOrder + } + + @Override + PredicateGenerator.OrderHandler orderHandler() { + { query, root, cb, order, context -> + PriceDistanceOrder byDistance = order as PriceDistanceOrder + Expression price = context.getFullyQualifiedPath(byDistance.property) as Expression + cb.asc(cb.abs(cb.diff(price, byDistance.target))) + } as PredicateGenerator.OrderHandler + } +} diff --git a/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider b/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider new file mode 100644 index 00000000000..c54a3cfae5b --- /dev/null +++ b/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider @@ -0,0 +1 @@ +example.query.PriceDistanceOrderHandlerProvider From de16f1f579f0352da533899dd4321bb4edb5ea32 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Thu, 22 Oct 2026 12:59:42 -0600 Subject: [PATCH 18/23] Write a test variable into each environment map without a wrapper The environment spec installed a variable through a Map wrapper that delegated to the case-insensitive map System.getenv(String) reads on Windows and overrode put and remove to write the map System.getenv() exposes as well. Groovy 6 compiles that class with the @Delegate-generated put in place of the override, so on 9.0.x the variable reached only the first map, the snapshot the config takes from System.getenv() never saw it, and every feature that installs a variable failed on Windows. The maps are now written one by one by installVariable and removeVariable, with no wrapper to compile. --- .../config/SystemEnvironmentConfigSpec.groovy | 107 ++++++++---------- 1 file changed, 50 insertions(+), 57 deletions(-) diff --git a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy index 57da02a8502..ecb3805c97d 100644 --- a/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/SystemEnvironmentConfigSpec.groovy @@ -43,7 +43,7 @@ property-with_mixed.symbols: from-yml @Issue('#10670') void 'configuration properties defined in SystemEnvironment take precedence'() { given: 'a value in the system environment' - modifiableSystemEnvironment.put(systemProperty, value) + installVariable(systemProperty, value) and: 'default configuration in application-yml' def config = configFor(YAML) @@ -52,7 +52,7 @@ property-with_mixed.symbols: from-yml config.getProperty(propertyToFind) == value cleanup: - modifiableSystemEnvironment.remove(systemProperty) + removeVariable(systemProperty) where: propertyToFind | systemProperty @@ -76,7 +76,7 @@ property-with_mixed.symbols: from-yml Locale defaultLocale = Locale.getDefault() and: 'a variable that is the upper-cased key' - modifiableSystemEnvironment.put('PROPERTY_WITH_ID', 'from-env') + installVariable('PROPERTY_WITH_ID', 'from-env') def config = configFor('property.with.id: from-yml') and: 'a default locale whose upper-case i is not I' @@ -87,19 +87,19 @@ property-with_mixed.symbols: from-yml cleanup: Locale.setDefault(defaultLocale) - modifiableSystemEnvironment.remove('PROPERTY_WITH_ID') + removeVariable('PROPERTY_WITH_ID') } void 'a key with a character above ASCII is matched through its upper-cased form'() { given: 'a variable that is the upper-cased key, in which the sharp s has become two letters' - modifiableSystemEnvironment.put('STRASSE_X', 'from-env') + installVariable('STRASSE_X', 'from-env') def config = configFor('stra\u00dfe.x: from-yml') expect: 'the variable overrides the configured value' config.getProperty('stra\u00dfe.x') == 'from-env' cleanup: - modifiableSystemEnvironment.remove('STRASSE_X') + removeVariable('STRASSE_X') } void 'a null key resolves to the default value'() { @@ -127,42 +127,42 @@ property-with_mixed.symbols: from-yml @IgnoreIf(value = { os.windows }, reason = 'Windows matches variable names regardless of case') void 'a variable that matches the key in none of the probed spellings does not override it'() { given: 'a lower-case variable for a mixed-case key' - modifiableSystemEnvironment.put('property_with_period', 'from-env') + installVariable('property_with_period', 'from-env') def config = configFor('Property.With.Period: from-yml') expect: 'only the key as written and its upper-cased form are probed, as Spring does' config.getProperty('Property.With.Period') == 'from-yml' cleanup: - modifiableSystemEnvironment.remove('property_with_period') + removeVariable('property_with_period') } @Requires({ os.windows }) void 'on Windows a variable matches the key in any case, as System.getenv does there'() { given: 'a mixed-case variable for a lower-case key' - modifiableSystemEnvironment.put('Property_With_Period', 'from-env') + installVariable('Property_With_Period', 'from-env') def config = configFor(YAML) expect: 'the variable overrides the configured value' config.getProperty('property.with.period') == 'from-env' cleanup: - modifiableSystemEnvironment.remove('Property_With_Period') + removeVariable('Property_With_Period') } @IgnoreIf(value = { os.windows }, reason = 'the spellings that differ only in case are one variable on Windows') void 'when several variables match a key the first probed spelling wins'() { given: 'two variables that both match the key' - modifiableSystemEnvironment.put(winner, 'from-winner') - modifiableSystemEnvironment.put(loser, 'from-loser') + installVariable(winner, 'from-winner') + installVariable(loser, 'from-loser') def config = configFor(YAML) expect: 'the value of the spelling that is probed first is returned' config.getProperty('property.with.period') == 'from-winner' cleanup: - modifiableSystemEnvironment.remove(winner) - modifiableSystemEnvironment.remove(loser) + removeVariable(winner) + removeVariable(loser) where: winner | loser @@ -173,7 +173,7 @@ property-with_mixed.symbols: from-yml void 'a property read more than once keeps resolving to the system environment value'() { given: 'configuration that is overridden by the environment' - modifiableSystemEnvironment.put('PROPERTY_WITH_PERIOD', 'from-env') + installVariable('PROPERTY_WITH_PERIOD', 'from-env') def config = configFor(YAML) expect: 'every read resolves to the environment value, not just the first' @@ -182,7 +182,7 @@ property-with_mixed.symbols: from-yml config.getProperty('property.with.period') == 'from-env' cleanup: - modifiableSystemEnvironment.remove('PROPERTY_WITH_PERIOD') + removeVariable('PROPERTY_WITH_PERIOD') } void 'a property that has already been read still reflects later configuration changes'() { @@ -207,7 +207,7 @@ property-with_mixed.symbols: from-yml before.getProperty('late.bound.property') == 'from-yml' when: 'the variable is installed and a new config is created' - modifiableSystemEnvironment.put('LATE_BOUND_PROPERTY', 'from-env') + installVariable('LATE_BOUND_PROPERTY', 'from-env') def after = configFor('late.bound.property: from-yml') then: 'the new config resolves to the environment value' @@ -217,7 +217,7 @@ property-with_mixed.symbols: from-yml before.getProperty('late.bound.property') == 'from-yml' cleanup: - modifiableSystemEnvironment.remove('LATE_BOUND_PROPERTY') + removeVariable('LATE_BOUND_PROPERTY') } void 'a variable installed after the config exists is seen once the config is refreshed'() { @@ -225,7 +225,7 @@ property-with_mixed.symbols: from-yml def config = configFor('late.bound.property: from-yml') when: 'the variable is installed' - modifiableSystemEnvironment.put('LATE_BOUND_PROPERTY', 'from-env') + installVariable('LATE_BOUND_PROPERTY', 'from-env') then: 'the config keeps the environment it was created with' config.getProperty('late.bound.property') == 'from-yml' @@ -237,7 +237,7 @@ property-with_mixed.symbols: from-yml config.getProperty('late.bound.property') == 'from-env' cleanup: - modifiableSystemEnvironment.remove('LATE_BOUND_PROPERTY') + removeVariable('LATE_BOUND_PROPERTY') } private static PropertySourcesConfig configFor(String yaml) { @@ -248,9 +248,28 @@ property-with_mixed.symbols: from-yml new PropertySourcesConfig(propertySources) } - // From https://github.com/spring-projects/spring-framework/blob/4.3.x/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java#L492 + private static void installVariable(String name, String value) { + for (def map : systemEnvironmentMaps()) { + map.put(name, value) + } + } + + private static void removeVariable(String name) { + for (def map : systemEnvironmentMaps()) { + map.remove(name) + } + } + + /** + * The maps behind System.getenv() and System.getenv(String), which a variable installed for a test + * has to reach. On Unix both read one map. On Windows System.getenv(String) reads a case-insensitive + * copy while System.getenv() exposes the original, so the variable goes into both. A wrapper that + * overrode put and remove to do that was compiled by Groovy 6 with the @Delegate-generated put + * instead of the override, so the maps are written one by one here. + */ @SuppressWarnings("unchecked") - static Map getModifiableSystemEnvironment() { + private static List> systemEnvironmentMaps() { + // From https://github.com/spring-projects/spring-framework/blob/4.3.x/spring-core/src/test/java/org/springframework/core/env/StandardEnvironmentTests.java#L492 // for os x / linux Class[] classes = Collections.class.getDeclaredClasses(); Map env = System.getenv(); @@ -261,7 +280,7 @@ property-with_mixed.symbols: from-yml field.setAccessible(true); Object obj = field.get(env); if (obj != null && obj.getClass().getName().equals('java.lang.ProcessEnvironment$StringEnvironment')) { - return (Map) obj; + return [(Map) obj] } } catch (Exception ex) { @@ -270,8 +289,7 @@ property-with_mixed.symbols: from-yml } } - // for windows: System.getenv(String) reads a case-insensitive copy of the environment while - // System.getenv() exposes the original, so a variable installed for a test has to go into both + // for windows Class processEnvironmentClass; try { processEnvironmentClass = Class.forName("java.lang.ProcessEnvironment"); @@ -280,15 +298,14 @@ property-with_mixed.symbols: from-yml throw new IllegalStateException(ex); } - Map caseInsensitiveEnvironment = staticMapField(processEnvironmentClass, 'theCaseInsensitiveEnvironment') - Map environment = staticMapField(processEnvironmentClass, 'theEnvironment') - if (caseInsensitiveEnvironment != null && environment != null) { - return new BothEnvironments(caseInsensitiveEnvironment, environment) + List> maps = [ + staticMapField(processEnvironmentClass, 'theCaseInsensitiveEnvironment'), + staticMapField(processEnvironmentClass, 'theEnvironment') + ].findAll() + if (maps) { + return maps } - if (environment != null) { - return environment - } - throw new IllegalStateException() + throw new IllegalStateException('The process environment could not be opened') } @SuppressWarnings("unchecked") @@ -305,28 +322,4 @@ property-with_mixed.symbols: from-yml throw new IllegalStateException(ex) } } - - private static class BothEnvironments implements Map { - - @Delegate - final Map primary - final Map secondary - - BothEnvironments(Map primary, Map secondary) { - this.primary = primary - this.secondary = secondary - } - - @Override - String put(String key, String value) { - secondary.put(key, value) - primary.put(key, value) - } - - @Override - String remove(Object key) { - secondary.remove(key) - primary.remove(key) - } - } } From 1e47f7ed5d5986dbd8694e9530e108a4610878e1 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Thu, 22 Oct 2026 15:11:29 -0500 Subject: [PATCH 19/23] Hibernate 7: let a handler order by the expression a sqlProjection records under its alias A custom order that names a sqlProjection alias could not reach that expression: the translator records it with registerSelectionAlias, and getFullyQualifiedPath and getFullyQualifiedExpression resolve property paths only. QueryPathContext now exposes getSelectionAlias(String), which JpaQueryContext already had, so an order handler orders by a projected value the way the built-in alias order does; predicate property resolution is unchanged. HibernateCriteriaBuilderSqlProjectionSpec orders by a sqlProjection alias through a handler and asserts the rows come in the projection's order and the SQL orders by the select item, not by a column. The guide names the accessor. --- .../orm/hibernate/query/JpaQueryContext.java | 1 + .../orm/hibernate/query/QueryPathContext.java | 7 ++++ ...ateCriteriaBuilderSqlProjectionSpec.groovy | 35 +++++++++++++++++++ .../src/docs/asciidoc/querying/criteria.adoc | 3 +- 4 files changed, 45 insertions(+), 1 deletion(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java index bf180c3c55d..16f01501ef8 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java @@ -149,6 +149,7 @@ public void registerSelectionAlias(String alias, Expression selection) { * @param alias the alias of the selection * @return the selection, or {@code null} if no selection of this query has the alias */ + @Override public Expression getSelectionAlias(String alias) { return selectionAliases.get(alias); } diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java index 5758615bac7..7a7a3df7514 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java @@ -46,4 +46,11 @@ public interface QueryPathContext { * @return the expression for it: a path, or the expression an alias or projection stands for */ Expression getFullyQualifiedExpression(String path); + + /** + * @param alias the column alias of a {@code sqlProjection} of the query + * @return the projected expression recorded under that alias, so that a handler can order by it, or {@code null} + * when no projection of the query carries the alias + */ + Expression getSelectionAlias(String alias); } diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy index fb0d025f13a..aca33a9420c 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy @@ -32,6 +32,8 @@ import spock.lang.Specification import grails.gorm.annotation.Entity import grails.gorm.transactions.Rollback +import org.grails.orm.hibernate.query.PredicateGenerator +import org.grails.datastore.mapping.query.Query import org.grails.datastore.mapping.core.DatastoreUtils import org.grails.orm.hibernate.HibernateDatastore import org.grails.orm.hibernate.cfg.Settings @@ -125,6 +127,31 @@ class HibernateCriteriaBuilderSqlProjectionSpec extends Specification { rows*.toList().sort { it[0] } == [[2, 3L, 24L], [4, 1L, 9L]] } + @Rollback + void 'an order handler orders by the expression a SQL projection records under its alias'() { + given: "a handler that orders by the selection an alias stands for, descending" + saveBoxes() + PredicateGenerator.registerOrderHandler(SqlProjectionAliasOrder) { query, root, cb, order, context -> + cb.desc(context.getSelectionAlias(order.property)) + } + + when: + List rows = SqlProjectionBox.createCriteria().list { + projections { + sqlProjection 'width * 10 as scaled', 'scaled', INTEGER + } + order(new SqlProjectionAliasOrder('scaled')) + } + + then: "the rows come in descending order of the projection, and the SQL orders by the select item, not by a column" + rows == rows.sort(false).reverse() + rows.size() == SqlProjectionBox.count() + sqlCapture.statements.any { it.contains('select width * 10 from') && it.contains('order by 1 desc') } + + cleanup: + PredicateGenerator.clearCustomOrderHandlers() + } + @Rollback void 'the results are ordered by the alias of a SQL projection'() { given: @@ -512,3 +539,11 @@ class SqlProjectionBox { Date packedAt SqlProjectionShelf shelf } + +/** An order that a registered handler converts, naming the column alias of a SQL projection. */ +class SqlProjectionAliasOrder extends Query.Order { + + SqlProjectionAliasOrder(String alias) { + super(alias) + } +} diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc index 6c6d315da19..0b3000fba2e 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc @@ -424,7 +424,8 @@ The handlers convert them. A criterion handler is given the query, its root, the criterion and the query's `QueryPathContext`. Through the context, `getFullyQualifiedPath` resolves a property path the way the built-in criteria do: `alias.property`, for an alias created with `createAlias`, or `association.property` becomes a path on the join GORM created for that association, so a custom criterion and a built-in one on the -same association restrict the same row rather than each joining the association again. An order handler is given +same association restrict the same row rather than each joining the association again, and `getSelectionAlias` +returns the expression a `sqlProjection` records under its column alias, so an order can sort by a projected value. An order handler is given the query, its root, the criteria builder, the order and the context, and returns the JPA order, or `null` to leave the order out. A query whose result is a count leaves every order out by itself, so no handler runs for it: From 54a44543b221b2345497942f9374a3f40c4b7459 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 23 Oct 2026 10:05:50 -0500 Subject: [PATCH 20/23] Hibernate 7: answer the review of the handler context: a closure registers without as, the alias resolves, the headers are in registerCriterionHandler takes a closure and registers it as a ContextualCriterionHandler when it has five parameters and as a CriterionHandler otherwise, so the two shapes of handler work without as, as the order handler does. The map of order handlers is private: registerOrderHandler, orderHandler and clearCustomOrderHandlers cover registration, lookup and cleanup, and a direct read skips the discovery. getFullyQualifiedExpression returns the projection a sqlProjection alias stands for when no property resolves the path, and a property of the same name still wins, so a restriction on it keeps restricting the property. QueryPathContext and the example order provider file carry the ASF header, the registration note says that discovery does not replace a registered handler while registering in code does, and the discovery spec's provider handles an interrupt. New specs: a createAlias alias shares the join GORM created, in a detached criteria and in the builder; a four and a five parameter closure register without as; the context resolves a projection alias while a property of the same name stays a property. --- .../orm/hibernate/query/JpaQueryContext.java | 6 +- .../hibernate/query/PredicateGenerator.java | 37 +++++++++-- .../orm/hibernate/query/QueryPathContext.java | 30 +++++---- .../OrderHandlerDiscoverySpec.groovy | 7 ++- .../HibernateCriteriaBuilderDirectSpec.groovy | 63 +++++++++++++++++++ ...ateCriteriaBuilderSqlProjectionSpec.groovy | 41 ++++++++++++ .../src/docs/asciidoc/querying/criteria.adoc | 8 +-- ...ry.PredicateGenerator$OrderHandlerProvider | 18 ++++++ 8 files changed, 185 insertions(+), 25 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java index 16f01501ef8..3ce4a6cf585 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java @@ -144,7 +144,8 @@ public void registerSelectionAlias(String alias, Expression selection) { } /** - * Returns the selection recorded under an alias by {@link #registerSelectionAlias(String, Expression)}. + * Returns the selection recorded under an alias by {@link #registerSelectionAlias(String, Expression)}. Unlike + * {@link #getFullyQualifiedExpression(String)}, which resolves a property first, this asks for the alias alone. * * @param alias the alias of the selection * @return the selection, or {@code null} if no selection of this query has the alias @@ -185,7 +186,8 @@ public Expression getFullyQualifiedExpression(String path) { return parent.getFullyQualifiedExpression("root." + path.substring(8)); } } - return resolver.resolve(path); + Expression resolved = resolver.resolve(path); + return resolved != null ? resolved : selectionAliases.get(path); } @Override diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java index 717e3b35d1a..b554412dfe4 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/PredicateGenerator.java @@ -29,6 +29,8 @@ import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; +import groovy.lang.Closure; + import jakarta.persistence.criteria.AbstractQuery; import jakarta.persistence.criteria.CriteriaBuilder; import jakarta.persistence.criteria.CriteriaQuery; @@ -121,12 +123,13 @@ default Predicate handle( /** * A {@link CriterionHandler} that is given the context of the query, through which it resolves property paths - * with {@link QueryPathContext#getFullyQualifiedPath(String)}. A closure with five parameters is coerced to it: + * with {@link QueryPathContext#getFullyQualifiedPath(String)}. A closure with five parameters registered with + * {@link #registerCriterionHandler(Class, Closure)} becomes one, with no {@code as}: *

-     *     PredicateGenerator.registerCriterionHandler(MyCriterion, { query, root, cb, criterion, context ->
+     *     PredicateGenerator.registerCriterionHandler(MyCriterion) { query, root, cb, criterion, context ->
      *         def c = criterion as MyCriterion
      *         cb.like(context.getFullyQualifiedPath(c.property) as Expression<String>, c.value as String)
-     *     } as PredicateGenerator.ContextualCriterionHandler)
+     *     }
      * 
* * @since 8.1 @@ -235,7 +238,7 @@ public interface CriterionHandlerProvider { public static final Map, CriterionHandler> CUSTOM_HANDLERS = new ConcurrentHashMap<>(); - public static final Map, OrderHandler> CUSTOM_ORDER_HANDLERS = + private static final Map, OrderHandler> CUSTOM_ORDER_HANDLERS = new ConcurrentHashMap<>(); private static final Object SERVICE_LOADER_LOCK = new Object(); @@ -244,8 +247,9 @@ public interface CriterionHandlerProvider { /** * Loads the handler providers once. A caller that arrives while another thread is loading them waits until the - * loading is complete, so no query decides on a handler before the discovered ones are registered. A discovered - * handler never replaces one registered in code: the maps keep the first registration. + * loading is complete, so no query decides on a handler before the discovered ones are registered. Discovery uses + * {@code putIfAbsent}, so a discovered handler does not replace one that is already registered; registering in + * code uses {@code put}, so a handler registered in code replaces a discovered one. */ private static void loadServiceProviders() { if (serviceLoadersInitialized) { @@ -312,6 +316,27 @@ public static void registerCriterionHandler(Class typ CUSTOM_HANDLERS.put(type, handler); } + /** + * Registers a closure as the {@link CriterionHandler} for the given criterion type. A closure with five + * parameters, {@code query, root, criteriaBuilder, criterion, context}, becomes a + * {@link ContextualCriterionHandler}; any other closure becomes a {@link CriterionHandler} and is called with + * four arguments. This is what makes the two shapes work without {@code as}, as {@link #registerOrderHandler} does + * for an order handler. + * + * @param type the exact criterion class to handle + * @param handler the closure that converts the criterion to a JPA {@link Predicate} + * @since 8.1 + */ + public static void registerCriterionHandler(Class type, Closure handler) { + if (handler.getMaximumNumberOfParameters() == 5) { + registerCriterionHandler(type, (ContextualCriterionHandler) (query, root, criteriaBuilder, criterion, context) -> + (Predicate) handler.call(query, root, criteriaBuilder, criterion, context)); + } else { + registerCriterionHandler(type, (CriterionHandler) (query, root, criteriaBuilder, criterion) -> + (Predicate) handler.call(query, root, criteriaBuilder, criterion)); + } + } + /** * Removes all registered custom criterion handlers and resets the ServiceLoader flag. * Useful for test cleanup. diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java index 7a7a3df7514..127a246e1d5 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java @@ -1,17 +1,20 @@ /* - * Copyright 2024-2025 the original author or authors. + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at + * https://www.apache.org/licenses/LICENSE-2.0 * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. */ package org.grails.orm.hibernate.query; @@ -43,7 +46,10 @@ public interface QueryPathContext { /** * @param path a property path, or the alias of a projection - * @return the expression for it: a path, or the expression an alias or projection stands for + * @return the expression for it: a path, or, when no property resolves, the projected expression the alias of a + * {@code sqlProjection} stands for. A property takes precedence over an alias of the same name, since a + * restriction on that property must still restrict the property; {@link #getSelectionAlias(String)} asks for + * the alias alone */ Expression getFullyQualifiedExpression(String path); diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy index 8ee49ae3176..a1c31c55e7d 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/gorm/tests/hibernatequery/OrderHandlerDiscoverySpec.groovy @@ -108,7 +108,12 @@ class OrderHandlerDiscoveryProvider implements PredicateGenerator.OrderHandlerPr @Override PredicateGenerator.OrderHandler orderHandler() { entered.countDown() - gate.await() + try { + gate.await() + } catch (InterruptedException e) { + Thread.currentThread().interrupt() + throw new IllegalStateException('Interrupted while the test held the provider', e) + } return { query, root, cb, order, context -> null } as PredicateGenerator.OrderHandler } diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy index ed9f672380f..8afbaa2d990 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderDirectSpec.groovy @@ -457,6 +457,61 @@ class HibernateCriteriaBuilderDirectSpec extends HibernateGormDatastoreSpec { PredicateGenerator.clearCustomCriterionHandlers() } + void "a contextual criterion handler resolves the alias of createAlias to the join GORM created for it"() { + given: "a parent with two children, and the same handler as above" + var parent = CriteriaTestEntity.findByName('A') + parent.addToChildren(new CriteriaTestChild(name: 'Kid')) + parent.addToChildren(new CriteriaTestChild(name: 'Other')).save(flush: true) + PredicateGenerator.registerCriterionHandler(CriteriaTestPathLike) { query, root, cb, criterion, context -> + def c = criterion as CriteriaTestPathLike + cb.like(context.getFullyQualifiedPath(c.property) as Expression, c.value as String) + } + + when: "a createAlias alias is restricted by a GORM criterion and by the custom criterion" + List both = new DetachedCriteria(CriteriaTestEntity).build { + createAlias('children', 'ch') + eq('ch.name', 'Kid') + add(new CriteriaTestPathLike('ch.name', 'K%')) + }.list() + List different = new DetachedCriteria(CriteriaTestEntity).build { + createAlias('children', 'ch') + eq('ch.name', 'Kid') + add(new CriteriaTestPathLike('ch.name', 'O%')) + }.list() + List inBuilder = c.list { + createAlias('children', 'ch') + eq('ch.name', 'Kid') + hibernateQuery.add(new CriteriaTestPathLike('ch.name', 'O%')) + } + + then: "the alias shares the join, in a detached criteria and in the builder" + both*.name == ['A'] + different.empty + inBuilder.empty + + cleanup: + PredicateGenerator.clearCustomCriterionHandlers() + } + + void "a closure registered as a criterion handler is a contextual handler with five parameters and a plain one otherwise"() { + given: "one criterion handled by a four parameter closure and one by a five parameter closure, with no as" + PredicateGenerator.registerCriterionHandler(CriteriaTestPathLike) { query, root, cb, criterion -> + def c = criterion as CriteriaTestPathLike + cb.like(root.get(c.property) as Expression, c.value as String) + } + PredicateGenerator.registerCriterionHandler(CriteriaTestContextLike) { query, root, cb, criterion, context -> + def c = criterion as CriteriaTestContextLike + cb.like(context.getFullyQualifiedPath(c.property) as Expression, c.value as String) + } + + expect: + new DetachedCriteria(CriteriaTestEntity).build { add(new CriteriaTestPathLike('name', 'A%')) }.list()*.name == ['A'] + new DetachedCriteria(CriteriaTestEntity).build { add(new CriteriaTestContextLike('name', 'B%')) }.list()*.name == ['B'] + + cleanup: + PredicateGenerator.clearCustomCriterionHandlers() + } + void "an order handler converts a Query.Order subclass in its place among the orders"() { given: "a handler that orders by the remainder of the amount modulo 15, descending" PredicateGenerator.registerOrderHandler(CriteriaTestRemainderOrder) { query, root, cb, order, context -> @@ -507,6 +562,14 @@ class CriteriaTestPathLike extends Query.PropertyCriterion { } } +/** A criterion that a five parameter closure converts. */ +class CriteriaTestContextLike extends Query.PropertyCriterion { + + CriteriaTestContextLike(String property, String value) { + super(property, value) + } +} + /** An order that a registered handler converts. */ class CriteriaTestRemainderOrder extends Query.Order { diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy index aca33a9420c..cbc21995166 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy @@ -152,6 +152,47 @@ class HibernateCriteriaBuilderSqlProjectionSpec extends Specification { PredicateGenerator.clearCustomOrderHandlers() } + @Rollback + void 'getFullyQualifiedExpression returns the projection a sqlProjection alias stands for, while a property of that name stays a property'() { + given: "a handler that orders by whatever getFullyQualifiedExpression returns for the alias, and one that records it" + saveBoxes() + List resolved = [] + PredicateGenerator.registerOrderHandler(SqlProjectionAliasOrder) { query, root, cb, order, context -> + def expression = context.getFullyQualifiedExpression(order.property) + resolved << expression?.getClass() + cb.desc(expression) + } + + when: "the alias names no property" + List rows = SqlProjectionBox.createCriteria().list { + projections { + sqlProjection 'width * 10 as scaled', 'scaled', INTEGER + } + order(new SqlProjectionAliasOrder('scaled')) + } + + then: "the order uses the projected expression, as it does through getSelectionAlias" + rows == rows.sort(false).reverse() + resolved.every { it != null } + sqlCapture.statements.any { it.contains('select width * 10 from') && it.contains('order by 1 desc') } + + when: "the alias is the name of a property: the property wins, since a restriction on it must still restrict the property" + resolved.clear() + SqlProjectionBox.createCriteria().list { + projections { + sqlProjection 'width * 10 as width', 'width', INTEGER + } + order(new SqlProjectionAliasOrder('width')) + } + + then: + resolved.size() == 1 + sqlCapture.statements.last().contains('order by') + + cleanup: + PredicateGenerator.clearCustomOrderHandlers() + } + @Rollback void 'the results are ordered by the alias of a SQL projection'() { given: diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc index 0b3000fba2e..4f000cbb1aa 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc @@ -431,18 +431,18 @@ the order out. A query whose result is a count leaves every order out by itself, [source,groovy] ---- -PredicateGenerator.registerCriterionHandler(NumberLikeCriterion, { query, root, cb, criterion, context -> +PredicateGenerator.registerCriterionHandler(NumberLikeCriterion) { query, root, cb, criterion, context -> def c = criterion as NumberLikeCriterion cb.like(cb.cast(context.getFullyQualifiedPath(c.property) as JpaExpression, String), c.value as String) -} as PredicateGenerator.ContextualCriterionHandler) +} PredicateGenerator.registerOrderHandler(ByRandom) { query, root, cb, order, context -> cb.asc(cb.function('random', Double)) } ---- -A criterion handler that does not need the context takes four parameters and is coerced to -`PredicateGenerator.CriterionHandler`. Handlers are registered for the exact class of the criterion or order, and +A closure with five parameters is registered as a handler that receives the context; a closure with four parameters +is registered as a handler that does not, so both shapes work without `as`. Handlers are registered for the exact class of the criterion or order, and a registered handler wins over the built-in handling, so a handler for `Query.Like` replaces how `like` is built. A plugin registers its handlers without any code in the application by implementing diff --git a/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider b/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider index c54a3cfae5b..bf9be375b77 100644 --- a/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider +++ b/grails-test-examples/hibernate7/criteria-extension/src/main/resources/META-INF/services/org.grails.orm.hibernate.query.PredicateGenerator$OrderHandlerProvider @@ -1 +1,19 @@ +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# https://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +# example.query.PriceDistanceOrderHandlerProvider From 3bdd02449ab886760b6dbbf06c7860cae407ac71 Mon Sep 17 00:00:00 2001 From: Walter Duque de Estrada Date: Fri, 23 Oct 2026 14:40:22 -0500 Subject: [PATCH 21/23] Hibernate 7: getFullyQualifiedExpression resolves like the built-in predicates; an order handler asks the selection alias first Folding the selection of a sqlProjection into getFullyQualifiedExpression changed what the built-in predicates restrict, because they resolve through the same method: with a projection alias equal to a property, eq('height', 9) would have restricted the projection and not the column. SQL scopes a select alias to the order by and never to the where, and Hibernate 5 does the same, so the resolver is back to resolving properties and the aliases the translator registers. The javadoc of QueryPathContext says which aliases each lookup resolves and points to getSelectionAlias for the column alias of a sqlProjection or sqlGroupProjection; the guide says an order handler asks getSelectionAlias first and getFullyQualifiedExpression second, which orders as order(alias) does, and a criterion handler asks getFullyQualifiedExpression alone. The spec pins the behaviour: a projection-only alias does not resolve as a path, a property of the same name does, the handler orders by the projection in both cases, and eq on that name restricts the property. --- .../orm/hibernate/query/JpaQueryContext.java | 3 +- .../orm/hibernate/query/QueryPathContext.java | 21 ++++++--- ...ateCriteriaBuilderSqlProjectionSpec.groovy | 44 ++++++++++++------- .../src/docs/asciidoc/querying/criteria.adoc | 8 +++- 4 files changed, 51 insertions(+), 25 deletions(-) diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java index 3ce4a6cf585..549ae1cdafe 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/JpaQueryContext.java @@ -186,8 +186,7 @@ public Expression getFullyQualifiedExpression(String path) { return parent.getFullyQualifiedExpression("root." + path.substring(8)); } } - Expression resolved = resolver.resolve(path); - return resolved != null ? resolved : selectionAliases.get(path); + return resolver.resolve(path); } @Override diff --git a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java index 127a246e1d5..da46b99be54 100644 --- a/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java +++ b/grails-data-hibernate7/core/src/main/groovy/org/grails/orm/hibernate/query/QueryPathContext.java @@ -45,16 +45,25 @@ public interface QueryPathContext { Path getFullyQualifiedPath(String path); /** - * @param path a property path, or the alias of a projection - * @return the expression for it: a path, or, when no property resolves, the projected expression the alias of a - * {@code sqlProjection} stands for. A property takes precedence over an alias of the same name, since a - * restriction on that property must still restrict the property; {@link #getSelectionAlias(String)} asks for - * the alias alone + * Resolves a path the way the built-in criteria resolve the property of a restriction, so a restriction on + * {@code height} restricts the property whatever a projection is called. + * + * @param path a property path, or an alias that the translator registers for an association or a projection of + * the query's own, such as a function + * @return the expression for it: a path, or the expression such an alias stands for; {@code null} when nothing + * resolves. The column alias of a {@code sqlProjection} or {@code sqlGroupProjection} is not resolved here, + * as SQL scopes a select alias to the order by and never to the where: ask {@link #getSelectionAlias(String)} + * for it */ Expression getFullyQualifiedExpression(String path); /** - * @param alias the column alias of a {@code sqlProjection} of the query + * Returns the projection a column alias stands for, which is what the built-in {@code order(alias)} sorts by: an + * order handler that sorts by a column alias checks this first and {@link #getFullyQualifiedExpression(String)} + * second, and so orders by the projection when an alias and a property have the same name, as {@code order(alias)} + * does. + * + * @param alias the column alias of a {@code sqlProjection} or {@code sqlGroupProjection} of the query * @return the projected expression recorded under that alias, so that a handler can order by it, or {@code null} * when no projection of the query carries the alias */ diff --git a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy index cbc21995166..6e71d92decb 100644 --- a/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy +++ b/grails-data-hibernate7/core/src/test/groovy/grails/orm/HibernateCriteriaBuilderSqlProjectionSpec.groovy @@ -153,14 +153,13 @@ class HibernateCriteriaBuilderSqlProjectionSpec extends Specification { } @Rollback - void 'getFullyQualifiedExpression returns the projection a sqlProjection alias stands for, while a property of that name stays a property'() { - given: "a handler that orders by whatever getFullyQualifiedExpression returns for the alias, and one that records it" + void 'an order handler asks the selection alias first and the path second, and a criterion handler asks the path alone'() { + given: "an order handler that orders as the built-in order does, and a criterion handler that restricts as eq does" saveBoxes() - List resolved = [] + List resolvedAsPath = [] PredicateGenerator.registerOrderHandler(SqlProjectionAliasOrder) { query, root, cb, order, context -> - def expression = context.getFullyQualifiedExpression(order.property) - resolved << expression?.getClass() - cb.desc(expression) + resolvedAsPath << context.getFullyQualifiedExpression(order.property) + cb.desc(context.getSelectionAlias(order.property) ?: context.getFullyQualifiedExpression(order.property)) } when: "the alias names no property" @@ -171,23 +170,38 @@ class HibernateCriteriaBuilderSqlProjectionSpec extends Specification { order(new SqlProjectionAliasOrder('scaled')) } - then: "the order uses the projected expression, as it does through getSelectionAlias" + then: "the path does not resolve it, the selection alias does, and the rows come in descending order of the projection" + resolvedAsPath == [null] rows == rows.sort(false).reverse() - resolved.every { it != null } sqlCapture.statements.any { it.contains('select width * 10 from') && it.contains('order by 1 desc') } - when: "the alias is the name of a property: the property wins, since a restriction on it must still restrict the property" - resolved.clear() + when: "the alias is the name of a property" + resolvedAsPath.clear() + sqlCapture.statements.clear() SqlProjectionBox.createCriteria().list { projections { - sqlProjection 'width * 10 as width', 'width', INTEGER + sqlProjection 'width * 10 as height', 'height', INTEGER } - order(new SqlProjectionAliasOrder('width')) + order(new SqlProjectionAliasOrder('height')) } - then: - resolved.size() == 1 - sqlCapture.statements.last().contains('order by') + then: "the path resolves the property, and the handler still orders by the projection, as order(alias) does" + resolvedAsPath.size() == 1 + resolvedAsPath[0] != null + sqlCapture.statements.any { it.contains('order by 1 desc') } + + when: "a restriction on that name goes through the path" + sqlCapture.statements.clear() + List restricted = SqlProjectionBox.createCriteria().list { + projections { + sqlProjection 'width * 10 as height', 'height', INTEGER + } + eq('height', 9) + } + + then: "it restricts the property, not the projection, as in SQL" + restricted.size() == 2 + sqlCapture.statements.any { it.contains('.height=?') } cleanup: PredicateGenerator.clearCustomOrderHandlers() diff --git a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc index 4f000cbb1aa..e6723f6af62 100644 --- a/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc +++ b/grails-data-hibernate7/docs/src/docs/asciidoc/querying/criteria.adoc @@ -424,8 +424,12 @@ The handlers convert them. A criterion handler is given the query, its root, the criterion and the query's `QueryPathContext`. Through the context, `getFullyQualifiedPath` resolves a property path the way the built-in criteria do: `alias.property`, for an alias created with `createAlias`, or `association.property` becomes a path on the join GORM created for that association, so a custom criterion and a built-in one on the -same association restrict the same row rather than each joining the association again, and `getSelectionAlias` -returns the expression a `sqlProjection` records under its column alias, so an order can sort by a projected value. An order handler is given +same association restrict the same row rather than each joining the association again. `getSelectionAlias` returns +the expression a `sqlProjection` or `sqlGroupProjection` records under its column alias. An order handler that sorts +by a column alias asks `getSelectionAlias` first and `getFullyQualifiedExpression` second, which orders by the +projection when an alias and a property have the same name, as `order(alias)` does; a criterion handler asks +`getFullyQualifiedExpression` alone, so a restriction on a property restricts the property, as in SQL, where a select +alias is visible to the `order by` and not to the `where`. An order handler is given the query, its root, the criteria builder, the order and the context, and returns the JPA order, or `null` to leave the order out. A query whose result is a count leaves every order out by itself, so no handler runs for it: From 6f172d6c29e259319ffb57c270a6492a1ca7da42 Mon Sep 17 00:00:00 2001 From: Scott Murphy Heiberg Date: Sun, 25 Oct 2026 08:54:25 -0700 Subject: [PATCH 22/23] Keep refreshSystemEnvironment within its package PropertySourcesConfig.refresh(), in the same package, is its only caller and PropertySourcesConfig its only subclass, so the method need not be protected on a class that is deprecated and may be replaced. --- .../src/main/groovy/org/grails/config/NavigableMapConfig.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java index 4449453a8fd..c092527d5e3 100644 --- a/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java +++ b/grails-core/src/main/groovy/org/grails/config/NavigableMapConfig.java @@ -226,7 +226,7 @@ public T getProperty(String key, Class targetType, T defaultValue, List Date: Sun, 25 Oct 2026 08:54:25 -0700 Subject: [PATCH 23/23] Lead with installing environment variables before startup in the upgrade guide The guide led with refresh(), a method of the configuration implementation rather than of the Config interface. It now leads with installing the variables before the application context is created, through the environment of the Gradle test task or an extension that runs before the context starts, and presents refresh() as what the implementation offers a test that cannot, which compiles only dynamically and may change with the implementation. The what's new entry keeps what a user can observe and drops the sentence about the walk through a dotted key. The pin of a key that runs past a scalar in PropertySourcesConfigSpec names the 9.0.x change that makes such a key absent. --- .../org/grails/config/PropertySourcesConfigSpec.groovy | 1 + grails-doc/src/en/guide/introduction/whatsNew.adoc | 2 +- grails-doc/src/en/guide/upgrading/upgrading81x.adoc | 6 ++++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy index 417557ae42c..a4d0a47e645 100644 --- a/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy +++ b/grails-core/src/test/groovy/org/grails/config/PropertySourcesConfigSpec.groovy @@ -133,6 +133,7 @@ class PropertySourcesConfigSpec extends Specification { config.getProperty('...') == null and: "a key that runs past a scalar answers with that scalar, as it always has" + // Grails 8 behaviour, pinned on purpose: apache/grails-core#16732 makes such a key absent on 9.0.x config.getProperty('plain.nested.value.deeper') == 'found' config.containsProperty('plain.nested.value.deeper') } diff --git a/grails-doc/src/en/guide/introduction/whatsNew.adoc b/grails-doc/src/en/guide/introduction/whatsNew.adoc index 9065c125c77..57a8ca21f2c 100644 --- a/grails-doc/src/en/guide/introduction/whatsNew.adoc +++ b/grails-doc/src/en/guide/introduction/whatsNew.adoc @@ -693,7 +693,7 @@ interval needs, and <> for the details. ==== Faster Configuration Lookups -`grailsApplication.config.getProperty` no longer probes the process environment on every call. The environment is read once, when the configuration is created, and its variable names are indexed, so a property that no variable can match is answered without touching `System.getenv`, and the walk through a dotted key no longer tokenises it. A test that installs environment variables after the application has started calls `refresh()` on the configuration to have them seen; see xref:upgrading#upgrading81x[the upgrade guide]. +`grailsApplication.config.getProperty` no longer probes the process environment on every call. The environment is read once, when the configuration is created, and its variable names are indexed, so a property that no variable can match is answered without touching `System.getenv`. A test that installs environment variables installs them before the application context is created, or calls `refresh()` on the configuration afterwards to have them seen; see xref:upgrading#upgrading81x[the upgrade guide]. ==== Compiled Tag Resolution diff --git a/grails-doc/src/en/guide/upgrading/upgrading81x.adoc b/grails-doc/src/en/guide/upgrading/upgrading81x.adoc index ce16d5f1960..53ab4253730 100644 --- a/grails-doc/src/en/guide/upgrading/upgrading81x.adoc +++ b/grails-doc/src/en/guide/upgrading/upgrading81x.adoc @@ -60,13 +60,15 @@ Default projects consistently include Mockito with Spock. When both Grails Data For applications **no changes are required**: the environment of a running JVM cannot change, so every lookup has the same answer as before. -A test that installs environment variables reflectively, with a library such as System Stubs or JUnit Pioneer, must install them before the application context is created, or ask the configuration to read the environment again afterwards: +A test that installs environment variables reflectively, with a library such as System Stubs or JUnit Pioneer, installs them before the application context is created, through the `environment` of the Gradle test task or an extension that runs before the context starts, so the configuration reads them when it is built. That is the supported approach: the configuration then behaves exactly as it does for a running application. + +A test that can only install them after the application has started asks the configuration to read the environment again: [source,groovy] ---- grailsApplication.config.refresh() ---- -`refresh()` is a method of the configuration implementation rather than of the `Config` interface, so it is a call for dynamically compiled test code. It reads the environment again and merges the property sources again, which replaces a value the test set on the configuration directly wherever a property source defines the same key. Call it again once the variables are removed: the configuration keeps the environment it last read. +`refresh()` is a method of the configuration implementation rather than of the `Config` interface, so it is a call for dynamically compiled test code and may change with the implementation. It reads the environment again and merges the property sources again, which replaces a value the test set on the configuration directly wherever a property source defines the same key. Call it again once the variables are removed: the configuration keeps the environment it last read. Two smaller differences: variable names are upper-cased with `Locale.ROOT`, as Spring does, so a key containing `i` matches its upper-cased variable under a Turkish default locale too; and `getProperty(null)` returns the default value instead of throwing a `NullPointerException`.