You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@grails.apache.org by ja...@apache.org on 2026/06/04 20:33:52 UTC
(grails-core) branch grails8-groovy5-sb4 updated: Fix GROOVY-12063 at source and correct Groovy 5 workaround records
This is an automated email from the ASF dual-hosted git repository.
jamesfredley pushed a commit to branch grails8-groovy5-sb4
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to refs/heads/grails8-groovy5-sb4 by this push:
new 9b47e14932 Fix GROOVY-12063 at source and correct Groovy 5 workaround records
9b47e14932 is described below
commit 9b47e149328ae94f2130f09dcb18b37d87d0fa4c
Author: James Fredley <ja...@users.noreply.github.com>
AuthorDate: Thu Jun 4 16:33:25 2026 -0400
Fix GROOVY-12063 at source and correct Groovy 5 workaround records
Replace the anonymous-HashMap-with-instance-initializer for
ConstrainedProperty.DEFAULT_MESSAGES with a Groovy map literal. On Groovy 5
(GROOVY-12063) the bareword references to the sibling DEFAULT_* constants
inside such an initializer compile to dynamic getProperty calls on `this`
(the HashMap); because the receiver is a Map, the MOP resolves them as key
lookups on the still-empty map, so every entry was captured as null. A map
literal resolves the constants against the enclosing interface scope and is
correct on every Groovy version.
This fixes DEFAULT_MESSAGES for all consumers, so the AbstractConstraint
MESSAGE_BUNDLE fallback is reverted to the base getDefaultMessage form. Also
add the previously-missing default.not.unique.message entry so the map covers
every key in DefaultErrorMessages.properties, and make DefaultMessageResolutionSpec
iterate the whole bundle as a complete regression guard.
Correct the now-disproven root-cause comments: the AbstractConstraint and
DefaultMessageResolutionSpec "interface field-initializer order" explanation
was wrong (the interface clinit field-init order is actually correct); the real
cause is the Map-MOP getProperty dispatch above. Reference GROOVY-12063. Add the
GROOVY-12062 id and the non-empty-finally trigger detail to the
ControllerActionTransformer ClassNodeSkip comment.
Assisted-by: claude-code:claude-4.8-opus
---
.../compiler/web/ControllerActionTransformer.java | 29 +++++++------
.../gorm/validation/ConstrainedProperty.groovy | 47 +++++++++++++---------
.../validation/constraints/AbstractConstraint.java | 30 +-------------
.../DefaultMessageResolutionSpec.groovy | 32 ++++++---------
4 files changed, 58 insertions(+), 80 deletions(-)
diff --git a/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java b/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java
index f877ae464f..52f9fe39ea 100644
--- a/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java
+++ b/grails-controllers/src/main/groovy/org/grails/compiler/web/ControllerActionTransformer.java
@@ -400,19 +400,22 @@ public class ControllerActionTransformer implements GrailsArtefactClassInjector,
MethodNode method = null;
if (methodNode.getParameters().length > 0) {
- // Workaround for an upstream Groovy 5 code-generation bug (this is not a Grails-side fix of
- // the bug itself). For a parameterized action the transformer generates a zero-arg wrapper
- // that binds the request parameters to LOCAL variables and then delegates to the original
- // action via `this.action(p1, p2)`. Groovy's OptimizingStatementWriter emits that delegating
- // call twice behind a `__$stMC` guard: a fast path that correctly loads the locals, and a
- // slow path that instead re-resolves the very same locals as dynamic property reads
- // (`this.getProperty("p1")`). The slow path is the one taken whenever the controller's
- // metaclass is non-standard - i.e. in any live application (Spring Security, plugins, ...) -
- // so under classic (indy=false) codegen the call throws MissingPropertyException at runtime
- // even though the compiled fast path looks correct (which is why it cannot be reproduced from
- // an isolated script). Tagging the controller with ClassNodeSkip suppresses the fast/slow
- // fork so a single, correct path is emitted. Remove once the upstream OptimizingStatementWriter
- // bug is fixed. Reproducer: https://github.com/jamesfredley/groovy5-controller-action-param-scope-bug
+ // Workaround for upstream Groovy 5 bug GROOVY-12062 (this is not a Grails-side fix of the
+ // bug itself). For a parameterized action the transformer generates a zero-arg wrapper that
+ // binds the request parameters to LOCAL variables and then delegates to the original action
+ // via `this.action(p1, p2)`, with the body wrapped in a try/catch/finally. Under classic
+ // (indy=false) codegen Groovy's OptimizingStatementWriter emits that delegating call twice
+ // behind a `__$stMC` guard: a fast path that correctly loads the locals, and a slow path that
+ // re-resolves the very same locals as dynamic property reads (`this.getProperty("p1")`). The
+ // non-empty finally block triggers a CompileStack scope restore that drops the locals between
+ // the two emissions, so the slow path's getVariable lookup misses and falls back to
+ // getProperty. The slow path is the one taken whenever the controller's metaclass is
+ // non-standard - i.e. in any live application (Spring Security, plugins, ...) - so the call
+ // throws MissingPropertyException at runtime even though the compiled fast path looks correct
+ // (which is why it cannot be reproduced from an isolated script). Tagging the controller with
+ // ClassNodeSkip suppresses the fast/slow fork so a single, correct path is emitted. Remove
+ // once GROOVY-12062 is fixed upstream.
+ // Reproducer: https://github.com/jamesfredley/groovy5-controller-action-param-scope-bug
if (classNode.getNodeMetaData(OptimizingStatementWriter.ClassNodeSkip.class) == null) {
classNode.putNodeMetaData(OptimizingStatementWriter.ClassNodeSkip.class, new OptimizingStatementWriter.ClassNodeSkip());
}
diff --git a/grails-datamapping-validation/src/main/groovy/grails/gorm/validation/ConstrainedProperty.groovy b/grails-datamapping-validation/src/main/groovy/grails/gorm/validation/ConstrainedProperty.groovy
index f4b354a09b..8783f6002b 100644
--- a/grails-datamapping-validation/src/main/groovy/grails/gorm/validation/ConstrainedProperty.groovy
+++ b/grails-datamapping-validation/src/main/groovy/grails/gorm/validation/ConstrainedProperty.groovy
@@ -50,6 +50,7 @@ interface ConstrainedProperty extends Constrained {
String DEFAULT_INVALID_VALIDATOR_MESSAGE_CODE = 'default.invalid.validator.message'
String DEFAULT_DOESNT_MATCH_MESSAGE_CODE = 'default.doesnt.match.message'
String DEFAULT_BLANK_MESSAGE_CODE = 'default.blank.message'
+ String DEFAULT_NOT_UNIQUE_MESSAGE_CODE = 'default.not.unique.message'
String DEFAULT_BLANK_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_BLANK_MESSAGE_CODE)
String DEFAULT_DOESNT_MATCH_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_DOESNT_MATCH_MESSAGE_CODE)
String DEFAULT_INVALID_URL_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_INVALID_URL_MESSAGE_CODE)
@@ -65,26 +66,32 @@ interface ConstrainedProperty extends Constrained {
String DEFAULT_INVALID_MIN_SIZE_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_INVALID_MIN_SIZE_MESSAGE_CODE)
String DEFAULT_NULL_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_NULL_MESSAGE_CODE)
String DEFAULT_INVALID_VALIDATOR_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_INVALID_VALIDATOR_MESSAGE_CODE)
-
- Map<String, String> DEFAULT_MESSAGES = new HashMap<String, String>() {
- {
- put(DEFAULT_BLANK_MESSAGE_CODE, DEFAULT_BLANK_MESSAGE)
- put(DEFAULT_DOESNT_MATCH_MESSAGE_CODE, DEFAULT_DOESNT_MATCH_MESSAGE)
- put(DEFAULT_INVALID_CREDIT_CARD_MESSAGE_CODE, DEFAULT_INVALID_CREDIT_CARD_MESSAGE)
- put(DEFAULT_INVALID_EMAIL_MESSAGE_CODE, DEFAULT_INVALID_EMAIL_MESSAGE)
- put(DEFAULT_INVALID_MAX_MESSAGE_CODE, DEFAULT_INVALID_MAX_MESSAGE)
- put(DEFAULT_INVALID_MAX_SIZE_MESSAGE_CODE, DEFAULT_INVALID_MAX_SIZE_MESSAGE)
- put(DEFAULT_INVALID_MIN_MESSAGE_CODE, DEFAULT_INVALID_MIN_MESSAGE)
- put(DEFAULT_INVALID_MIN_SIZE_MESSAGE_CODE, DEFAULT_INVALID_MIN_SIZE_MESSAGE)
- put(DEFAULT_INVALID_RANGE_MESSAGE_CODE, DEFAULT_INVALID_RANGE_MESSAGE)
- put(DEFAULT_INVALID_SIZE_MESSAGE_CODE, DEFAULT_INVALID_SIZE_MESSAGE)
- put(DEFAULT_INVALID_URL_MESSAGE_CODE, DEFAULT_INVALID_URL_MESSAGE)
- put(DEFAULT_NOT_EQUAL_MESSAGE_CODE, DEFAULT_NOT_EQUAL_MESSAGE)
- put(DEFAULT_NOT_INLIST_MESSAGE_CODE, DEFAULT_NOT_IN_LIST_MESSAGE)
- put(DEFAULT_NULL_MESSAGE_CODE, DEFAULT_NULL_MESSAGE)
- put(DEFAULT_INVALID_VALIDATOR_MESSAGE_CODE, DEFAULT_INVALID_VALIDATOR_MESSAGE)
- }
- }
+ String DEFAULT_NOT_UNIQUE_MESSAGE = MESSAGE_BUNDLE.getString(DEFAULT_NOT_UNIQUE_MESSAGE_CODE)
+
+ // Built with a Groovy map literal rather than an anonymous-HashMap-with-instance-initializer.
+ // On Groovy 5 (GROOVY-12063) the bareword references to the sibling DEFAULT_* constants inside
+ // such an initializer compile to dynamic getProperty calls on `this` (the HashMap), which the
+ // MOP resolves as map-key lookups on the still-empty map, so every entry was captured as null.
+ // A map literal resolves the constants against the enclosing interface scope and is correct on
+ // every Groovy version.
+ Map<String, String> DEFAULT_MESSAGES = [
+ (DEFAULT_BLANK_MESSAGE_CODE) : DEFAULT_BLANK_MESSAGE,
+ (DEFAULT_DOESNT_MATCH_MESSAGE_CODE) : DEFAULT_DOESNT_MATCH_MESSAGE,
+ (DEFAULT_INVALID_CREDIT_CARD_MESSAGE_CODE): DEFAULT_INVALID_CREDIT_CARD_MESSAGE,
+ (DEFAULT_INVALID_EMAIL_MESSAGE_CODE) : DEFAULT_INVALID_EMAIL_MESSAGE,
+ (DEFAULT_INVALID_MAX_MESSAGE_CODE) : DEFAULT_INVALID_MAX_MESSAGE,
+ (DEFAULT_INVALID_MAX_SIZE_MESSAGE_CODE) : DEFAULT_INVALID_MAX_SIZE_MESSAGE,
+ (DEFAULT_INVALID_MIN_MESSAGE_CODE) : DEFAULT_INVALID_MIN_MESSAGE,
+ (DEFAULT_INVALID_MIN_SIZE_MESSAGE_CODE) : DEFAULT_INVALID_MIN_SIZE_MESSAGE,
+ (DEFAULT_INVALID_RANGE_MESSAGE_CODE) : DEFAULT_INVALID_RANGE_MESSAGE,
+ (DEFAULT_INVALID_SIZE_MESSAGE_CODE) : DEFAULT_INVALID_SIZE_MESSAGE,
+ (DEFAULT_INVALID_URL_MESSAGE_CODE) : DEFAULT_INVALID_URL_MESSAGE,
+ (DEFAULT_NOT_EQUAL_MESSAGE_CODE) : DEFAULT_NOT_EQUAL_MESSAGE,
+ (DEFAULT_NOT_INLIST_MESSAGE_CODE) : DEFAULT_NOT_IN_LIST_MESSAGE,
+ (DEFAULT_NULL_MESSAGE_CODE) : DEFAULT_NULL_MESSAGE,
+ (DEFAULT_INVALID_VALIDATOR_MESSAGE_CODE) : DEFAULT_INVALID_VALIDATOR_MESSAGE,
+ (DEFAULT_NOT_UNIQUE_MESSAGE_CODE) : DEFAULT_NOT_UNIQUE_MESSAGE
+ ]
String CREDIT_CARD_CONSTRAINT = 'creditCard'
String EMAIL_CONSTRAINT = 'email'
diff --git a/grails-datamapping-validation/src/main/groovy/org/grails/datastore/gorm/validation/constraints/AbstractConstraint.java b/grails-datamapping-validation/src/main/groovy/org/grails/datastore/gorm/validation/constraints/AbstractConstraint.java
index f1191b8bf9..6f12a65682 100644
--- a/grails-datamapping-validation/src/main/groovy/org/grails/datastore/gorm/validation/constraints/AbstractConstraint.java
+++ b/grails-datamapping-validation/src/main/groovy/org/grails/datastore/gorm/validation/constraints/AbstractConstraint.java
@@ -233,39 +233,13 @@ public abstract class AbstractConstraint implements Constraint {
return messageSource.getMessage(code, null, LocaleContextHolder.getLocale());
}
- return getDefaultMessageFromBundle(code);
+ return ConstrainedProperty.DEFAULT_MESSAGES.get(code);
}
catch (Exception e) {
- return getDefaultMessageFromBundle(code);
+ return ConstrainedProperty.DEFAULT_MESSAGES.get(code);
}
}
- /**
- * Resolves the default message for {@code code}, falling back to {@link ConstrainedProperty#MESSAGE_BUNDLE}
- * when {@link ConstrainedProperty#DEFAULT_MESSAGES} returns {@code null}.
- *
- * <p>{@code ConstrainedProperty} is a Groovy interface, so its fields are implicitly {@code static final}.
- * {@code DEFAULT_MESSAGES} is a map literal whose values reference the sibling {@code DEFAULT_*_MESSAGE}
- * String constants. Under Groovy 4 those String constants were already assigned by the time the map literal
- * ran, so the map held real messages. Under Groovy 5 the order in which interface field initializers run
- * changed: the {@code DEFAULT_MESSAGES} initializer now executes before the referenced constants are
- * assigned, so the map captures {@code null} for every value. {@code MESSAGE_BUNDLE} (a {@code ResourceBundle})
- * is unaffected and still resolves every code, so it is used as the fallback. Guarded by
- * {@code DefaultMessageResolutionSpec}; this can be removed if the constants resolve eagerly again upstream.</p>
- */
- private String getDefaultMessageFromBundle(String code) {
- String message = ConstrainedProperty.DEFAULT_MESSAGES.get(code);
- if (message == null) {
- try {
- message = ConstrainedProperty.MESSAGE_BUNDLE.getString(code);
- }
- catch (java.util.MissingResourceException ignored) {
- // Code not found in bundle
- }
- }
- return message;
- }
-
protected abstract void processValidate(Object target, Object propertyValue, Errors errors);
@Override
diff --git a/grails-datamapping-validation/src/test/groovy/org/grails/datastore/gorm/validation/constraints/DefaultMessageResolutionSpec.groovy b/grails-datamapping-validation/src/test/groovy/org/grails/datastore/gorm/validation/constraints/DefaultMessageResolutionSpec.groovy
index 84368d13fd..816a15f7dd 100644
--- a/grails-datamapping-validation/src/test/groovy/org/grails/datastore/gorm/validation/constraints/DefaultMessageResolutionSpec.groovy
+++ b/grails-datamapping-validation/src/test/groovy/org/grails/datastore/gorm/validation/constraints/DefaultMessageResolutionSpec.groovy
@@ -28,16 +28,18 @@ import spock.lang.Specification
import spock.lang.Unroll
/**
- * Reproduces the Groovy 5 {@link ConstrainedProperty} interface static-initialisation
- * regression and verifies that {@code AbstractConstraint.getDefaultMessage} still resolves
- * the correct default message when no Spring {@code MessageSource} is available.
+ * Regression guard for {@link ConstrainedProperty#DEFAULT_MESSAGES} on Groovy 5 (GROOVY-12063):
+ * verifies that {@code AbstractConstraint.getDefaultMessage} resolves the correct default message
+ * from the map when no Spring {@code MessageSource} is available.
*
- * <p>On Groovy 5 the {@code ConstrainedProperty.DEFAULT_MESSAGES} map is initialised with
- * null values: its map initialiser runs before the {@code DEFAULT_*_MESSAGE} constants it
- * references are assigned, so {@code DEFAULT_MESSAGES.get(code)} returns null even though
- * {@code MESSAGE_BUNDLE} resolves the same code correctly. {@code AbstractConstraint} works
- * around this by falling back to {@code MESSAGE_BUNDLE} - this spec guards that fallback so
- * it is not removed again while the regression is present.</p>
+ * <p>{@code DEFAULT_MESSAGES} must be built with a Groovy map literal rather than an
+ * anonymous-{@code HashMap}-with-instance-initializer. On Groovy 5 the bareword references to the
+ * sibling {@code DEFAULT_*_MESSAGE} constants inside such an initializer compile to dynamic
+ * {@code getProperty} calls on {@code this} (the {@code HashMap}); because the receiver is a
+ * {@code Map}, the MOP resolves them as key lookups on the still-empty map, so every entry was
+ * captured as {@code null} and {@code DEFAULT_MESSAGES.get(code)} returned {@code null}. The map
+ * literal resolves the constants against the enclosing interface scope and is correct on every
+ * version; this spec fails if that regresses.</p>
*/
class DefaultMessageResolutionSpec extends Specification {
@@ -50,16 +52,8 @@ class DefaultMessageResolutionSpec extends Specification {
constraint.resolveDefaultMessage(code) != null
constraint.resolveDefaultMessage(code) == ConstrainedProperty.MESSAGE_BUNDLE.getString(code)
- where:
- code << [
- ConstrainedProperty.DEFAULT_BLANK_MESSAGE_CODE,
- ConstrainedProperty.DEFAULT_NULL_MESSAGE_CODE,
- ConstrainedProperty.DEFAULT_INVALID_EMAIL_MESSAGE_CODE,
- ConstrainedProperty.DEFAULT_INVALID_URL_MESSAGE_CODE,
- ConstrainedProperty.DEFAULT_INVALID_RANGE_MESSAGE_CODE,
- ConstrainedProperty.DEFAULT_NOT_INLIST_MESSAGE_CODE,
- ConstrainedProperty.DEFAULT_INVALID_VALIDATOR_MESSAGE_CODE
- ]
+ where: "every code in the default error-message bundle is covered by DEFAULT_MESSAGES"
+ code << ConstrainedProperty.MESSAGE_BUNDLE.keySet()
}
static class TestConstraint extends AbstractConstraint {