This is an automated email from the ASF dual-hosted git repository.
borinquenkid pushed a commit to branch
chore/8.1.x-datamapping-core-transform-cleanup
in repository https://gitbox.apache.org/repos/asf/grails-core.git
The following commit(s) were added to
refs/heads/chore/8.1.x-datamapping-core-transform-cleanup by this push:
new 1720abd711 Add specs and clean up transactions.transform/transform in
grails-datamapping-core
1720abd711 is described below
commit 1720abd7117ef96c4703819792bff37ee8a11da4
Author: Walter Duque de Estrada <[email protected]>
AuthorDate: Tue Aug 18 12:55:22 2026 -0500
Add specs and clean up transactions.transform/transform in
grails-datamapping-core
Closes direct unit-spec coverage gaps left after PR #16148 for
RollbackTransform, AbstractDatastoreMethodDecoratingTransformation,
AbstractMethodDecoratingTransformation, AstMethodDispatchUtils, and
AstPropertyResolveUtils. Writing the AstPropertyResolveUtils spec
surfaced a real correctness bug: its property cache was keyed by
class name (a String), so two distinct ClassNode instances sharing a
name (e.g. from separate compilations of dynamically-generated/test
classes) silently corrupted each other's cached property data under
concurrent use. Fixed by keying the cache on ClassNode identity via a
synchronized IdentityHashMap.
Also extracts the duplicated applied-marker idempotency check/mark
pattern (repeated across AbstractGormASTTransformation,
AbstractMethodDecoratingTransformation, and
AbstractDatastoreMethodDecoratingTransformation) into shared
isAlreadyApplied/markApplied helpers, and applies a batch of small
cleanups flagged by static analysis: equals() calls replaced with ==,
an unused method parameter removed, Java 21 instanceof pattern
variables replacing raw-type casts, String#isEmpty() over
length()==0, Class#getDeclaredConstructor().newInstance() over the
deprecated Class#newInstance(), and two stray doc/comment fixes.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
---
.../transform/TransactionalTransform.groovy | 6 +-
...tDatastoreMethodDecoratingTransformation.groovy | 5 +-
.../transform/AbstractGormASTTransformation.groovy | 28 +-
.../AbstractMethodDecoratingTransformation.groovy | 5 +-
...stractTraitApplyingGormASTTransformation.groovy | 2 +-
.../gorm/transform/AstPropertyResolveUtils.java | 55 ++--
.../gorm/transform/GormASTTransformationClass.java | 3 +-
.../transform/OrderedGormTransformation.groovy | 2 +-
.../transform/RollbackTransformSpec.groovy | 58 +++++
...astoreMethodDecoratingTransformationSpec.groovy | 183 +++++++++++++
...stractMethodDecoratingTransformationSpec.groovy | 220 ++++++++++++++++
.../gorm/transform/ApplyTestMethodDecorating.java} | 18 +-
.../transform/AstMethodDispatchUtilsSpec.groovy | 171 ++++++++++++
.../transform/AstPropertyResolveUtilsSpec.groovy | 290 +++++++++++++++++++++
.../TestMethodDecoratingTransformation.groovy | 72 +++++
15 files changed, 1072 insertions(+), 46 deletions(-)
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transactions/transform/TransactionalTransform.groovy
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transactions/transform/TransactionalTransform.groovy
index 3f1e5a2d3c..4ffe2655d9 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transactions/transform/TransactionalTransform.groovy
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transactions/transform/TransactionalTransform.groovy
@@ -200,7 +200,7 @@ class TransactionalTransform extends
AbstractDatastoreMethodDecoratingTransforma
@Override
protected void enhanceClassNode(SourceUnit source, AnnotationNode
annotationNode, ClassNode declaringClassNode) {
- weaveTransactionManagerAware(sourceUnit, annotationNode,
declaringClassNode)
+ weaveTransactionManagerAware(annotationNode, declaringClassNode)
super.enhanceClassNode(source, annotationNode, declaringClassNode)
}
@@ -221,7 +221,7 @@ class TransactionalTransform extends
AbstractDatastoreMethodDecoratingTransforma
}
- protected void weaveTransactionManagerAware(SourceUnit source,
AnnotationNode annotationNode, ClassNode declaringClassNode) {
+ protected void weaveTransactionManagerAware(AnnotationNode annotationNode,
ClassNode declaringClassNode) {
if (declaringClassNode.getNodeMetaData(APPLIED_MARKER) ==
APPLIED_MARKER) {
return
}
@@ -419,7 +419,7 @@ class TransactionalTransform extends
AbstractDatastoreMethodDecoratingTransforma
final ClassNode rollbackRuleAttributeClassNode =
make(RollbackRuleAttribute)
final ClassNode noRollbackRuleAttributeClassNode =
make(NoRollbackRuleAttribute)
final Map<String, Expression> members = annotationNode.getMembers()
- if (READ_ONLY_TYPE.equals(annotationNode.classNode)) {
+ if (READ_ONLY_TYPE == annotationNode.classNode) {
methodBody.addStatement(
assignS(propX(transactionAttributeVar, 'readOnly'),
ConstantExpression.TRUE)
)
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformation.groovy
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformation.groovy
index e8dac8d3b1..466adc9c30 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformation.groovy
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformation.groovy
@@ -79,14 +79,13 @@ abstract class
AbstractDatastoreMethodDecoratingTransformation extends AbstractM
@Override
protected void enhanceClassNode(SourceUnit source, AnnotationNode
annotationNode, ClassNode declaringClassNode) {
- def appliedMarker = getAppliedMarker()
- if (declaringClassNode.getNodeMetaData(appliedMarker) ==
appliedMarker) {
+ if (isAlreadyApplied(declaringClassNode)) {
return
}
if (declaringClassNode.isInterface()) {
return
}
- declaringClassNode.putNodeMetaData(appliedMarker, appliedMarker)
+ markApplied(declaringClassNode)
Expression connectionName = annotationNode.getMember('connection')
boolean hasDataSourceProperty = connectionName != null
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractGormASTTransformation.groovy
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractGormASTTransformation.groovy
index 6749943ed5..2a35d7d6bd 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractGormASTTransformation.groovy
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractGormASTTransformation.groovy
@@ -55,18 +55,38 @@ abstract class AbstractGormASTTransformation extends
AbstractASTTransformation i
return
}
- Object appliedMarker = getAppliedMarker()
- if (annotatedNode.getNodeMetaData(appliedMarker) == appliedMarker) {
+ if (isAlreadyApplied(annotatedNode)) {
return
}
visit(source, annotationNode, annotatedNode)
- annotatedNode.putNodeMetaData(appliedMarker, appliedMarker)
+ markApplied(annotatedNode)
}
protected boolean isValidAnnotation(AnnotationNode annotationNode,
AnnotatedNode classNode) {
- return getAnnotationType().equals(annotationNode.getClassNode()) ||
!(classNode instanceof ClassNode)
+ return getAnnotationType() == annotationNode.getClassNode() ||
!(classNode instanceof ClassNode)
+ }
+
+ /**
+ * Whether the given node already carries this transformation's applied
marker.
+ *
+ * @param node The node
+ * @return true if {@link #visit} (or an equivalent per-method/per-class
idempotency check in a subclass) has already run for this node
+ */
+ protected boolean isAlreadyApplied(AnnotatedNode node) {
+ Object appliedMarker = getAppliedMarker()
+ node.getNodeMetaData(appliedMarker) == appliedMarker
+ }
+
+ /**
+ * Marks the given node as having had this transformation applied, so a
later {@link #isAlreadyApplied} check short-circuits.
+ *
+ * @param node The node
+ */
+ protected void markApplied(AnnotatedNode node) {
+ Object appliedMarker = getAppliedMarker()
+ node.putNodeMetaData(appliedMarker, appliedMarker)
}
abstract void visit(SourceUnit source, AnnotationNode annotationNode,
AnnotatedNode annotatedNode)
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformation.groovy
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformation.groovy
index be943b4042..0938755b33 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformation.groovy
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformation.groovy
@@ -187,15 +187,14 @@ abstract class AbstractMethodDecoratingTransformation
extends AbstractGormASTTra
* @return The new method's body
*/
protected MethodNode weaveNewMethod(SourceUnit sourceUnit, AnnotationNode
annotationNode, ClassNode classNode, MethodNode methodNode, Map<String,
ClassNode> genericsSpec) {
- Object appliedMarker = getAppliedMarker()
- if (methodNode.getNodeMetaData(appliedMarker) == appliedMarker) {
+ if (isAlreadyApplied(methodNode)) {
return methodNode
}
if (methodNode.isAbstract()) {
return methodNode
}
- methodNode.putNodeMetaData(appliedMarker, appliedMarker)
+ markApplied(methodNode)
enhanceClassNode(sourceUnit, annotationNode, classNode)
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractTraitApplyingGormASTTransformation.groovy
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractTraitApplyingGormASTTransformation.groovy
index d072c511ad..55ecc8c13b 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractTraitApplyingGormASTTransformation.groovy
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AbstractTraitApplyingGormASTTransformation.groovy
@@ -116,7 +116,7 @@ abstract class AbstractTraitApplyingGormASTTransformation
extends AbstractGormAS
}
void visitAfterTraitApplied(SourceUnit sourceUnit, AnnotationNode
annotationNode, ClassNode classNode) {
- // no-dop
+ // no-op
}
protected abstract Class getTraitClass()
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtils.java
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtils.java
index 4deeed44ea..64e802c269 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtils.java
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtils.java
@@ -20,7 +20,9 @@
package org.grails.datastore.gorm.transform;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
+import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
@@ -47,7 +49,14 @@ import org.grails.datastore.mapping.reflect.NameUtils;
* @since 6.1
*/
public class AstPropertyResolveUtils {
- protected static Map<String, Map<String, ClassNode>> cachedClassProperties
= new HashMap<>();
+
+ // ClassNode#equals()/hashCode() compare by name, so two distinct
ClassNode instances from
+ // separate compilations (as happens with dynamically-generated/test
classes) can legitimately
+ // share a name. Keying by name alone would let one class's resolved
properties leak into an
+ // unrelated class node that happens to share it, so the cache is keyed by
ClassNode identity
+ // instead, and guarded so the check-then-populate-then-store sequence
below is atomic.
+ protected static final Map<ClassNode, Map<String, ClassNode>>
cachedClassProperties =
+ Collections.synchronizedMap(new IdentityHashMap<>());
/**
* Resolves the type of of the given property
@@ -57,7 +66,7 @@ public class AstPropertyResolveUtils {
* @return The type
*/
public static ClassNode getPropertyType(ClassNode classNode, String
propertyName) {
- if (propertyName == null || propertyName.length() == 0) {
+ if (propertyName == null || propertyName.isEmpty()) {
return null;
}
Map<String, ClassNode> cachedProperties =
getPropertiesFromCache(classNode);
@@ -94,22 +103,24 @@ public class AstPropertyResolveUtils {
}
private static Map<String, ClassNode> getPropertiesFromCache(ClassNode
classNode) {
- String className = classNode.getName();
- Map<String, ClassNode> cachedProperties =
cachedClassProperties.get(className);
- if (cachedProperties == null) {
- cachedProperties = new HashMap<>();
- boolean isDomainClass = AstUtils.isDomainClass(classNode);
- if (isDomainClass) {
- cachedProperties.put(GormProperties.IDENTITY, new
ClassNode(Long.class));
- cachedProperties.put(GormProperties.VERSION, new
ClassNode(Long.class));
- }
- cachedClassProperties.put(className, cachedProperties);
- ClassNode currentNode = classNode;
- while (currentNode != null &&
!currentNode.equals(ClassHelper.OBJECT_TYPE)) {
- populatePropertiesForClassNode(currentNode, cachedProperties,
isDomainClass, !isDomainClass);
- currentNode = currentNode.getSuperClass();
+ synchronized (cachedClassProperties) {
+ Map<String, ClassNode> cachedProperties =
cachedClassProperties.get(classNode);
+ if (cachedProperties == null) {
+ cachedProperties = new HashMap<>();
+ boolean isDomainClass = AstUtils.isDomainClass(classNode);
+ if (isDomainClass) {
+ cachedProperties.put(GormProperties.IDENTITY, new
ClassNode(Long.class));
+ cachedProperties.put(GormProperties.VERSION, new
ClassNode(Long.class));
+ }
+ cachedClassProperties.put(classNode, cachedProperties);
+ ClassNode currentNode = classNode;
+ while (currentNode != null &&
!currentNode.equals(ClassHelper.OBJECT_TYPE)) {
+ populatePropertiesForClassNode(currentNode,
cachedProperties, isDomainClass, !isDomainClass);
+ currentNode = currentNode.getSuperClass();
+ }
}
- } return cachedProperties;
+ return cachedProperties;
+ }
}
private static void populatePropertiesForClassNode(ClassNode classNode,
Map<String, ClassNode> cachedProperties, boolean isDomainClass, boolean
allowAbstract) {
@@ -155,12 +166,11 @@ public class AstPropertyResolveUtils {
private static void cachePropertiesForAssociationMetadata(Map<String,
ClassNode> cachedProperties, ClassPropertyFetcher propertyFetcher, String
associationMetadataName) {
if (propertyFetcher.isReadableProperty(associationMetadataName)) {
Object propertyValue =
propertyFetcher.getPropertyValue(associationMetadataName);
- if (propertyValue instanceof Map) {
- Map hasManyMap = (Map) propertyValue;
+ if (propertyValue instanceof Map<?, ?> hasManyMap) {
for (Object propertyName : hasManyMap.keySet()) {
Object val = hasManyMap.get(propertyName);
- if (val instanceof Class) {
- cachedProperties.put(propertyName.toString(),
ClassHelper.make((Class) val).getPlainNodeReference());
+ if (val instanceof Class<?> valType) {
+ cachedProperties.put(propertyName.toString(),
ClassHelper.make(valType).getPlainNodeReference());
}
}
}
@@ -168,8 +178,7 @@ public class AstPropertyResolveUtils {
}
private static void populatePropertiesForInitialExpression(Map<String,
ClassNode> cachedProperties, Expression initialExpression) {
- if (initialExpression instanceof MapExpression) {
- MapExpression me = (MapExpression) initialExpression;
+ if (initialExpression instanceof MapExpression me) {
List<MapEntryExpression> mapEntryExpressions =
me.getMapEntryExpressions();
for (MapEntryExpression mapEntryExpression : mapEntryExpressions) {
Expression keyExpression =
mapEntryExpression.getKeyExpression();
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
index 7c7e298cdf..eb71c6a054 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
@@ -24,7 +24,8 @@ import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
/**
- * U
+ * Marker meta-annotation that points a GORM annotation (e.g. {@code
@Transactional}, {@code @Rollback},
+ * {@code @Tenant}) at the {@link
org.codehaus.groovy.transform.ASTTransformation} class that implements it.
*
* @author Graeme Rocher
* @since 6.1
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/OrderedGormTransformation.groovy
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/OrderedGormTransformation.groovy
index ab22018d10..f33e014081 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/OrderedGormTransformation.groovy
+++
b/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/OrderedGormTransformation.groovy
@@ -83,7 +83,7 @@ class OrderedGormTransformation extends
AbstractASTTransformation implements Com
String transformName = findTransformName(ann)
if (transformName) {
try {
- def newTransform =
ClassUtils.forName(transformName).newInstance()
+ def newTransform =
ClassUtils.forName(transformName).getDeclaredConstructor().newInstance()
if (newTransform instanceof ASTTransformation) {
if (newTransform instanceof CompilationUnitAware) {
((CompilationUnitAware)
newTransform).setCompilationUnit(compilationUnit)
diff --git
a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transactions/transform/RollbackTransformSpec.groovy
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transactions/transform/RollbackTransformSpec.groovy
new file mode 100644
index 0000000000..0ff617cbab
--- /dev/null
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transactions/transform/RollbackTransformSpec.groovy
@@ -0,0 +1,58 @@
+/*
+ * 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 org.grails.datastore.gorm.transactions.transform
+
+import spock.lang.Specification
+
+import grails.gorm.transactions.Rollback
+import org.apache.grails.common.compiler.GroovyTransformOrder
+
+/**
+ * {@code RollbackTransform} only overrides two methods of {@link
TransactionalTransform} and its
+ * end-to-end weaving behavior is already exercised (via the {@code @Rollback}
annotation) by
+ * {@code TransactionalTransformSpec}. This spec covers what those behavioral
tests can't: that the
+ * overrides themselves - the transaction template method name and the
transform ordering priority -
+ * are the values that make {@code @Rollback} behave differently from plain
{@code @Transactional}.
+ */
+class RollbackTransformSpec extends Specification {
+
+ void "getTransactionTemplateMethodName overrides the parent to route
through the rollback-forcing template method"() {
+ given:
+ RollbackTransform transform = new RollbackTransform()
+
+ expect:
+ transform.getTransactionTemplateMethodName() == 'executeAndRollback'
+ new TransactionalTransform().getTransactionTemplateMethodName() ==
'execute'
+ }
+
+ void "priority orders RollbackTransform after TransactionalTransform"() {
+ given:
+ RollbackTransform transform = new RollbackTransform()
+
+ expect:
+ transform.priority() == GroovyTransformOrder.ROLLBACK_ORDER
+ transform.priority() < GroovyTransformOrder.TRANSACTIONAL_ORDER
+ }
+
+ void "MY_TYPE identifies the Rollback annotation and the class extends
TransactionalTransform"() {
+ expect:
+ RollbackTransform.MY_TYPE.name == Rollback.name
+ TransactionalTransform.isAssignableFrom(RollbackTransform)
+ }
+}
diff --git
a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformationSpec.groovy
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformationSpec.groovy
new file mode 100644
index 0000000000..f9cc6cace0
--- /dev/null
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AbstractDatastoreMethodDecoratingTransformationSpec.groovy
@@ -0,0 +1,183 @@
+/*
+ * 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 org.grails.datastore.gorm.transform
+
+import java.lang.reflect.Modifier
+
+import groovy.transform.CompileStatic
+import org.codehaus.groovy.ast.AnnotationNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.MethodNode
+import org.codehaus.groovy.ast.Parameter
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codehaus.groovy.ast.stmt.BlockStatement
+import org.codehaus.groovy.control.SourceUnit
+
+import org.springframework.beans.factory.annotation.Autowired
+
+import spock.lang.Specification
+
+import org.grails.datastore.mapping.core.Datastore
+import
org.grails.datastore.mapping.core.connections.MultipleConnectionSourceCapableDatastore
+import org.grails.datastore.mapping.services.Service
+
+/**
+ * {@code AbstractDatastoreMethodDecoratingTransformation} is only ever
exercised in this module
+ * through {@code TransactionalTransform} and {@code TenantTransform}, both of
which are always driven
+ * through a real compilation, so {@code enhanceClassNode} is never called
directly and its Service-
+ * interface branch (only reachable for a class implementing {@code
org.grails.datastore.mapping.services.Service},
+ * which none of the real transform specs' fixtures do) is never exercised at
all. Because
+ * {@code enhanceClassNode} only touches the {@code ClassNode} it's given - it
never dereferences the
+ * {@code SourceUnit} parameter unless {@code compilationUnit} is set, which
it isn't for a bare
+ * instance - it can be called directly against hand-built {@code ClassNode}s,
the same technique used
+ * for the other abstract transformation specs in this package.
+ */
+class AbstractDatastoreMethodDecoratingTransformationSpec extends
Specification {
+
+ static class MinimalDatastoreDecoratingTransformation extends
AbstractDatastoreMethodDecoratingTransformation {
+
+ @Override
+ protected ClassNode getAnnotationType() {
+ ClassHelper.make(CompileStatic)
+ }
+
+ @Override
+ protected Object getAppliedMarker() {
+ 'datastore-decorating-applied-marker'
+ }
+
+ @Override
+ protected String getRenamedMethodPrefix() {
+ '$test__'
+ }
+
+ @Override
+ protected Expression buildDelegatingMethodCall(SourceUnit sourceUnit,
AnnotationNode annotationNode, ClassNode classNode,
+ MethodNode
methodNode, MethodCallExpression originalMethodCall, BlockStatement
newMethodBody) {
+ originalMethodCall
+ }
+
+ @Override
+ int priority() {
+ 0
+ }
+ }
+
+ private static ClassNode newTargetClassNode(String name) {
+ new ClassNode(name, Modifier.PUBLIC, ClassHelper.OBJECT_TYPE)
+ }
+
+ private static Parameter[] stringConnectionNameParam() {
+ [new Parameter(ClassHelper.STRING_TYPE, 'connectionName')] as
Parameter[]
+ }
+
+ void "enhanceClassNode adds a targetDatastore field and public
getter/setter methods to a plain class"() {
+ given:
+ MinimalDatastoreDecoratingTransformation transformation = new
MinimalDatastoreDecoratingTransformation()
+ ClassNode classNode =
newTargetClassNode('org.grails.datastore.gorm.transform.fixture.PlainDecoratedTarget')
+ AnnotationNode annotationNode = new
AnnotationNode(ClassHelper.make(CompileStatic))
+
+ when:
+ transformation.enhanceClassNode(null, annotationNode, classNode)
+
+ then: 'the datastore field is added, typed as the default Datastore'
+ classNode.getField('$targetDatastore').type ==
ClassHelper.make(Datastore)
+
+ and: 'both getTargetDatastore overloads are added as public methods'
+ classNode.getMethod('getTargetDatastore', Parameter.EMPTY_ARRAY) !=
null
+ classNode.getMethod('getTargetDatastore', stringConnectionNameParam())
!= null
+
+ and: 'a public setter is added, autowired but not required'
+ MethodNode setter = classNode.getMethods('setTargetDatastore')[0]
+ Modifier.isPublic(setter.modifiers)
+ AnnotationNode autowired =
setter.getAnnotations(ClassHelper.make(Autowired))[0]
+ ((ConstantExpression) autowired.getMember('required')).value == false
+ }
+
+ void "enhanceClassNode adds only protected getTargetDatastore methods and
no field when the class implements Service"() {
+ given:
+ MinimalDatastoreDecoratingTransformation transformation = new
MinimalDatastoreDecoratingTransformation()
+ ClassNode classNode =
newTargetClassNode('org.grails.datastore.gorm.transform.fixture.ServiceDecoratedTarget')
+ classNode.addInterface(ClassHelper.make(Service))
+ AnnotationNode annotationNode = new
AnnotationNode(ClassHelper.make(CompileStatic))
+
+ when:
+ transformation.enhanceClassNode(null, annotationNode, classNode)
+
+ then: 'no field is added - the Service is looked up rather than
injected'
+ classNode.getField('$targetDatastore') == null
+
+ and: 'both getTargetDatastore overloads are added, but protected
rather than public'
+ MethodNode noArgGetter = classNode.getMethod('getTargetDatastore',
Parameter.EMPTY_ARRAY)
+ MethodNode connectionGetter =
classNode.getMethod('getTargetDatastore', stringConnectionNameParam())
+ Modifier.isProtected(noArgGetter.modifiers)
+ Modifier.isProtected(connectionGetter.modifiers)
+
+ and: 'no setter is added at all'
+ classNode.getMethods('setTargetDatastore').empty
+ }
+
+ void "enhanceClassNode uses MultipleConnectionSourceCapableDatastore as
the field type when a connection name is specified"() {
+ given:
+ MinimalDatastoreDecoratingTransformation transformation = new
MinimalDatastoreDecoratingTransformation()
+ ClassNode classNode =
newTargetClassNode('org.grails.datastore.gorm.transform.fixture.ConnectionDecoratedTarget')
+ AnnotationNode annotationNode = new
AnnotationNode(ClassHelper.make(CompileStatic))
+ annotationNode.addMember('connection', new ConstantExpression('foo'))
+
+ when:
+ transformation.enhanceClassNode(null, annotationNode, classNode)
+
+ then:
+ classNode.getField('$targetDatastore').type ==
ClassHelper.make(MultipleConnectionSourceCapableDatastore)
+ }
+
+ void "enhanceClassNode is idempotent once the applied marker is already
set on the class node"() {
+ given:
+ MinimalDatastoreDecoratingTransformation transformation = new
MinimalDatastoreDecoratingTransformation()
+ ClassNode classNode =
newTargetClassNode('org.grails.datastore.gorm.transform.fixture.AlreadyAppliedTarget')
+ classNode.putNodeMetaData(transformation.getAppliedMarker(),
transformation.getAppliedMarker())
+ AnnotationNode annotationNode = new
AnnotationNode(ClassHelper.make(CompileStatic))
+
+ when:
+ transformation.enhanceClassNode(null, annotationNode, classNode)
+
+ then: 'nothing is added because the marker short-circuits enhancement'
+ classNode.getField('$targetDatastore') == null
+ classNode.methods.every { it.name != 'getTargetDatastore' }
+ }
+
+ void "enhanceClassNode is a no-op for an interface class node"() {
+ given:
+ MinimalDatastoreDecoratingTransformation transformation = new
MinimalDatastoreDecoratingTransformation()
+ ClassNode interfaceNode = new ClassNode(
+
'org.grails.datastore.gorm.transform.fixture.NoDecorationInterfaceTarget',
+ Modifier.PUBLIC | Modifier.INTERFACE, ClassHelper.OBJECT_TYPE)
+ AnnotationNode annotationNode = new
AnnotationNode(ClassHelper.make(CompileStatic))
+
+ when:
+ transformation.enhanceClassNode(null, annotationNode, interfaceNode)
+
+ then:
+ interfaceNode.getField('$targetDatastore') == null
+ interfaceNode.methods.empty
+ }
+}
diff --git
a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformationSpec.groovy
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformationSpec.groovy
new file mode 100644
index 0000000000..cae293280f
--- /dev/null
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AbstractMethodDecoratingTransformationSpec.groovy
@@ -0,0 +1,220 @@
+/*
+ * 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 org.grails.datastore.gorm.transform
+
+import org.springframework.util.ReflectionUtils
+
+import spock.lang.Specification
+
+/**
+ * {@code AbstractMethodDecoratingTransformation}'s method-selection logic in
{@code weaveClassNode} -
+ * which methods a decorating transform like {@code @Transactional} actually
gets applied to - is
+ * normally exercised indirectly and incompletely through whichever fixtures
the real transforms'
+ * specs happen to declare. This spec drives it directly, through a real
compilation, using
+ * {@link TestMethodDecoratingTransformation} - a pass-through decorator with
no side effects of its
+ * own - so each inclusion/exclusion branch can be asserted on independently
of any particular real
+ * transform's semantics.
+ */
+class AbstractMethodDecoratingTransformationSpec extends Specification {
+
+ private static Class<?> compile(String source) {
+ new GroovyClassLoader().parseClass(source)
+ }
+
+ void "a plain public instance method is renamed and re-dispatched to"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class PlainMethodTarget {
+ String updateFoo() { 'original' }
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(target, '$test__updateFoo') != null
+ target.getDeclaredConstructor().newInstance().updateFoo() == 'original'
+ }
+
+ void "static, private and abstract methods are never woven"() {
+ when:
+ Class<?> concrete = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class StaticAndPrivateMethodsTarget {
+ static void staticMethod() { }
+ private void privateMethod() { }
+ }
+ ''')
+ Class<?> abstractTarget = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ abstract class AbstractMethodTarget {
+ abstract void abstractMethod()
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(concrete, '$test__staticMethod') == null
+ ReflectionUtils.findMethod(concrete, '$test__privateMethod', String)
== null
+ concrete.declaredMethods.every {
!it.name.contains('$test__privateMethod') }
+ ReflectionUtils.findMethod(abstractTarget, '$test__abstractMethod') ==
null
+ }
+
+ void "METHOD_NAME_EXCLUDES keeps lifecycle method names such as
afterPropertiesSet and destroy unwoven"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class LifecycleMethodTarget {
+ void afterPropertiesSet() { }
+ void destroy() { }
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(target, '$test__afterPropertiesSet') == null
+ ReflectionUtils.findMethod(target, '$test__destroy') == null
+ }
+
+ void "setters are never woven and a getter is only woven when it has no
matching setter"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class GetterSetterTarget {
+ String name
+ String getAge() { 'ageless' }
+ }
+ ''')
+
+ then: 'the setter itself is never a weaving candidate'
+ ReflectionUtils.findMethod(target, '$test__setName', String) == null
+
+ and: 'the getter that has a matching setter is skipped'
+ ReflectionUtils.findMethod(target, '$test__getName') == null
+
+ and: 'the getter with no matching setter is woven'
+ ReflectionUtils.findMethod(target, '$test__getAge') != null
+ }
+
+ void "a dollar-prefixed method name that is not a spock feature method is
skipped"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class DollarMethodTarget {
+ void $rawMethod() { }
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(target, '$test__$rawMethod') == null
+ }
+
+ void "hasExcludedAnnotation skips methods annotated with PostConstruct"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class PostConstructTarget {
+ @jakarta.annotation.PostConstruct
+ void init() { }
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(target, '$test__init') == null
+ }
+
+ void "spock setup and cleanup are routed to weaveTestSetupMethod instead
of being renamed"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class SpockSetupCleanupTarget extends spock.lang.Specification {
+ def setup() { }
+ def cleanup() { }
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(target, '$test__setup') == null
+ ReflectionUtils.findMethod(target, '$test__cleanup') == null
+ }
+
+ void "a JUnit-annotated method is routed to weaveTestSetupMethod instead
of being renamed"() {
+ when:
+ Class<?> target = compile('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class JunitAnnotatedTarget {
+ @org.junit.jupiter.api.BeforeEach
+ void junitSetup() { }
+ }
+ ''')
+
+ then:
+ ReflectionUtils.findMethod(target, '$test__junitSetup') == null
+ }
+
+ void "an overriding method is renamed with a class-qualified prefix while
the parent's own method uses the plain prefix"() {
+ when: 'evaluating the script (rather than parsing a single class)
returns both declared types, in order'
+ List<Class<?>> types = new GroovyShell().evaluate('''
+ package org.grails.datastore.gorm.transform.fixture
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class OverrideParentTarget {
+ String sound() { 'parent' }
+ }
+
+ @org.grails.datastore.gorm.transform.ApplyTestMethodDecorating
+ class OverrideChildTarget extends OverrideParentTarget {
+ @Override
+ String sound() { 'child' }
+ }
+
+ [OverrideParentTarget, OverrideChildTarget]
+ ''') as List<Class<?>>
+ Class<?> parent = types[0]
+ Class<?> child = types[1]
+
+ then: 'the parent method - not an override - uses the plain renamed
prefix'
+ ReflectionUtils.findMethod(parent, '$test__sound') != null
+
+ and: 'the overriding method is renamed with the decapitalized
declaring class name mixed in'
+ ReflectionUtils.findMethod(child, '$test__overrideChildTarget_sound')
!= null
+
+ and: 'the child does not declare its own plain-prefixed renamed method
(only inherits the parent one)'
+ child.declaredMethods.every { it.name != '$test__sound' }
+
+ and: 'the woven methods still dispatch correctly'
+ parent.getDeclaredConstructor().newInstance().sound() == 'parent'
+ child.getDeclaredConstructor().newInstance().sound() == 'child'
+ }
+}
diff --git
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/ApplyTestMethodDecorating.java
similarity index 59%
copy from
grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
copy to
grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/ApplyTestMethodDecorating.java
index 7c7e298cdf..8166262ca6 100644
---
a/grails-datamapping-core/src/main/groovy/org/grails/datastore/gorm/transform/GormASTTransformationClass.java
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/ApplyTestMethodDecorating.java
@@ -16,6 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
+
package org.grails.datastore.gorm.transform;
import java.lang.annotation.ElementType;
@@ -23,14 +24,17 @@ import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
+import org.codehaus.groovy.transform.GroovyASTTransformationClass;
+
/**
- * U
- *
- * @author Graeme Rocher
- * @since 6.1
+ * Local marker annotation used only for testing {@link
AbstractMethodDecoratingTransformation}
+ * through a real, live compilation - triggering {@link
TestMethodDecoratingTransformation}, a
+ * pass-through decorator that renames and re-dispatches to the original
method body without
+ * wrapping it in anything (unlike the real {@code Transactional}/{@code
CurrentTenant} transforms),
+ * so the method-selection logic in {@code weaveClassNode} can be exercised in
isolation.
*/
@Retention(RetentionPolicy.RUNTIME)
-@Target(ElementType.ANNOTATION_TYPE)
-public @interface GormASTTransformationClass {
- String value();
+@Target({ElementType.TYPE})
+@GroovyASTTransformationClass("org.grails.datastore.gorm.transform.TestMethodDecoratingTransformation")
+public @interface ApplyTestMethodDecorating {
}
diff --git
a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AstMethodDispatchUtilsSpec.groovy
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AstMethodDispatchUtilsSpec.groovy
new file mode 100644
index 0000000000..1e6d73ee59
--- /dev/null
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AstMethodDispatchUtilsSpec.groovy
@@ -0,0 +1,171 @@
+/*
+ * 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 org.grails.datastore.gorm.transform
+
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.Parameter
+import org.codehaus.groovy.ast.expr.ClassExpression
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.MapExpression
+import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codehaus.groovy.ast.expr.TupleExpression
+import org.codehaus.groovy.ast.expr.VariableExpression
+import spock.lang.Specification
+
+import static org.codehaus.groovy.ast.tools.GeneralUtils.args
+import static org.codehaus.groovy.ast.tools.GeneralUtils.constX
+import static org.codehaus.groovy.ast.tools.GeneralUtils.varX
+import static org.grails.datastore.mapping.reflect.AstUtils.ZERO_ARGUMENTS
+
+/**
+ * {@code AstMethodDispatchUtils} builds AST method-call expressions and, when
it can resolve the
+ * target method on the declared type, wires the {@code MethodTarget} onto the
call so the compiler
+ * doesn't have to do dynamic dispatch. This spec proves that resolution
happens when the method
+ * genuinely exists on the target type, is left unresolved when it doesn't,
and that the small
+ * argument/parameter-building helpers (`namedArgs`, `paramsForArgs`) produce
the shapes their callers
+ * across the module (service implementers, `TenantTransform`, the
transactional transforms) rely on.
+ */
+class AstMethodDispatchUtilsSpec extends Specification {
+
+ static class Greeter {
+ String greet(String name) { "hello $name" }
+ }
+
+ void "namedArgs builds a MapExpression with one entry per named
argument"() {
+ given:
+ ConstantExpression trueExpr = ConstantExpression.TRUE
+
+ when:
+ MapExpression mapExpression =
AstMethodDispatchUtils.namedArgs(failOnError: trueExpr)
+
+ then:
+ mapExpression.mapEntryExpressions.size() == 1
+ mapExpression.mapEntryExpressions[0].keyExpression.text ==
'failOnError'
+ mapExpression.mapEntryExpressions[0].valueExpression.is(trueExpr)
+ }
+
+ void "callD(Class, var, methodName) resolves the method target when the
method exists on the target type"() {
+ when:
+ MethodCallExpression call = AstMethodDispatchUtils.callD(Greeter,
'greeter', 'greet', args(constX('World')))
+
+ then:
+ call.methodAsString == 'greet'
+ call.methodTarget != null
+ call.methodTarget.name == 'greet'
+ call.objectExpression instanceof VariableExpression
+ ((VariableExpression) call.objectExpression).name == 'greeter'
+ }
+
+ void "callD(ClassNode, var, methodName) leaves the method target unset
when the method does not exist"() {
+ given:
+ ClassNode greeterType = ClassHelper.make(Greeter)
+
+ when:
+ MethodCallExpression call = AstMethodDispatchUtils.callD(greeterType,
'greeter', 'doesNotExist')
+
+ then:
+ call.methodAsString == 'doesNotExist'
+ call.methodTarget == null
+ }
+
+ void "callD(Expression, methodName) resolves against the expression's
static type"() {
+ given:
+ VariableExpression target = varX('greeter', ClassHelper.make(Greeter))
+
+ when:
+ MethodCallExpression call = AstMethodDispatchUtils.callD(target,
'greet', args(constX('World')))
+
+ then:
+ call.methodTarget != null
+ call.methodTarget.name == 'greet'
+ }
+
+ void "callD defaults to ZERO_ARGUMENTS when no arguments are supplied"() {
+ when:
+ MethodCallExpression call = AstMethodDispatchUtils.callD(Greeter,
'greeter', 'greet')
+
+ then:
+ call.arguments.is(ZERO_ARGUMENTS)
+ }
+
+ void "callThisD(Class, methodName) builds a call on an explicit 'this' of
the given type and resolves the target"() {
+ when:
+ MethodCallExpression call = AstMethodDispatchUtils.callThisD(Greeter,
'greet', args(constX('World')))
+
+ then:
+ call.methodTarget != null
+ call.methodTarget.name == 'greet'
+ ((VariableExpression) call.objectExpression).name == 'this'
+ ((VariableExpression) call.objectExpression).type ==
ClassHelper.make(Greeter)
+ }
+
+ void "callThisD(ClassNode, methodName, arguments) leaves the method target
unset when the method does not exist"() {
+ given:
+ ClassNode greeterType = ClassHelper.make(Greeter)
+
+ when:
+ MethodCallExpression call =
AstMethodDispatchUtils.callThisD(greeterType, 'doesNotExist', ZERO_ARGUMENTS)
+
+ then:
+ call.methodTarget == null
+ }
+
+ void "paramsForArgs builds one parameter per expression in a
TupleExpression, typed from each expression"() {
+ given:
+ TupleExpression tuple = args(constX('a string'), constX(1))
+
+ when:
+ Parameter[] params = AstMethodDispatchUtils.paramsForArgs(tuple)
+
+ then:
+ params.length == 2
+ params[0].name == 'p0'
+ params[0].type == ClassHelper.STRING_TYPE
+ params[1].name == 'p1'
+ params[1].type == ClassHelper.Integer_TYPE
+ }
+
+ void "paramsForArgs treats a ClassExpression argument as typed Class, not
the referenced type"() {
+ given:
+ Expression classArg = new ClassExpression(ClassHelper.make(Greeter))
+ TupleExpression tuple = args(classArg)
+
+ when:
+ Parameter[] params = AstMethodDispatchUtils.paramsForArgs(tuple)
+
+ then:
+ params.length == 1
+ params[0].type == ClassHelper.CLASS_Type
+ }
+
+ void "paramsForArgs builds a single parameter for a bare (non-tuple)
expression"() {
+ given:
+ Expression singleArg = constX('solo')
+
+ when:
+ Parameter[] params = AstMethodDispatchUtils.paramsForArgs(singleArg)
+
+ then:
+ params.length == 1
+ params[0].name == 'p'
+ params[0].type == ClassHelper.STRING_TYPE
+ }
+}
diff --git
a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtilsSpec.groovy
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtilsSpec.groovy
new file mode 100644
index 0000000000..fd9fa3ddaa
--- /dev/null
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/AstPropertyResolveUtilsSpec.groovy
@@ -0,0 +1,290 @@
+/*
+ * 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 org.grails.datastore.gorm.transform
+
+import java.lang.reflect.Modifier
+import java.util.concurrent.Callable
+import java.util.concurrent.CyclicBarrier
+import java.util.concurrent.ExecutorService
+import java.util.concurrent.Executors
+import java.util.concurrent.Future
+import java.util.concurrent.TimeUnit
+
+import org.codehaus.groovy.ast.AnnotationNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.expr.ClassExpression
+import org.codehaus.groovy.ast.expr.ConstantExpression
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.MapExpression
+import spock.lang.Specification
+
+import grails.gorm.annotation.Entity
+import org.grails.datastore.mapping.model.config.GormProperties
+
+/**
+ * {@link AstPropertyResolveUtils} caches resolved property metadata as
metadata on the
+ * {@link ClassNode} it describes (see that class's javadoc). Two distinct
compilations (e.g. the
+ * same source parsed in two different {@code GroovyClassLoader}s, as happens
for
+ * dynamically-generated sources and in tests) produce distinct {@code
ClassNode} instances that
+ * can legitimately share the exact same name - {@code
ClassNode#equals(Object)} compares by name,
+ * so a naive name- or equals()-based cache key would conflate them,
corrupting the resolved
+ * properties of one class with those of an unrelated class that happens to
share its name. This
+ * spec proves the cache is scoped strictly per {@code ClassNode} instance, so
same-named-but-distinct
+ * class nodes never contaminate each other's cached property data, that
domain-class-specific
+ * resolution (identity/version injection, association metadata) works, and
that concurrent
+ * resolution of distinct nodes is safe.
+ */
+class AstPropertyResolveUtilsSpec extends Specification {
+
+ void "property lookups for two same-named ClassNodes in different packages
do not corrupt each other"() {
+ given: 'two distinct ClassNodes with the same simple name declared in
different packages'
+ ClassNode first = new ClassNode('org.example.one.Widget',
Modifier.PUBLIC, ClassHelper.OBJECT_TYPE)
+ first.addProperty('color', Modifier.PUBLIC, ClassHelper.STRING_TYPE,
null, null, null)
+
+ ClassNode second = new ClassNode('org.example.two.Widget',
Modifier.PUBLIC, ClassHelper.OBJECT_TYPE)
+ second.addProperty('weight', Modifier.PUBLIC,
ClassHelper.Integer_TYPE, null, null, null)
+
+ when: 'the first class node is resolved, populating its cache entry'
+ List<String> firstProperties =
AstPropertyResolveUtils.getPropertyNames(first)
+
+ then: 'only its own property is resolved'
+ firstProperties.contains('color')
+ !firstProperties.contains('weight')
+
+ when: 'the second, differently-packaged, same-simple-name class node
is resolved'
+ List<String> secondProperties =
AstPropertyResolveUtils.getPropertyNames(second)
+
+ then: 'its own property is resolved, not leaked from the first class
node'
+ secondProperties.contains('weight')
+ !secondProperties.contains('color')
+
+ and: 'the first class node cache entry remains unaffected by resolving
the second'
+ List<String> firstPropertiesAfter =
AstPropertyResolveUtils.getPropertyNames(first)
+ firstPropertiesAfter.contains('color')
+ !firstPropertiesAfter.contains('weight')
+ }
+
+ void "property lookups for two distinct ClassNode instances with the exact
same unqualified name do not corrupt each other"() {
+ given: 'two distinct ClassNode instances - as produced by two separate
compilations - sharing an identical unqualified name'
+ ClassNode first = new ClassNode('Widget', Modifier.PUBLIC,
ClassHelper.OBJECT_TYPE)
+ first.addProperty('color', Modifier.PUBLIC, ClassHelper.STRING_TYPE,
null, null, null)
+
+ ClassNode second = new ClassNode('Widget', Modifier.PUBLIC,
ClassHelper.OBJECT_TYPE)
+ second.addProperty('weight', Modifier.PUBLIC,
ClassHelper.Integer_TYPE, null, null, null)
+
+ and: 'they are genuinely different instances - the precondition a
name- or equals()-keyed cache would get wrong'
+ // ClassNode#equals()/hashCode() compare by getText() (essentially the
class name), so
+ // first == second and first.hashCode() == second.hashCode() both hold
here even though
+ // these are two unrelated ClassNode instances with different declared
properties. A cache
+ // keyed by name or by equals()/hashCode() would treat them as the
same entry; only
+ // reference identity (!first.is(second)) tells them apart, which is
exactly what the
+ // cache must key on. This is an "and:" continuing "given:", so Spock
does not apply an
+ // implicit condition here - the explicit assert is required for this
to actually fail
+ // the test if it were ever untrue.
+ assert !first.is(second)
+
+ when: 'both class nodes are resolved'
+ List<String> firstProperties =
AstPropertyResolveUtils.getPropertyNames(first)
+ List<String> secondProperties =
AstPropertyResolveUtils.getPropertyNames(second)
+
+ then: 'each keeps its own, independently-resolved properties despite
comparing equal'
+ firstProperties.contains('color')
+ !firstProperties.contains('weight')
+ secondProperties.contains('weight')
+ !secondProperties.contains('color')
+ }
+
+ void "getPropertyType resolves the type of a declared property"() {
+ given: 'a class node with a declared property'
+ ClassNode classNode = new ClassNode('org.example.PropertyTypeWidget',
Modifier.PUBLIC, ClassHelper.OBJECT_TYPE)
+ classNode.addProperty('label', Modifier.PUBLIC,
ClassHelper.STRING_TYPE, null, null, null)
+
+ expect: 'the resolved property type matches the declared type, both on
first and second lookup'
+ AstPropertyResolveUtils.getPropertyType(classNode, 'label') ==
ClassHelper.STRING_TYPE
+ AstPropertyResolveUtils.getPropertyType(classNode, 'label') ==
ClassHelper.STRING_TYPE
+ }
+
+ void "getPropertyNames returns the snapshot taken on first lookup rather
than reflecting properties added afterwards"() {
+ given: 'a class node with one declared property'
+ ClassNode classNode = new
ClassNode('org.example.CachedSnapshotWidget', Modifier.PUBLIC,
ClassHelper.OBJECT_TYPE)
+ classNode.addProperty('label', Modifier.PUBLIC,
ClassHelper.STRING_TYPE, null, null, null)
+
+ when: 'the property names are resolved once, populating the cache'
+ List<String> firstLookup =
AstPropertyResolveUtils.getPropertyNames(classNode)
+
+ then:
+ firstLookup == ['label']
+
+ when: 'a second property is added directly to the ClassNode after the
cache has already been populated'
+ classNode.addProperty('extra', Modifier.PUBLIC,
ClassHelper.Integer_TYPE, null, null, null)
+
+ then: 'a direct lookup on the ClassNode confirms the property really
was added - so the cache below is stale, not simply broken'
+ classNode.getProperty('extra') != null
+
+ and: 'getPropertyNames still returns the cached snapshot from the
first lookup, proving the result was actually cached rather than recomputed on
every call'
+ !AstPropertyResolveUtils.getPropertyNames(classNode).contains('extra')
+ }
+
+ void "getPropertyNames injects identity and version for a domain class and
resolves hasMany/belongsTo/hasOne declared via AST initial expressions"() {
+ given: 'a domain class node declaring hasMany/belongsTo/hasOne as
property initial expressions, as "static hasMany = [...]" compiles to'
+ ClassNode associatedType = new
ClassNode('org.example.AssociatedThing', Modifier.PUBLIC,
ClassHelper.OBJECT_TYPE)
+ ClassNode classNode = new ClassNode('org.example.AstDrivenDomain',
Modifier.PUBLIC, ClassHelper.OBJECT_TYPE)
+ classNode.addAnnotation(new AnnotationNode(ClassHelper.make(Entity)))
+ classNode.addProperty('title', Modifier.PUBLIC,
ClassHelper.STRING_TYPE, null, null, null)
+ classNode.addProperty(GormProperties.HAS_MANY, Modifier.PUBLIC |
Modifier.STATIC, ClassHelper.MAP_TYPE.getPlainNodeReference(),
+ mapExpressionOf('things', associatedType), null, null)
+
+ when:
+ List<String> propertyNames =
AstPropertyResolveUtils.getPropertyNames(classNode)
+
+ then: 'the AST-declared property is present alongside the injected
identity/version properties'
+ propertyNames.containsAll(['title', GormProperties.IDENTITY,
GormProperties.VERSION, 'things'])
+ AstPropertyResolveUtils.getPropertyType(classNode,
GormProperties.IDENTITY) == new ClassNode(Long.class)
+ AstPropertyResolveUtils.getPropertyType(classNode, 'things') ==
associatedType
+
+ and: 'the raw hasMany/belongsTo/hasOne map property itself is not
exposed as a plain property'
+ !propertyNames.contains(GormProperties.HAS_MANY)
+ }
+
+ void "getPropertyNames resolves hasMany/belongsTo/hasOne association
metadata via reflection once the domain class is fully resolved"() {
+ given: 'a real, already-compiled domain class with
hasMany/belongsTo/hasOne associations'
+ GroovyClassLoader gcl = new GroovyClassLoader()
+ gcl.parseClass('''
+ import grails.gorm.annotation.Entity
+
+ @Entity
+ class ReflectedAssociationAuthor {
+ String name
+ }
+
+ @Entity
+ class ReflectedAssociationBook {
+ String title
+ }
+
+ @Entity
+ class ReflectedAssociationPublisher {
+ String company
+ }
+
+ @Entity
+ class ReflectedAssociationFixture {
+ static hasMany = [books: ReflectedAssociationBook]
+ static belongsTo = [author: ReflectedAssociationAuthor]
+ static hasOne = [publisher: ReflectedAssociationPublisher]
+ }
+ ''')
+ Class<?> domainClass = gcl.loadedClasses.find { it.simpleName ==
'ReflectedAssociationFixture' }
+
+ and: 'a fresh ClassNode built from the already-compiled class, as
happens once compilation has finished'
+ ClassNode resolvedNode = ClassHelper.make(domainClass)
+
+ expect: 'the node reports itself resolved, which is what gates the
reflection-based association lookup'
+ resolvedNode.isResolved()
+
+ when:
+ List<String> propertyNames =
AstPropertyResolveUtils.getPropertyNames(resolvedNode)
+
+ then: 'the reflected association properties are present alongside the
injected identity/version properties'
+ propertyNames.containsAll([GormProperties.IDENTITY,
GormProperties.VERSION, 'books', 'author', 'publisher'])
+
+ cleanup:
+ gcl.close()
+ }
+
+ void "concurrent resolution of distinct, identically-named ClassNode
instances never corrupts each other's cached properties"() {
+ given: 'many threads, each building and resolving its own distinct
ClassNode sharing one common name'
+ int threadCount = 20
+ ExecutorService executor = Executors.newFixedThreadPool(threadCount)
+ CyclicBarrier barrier = new CyclicBarrier(threadCount)
+
+ when: 'all threads race to populate the cache for their own instance
at the same time'
+ List<Future<Boolean>> futures = (0..<threadCount).collect { int i ->
+ executor.submit({ ->
+ barrier.await(30, TimeUnit.SECONDS)
+ ClassNode node = new ClassNode('ConcurrentWidget',
Modifier.PUBLIC, ClassHelper.OBJECT_TYPE)
+ String propertyName = "prop${i}".toString()
+ node.addProperty(propertyName, Modifier.PUBLIC,
ClassHelper.STRING_TYPE, null, null, null)
+
+ List<String> names =
AstPropertyResolveUtils.getPropertyNames(node)
+ names.contains(propertyName) && names.count {
it.startsWith('prop') } == 1
+ } as Callable<Boolean>)
+ }
+ List<Boolean> outcomes = futures.collect { Future<Boolean> future ->
future.get(30, TimeUnit.SECONDS) }
+
+ then: 'every thread resolved its own property set, uncontaminated by
any of the other concurrently-resolved same-named instances'
+ outcomes.every { it }
+
+ cleanup:
+ executor.shutdownNow()
+ }
+
+ void "concurrent resolution of the exact same shared ClassNode instance
from many threads is safe"() {
+ // Distinct-instance concurrency (the test above) can never exercise a
race on the
+ // underlying node-metadata storage, because nothing is shared between
the threads. A single
+ // ClassNode instance genuinely can be looked up from more than one
thread at once in
+ // practice - e.g. ClassHelper.OBJECT_TYPE/STRING_TYPE are JVM-wide
singletons that this
+ // utility's callers can resolve to for a plain Object- or def-typed
property, so two
+ // unrelated, concurrently-running compilations could both reach this
cache for the exact
+ // same node. This test exercises that shared-node case directly and
asserts every thread's
+ // returned value is correct.
+ //
+ // Note on what this test can and can't prove: the cached computation
here is deterministic
+ // and idempotent, so even with the "synchronized
(cachedClassProperties)" guard in
+ // AstPropertyResolveUtils#getPropertiesFromCache removed, every
thread still computes and
+ // returns the same correct value in practice - a black-box test of
returned values cannot
+ // reliably force ClassNode's underlying,
explicitly-documented-not-thread-safe metadata
+ // storage into an observably-wrong state without reaching into Groovy
internals neither this
+ // spec nor AstPropertyResolveUtils controls. Verified by temporarily
removing that guard and
+ // running this test 8 times without a failure. The synchronization is
kept as a correctness
+ // fix justified by IdentityHashMap's own documentation (it is not
synchronized and concurrent
+ // structural modification can corrupt it), not because this test can
demonstrate its absence
+ // breaking anything; this test instead guards against a regression to
something observably
+ // broken (an exception, a null, a wrong/partial result) under real
concurrent load, which is
+ // the failure mode a future refactor could plausibly introduce.
+ given: 'one ClassNode instance that every thread will resolve
concurrently'
+ ClassNode sharedNode = new ClassNode('SharedWidget', Modifier.PUBLIC,
ClassHelper.OBJECT_TYPE)
+ sharedNode.addProperty('label', Modifier.PUBLIC,
ClassHelper.STRING_TYPE, null, null, null)
+ int threadCount = 32
+ ExecutorService executor = Executors.newFixedThreadPool(threadCount)
+ CyclicBarrier barrier = new CyclicBarrier(threadCount)
+
+ when: 'all threads race to resolve properties for the same instance at
once'
+ List<Future<List<String>>> futures = (0..<threadCount).collect {
+ executor.submit({ ->
+ barrier.await(30, TimeUnit.SECONDS)
+ AstPropertyResolveUtils.getPropertyNames(sharedNode)
+ } as Callable<List<String>>)
+ }
+ List<List<String>> results = futures.collect { Future<List<String>>
future -> future.get(30, TimeUnit.SECONDS) }
+
+ then: 'every thread observes the same, fully and correctly populated
result - none sees a partial or corrupted map'
+ results.every { it == ['label'] }
+
+ cleanup:
+ executor.shutdownNow()
+ }
+
+ private static Expression mapExpressionOf(String key, ClassNode valueType)
{
+ MapExpression mapExpression = new MapExpression()
+ mapExpression.addMapEntryExpression(new ConstantExpression(key), new
ClassExpression(valueType))
+ return mapExpression
+ }
+}
diff --git
a/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/TestMethodDecoratingTransformation.groovy
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/TestMethodDecoratingTransformation.groovy
new file mode 100644
index 0000000000..4e69f30003
--- /dev/null
+++
b/grails-datamapping-core/src/test/groovy/org/grails/datastore/gorm/transform/TestMethodDecoratingTransformation.groovy
@@ -0,0 +1,72 @@
+/*
+ * 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 org.grails.datastore.gorm.transform
+
+import org.codehaus.groovy.ast.AnnotationNode
+import org.codehaus.groovy.ast.ClassHelper
+import org.codehaus.groovy.ast.ClassNode
+import org.codehaus.groovy.ast.MethodNode
+import org.codehaus.groovy.ast.expr.Expression
+import org.codehaus.groovy.ast.expr.MethodCallExpression
+import org.codehaus.groovy.ast.stmt.BlockStatement
+import org.codehaus.groovy.control.CompilePhase
+import org.codehaus.groovy.control.SourceUnit
+import org.codehaus.groovy.transform.ASTTransformation
+import org.codehaus.groovy.transform.GroovyASTTransformation
+
+/**
+ * Local, annotation-driven transformation used only to test {@link
AbstractMethodDecoratingTransformation}
+ * through a genuine compilation. It simply renames the decorated method and
dispatches straight to
+ * it - no wrapping closure, no transaction/tenant semantics - so tests can
assert purely on which
+ * methods {@code weaveClassNode} chose to decorate.
+ *
+ * @see AbstractMethodDecoratingTransformationSpec
+ */
+@GroovyASTTransformation(phase = CompilePhase.CANONICALIZATION)
+class TestMethodDecoratingTransformation extends
AbstractMethodDecoratingTransformation implements ASTTransformation {
+
+ private static final ClassNode MY_TYPE =
ClassHelper.make(ApplyTestMethodDecorating)
+ private static final Object APPLIED_MARKER = new Object()
+
+ @Override
+ protected ClassNode getAnnotationType() {
+ MY_TYPE
+ }
+
+ @Override
+ protected Object getAppliedMarker() {
+ APPLIED_MARKER
+ }
+
+ @Override
+ protected String getRenamedMethodPrefix() {
+ '$test__'
+ }
+
+ @Override
+ protected Expression buildDelegatingMethodCall(SourceUnit sourceUnit,
AnnotationNode annotationNode, ClassNode classNode,
+ MethodNode methodNode,
MethodCallExpression originalMethodCall, BlockStatement newMethodBody) {
+ originalMethodCall
+ }
+
+ @Override
+ int priority() {
+ 0
+ }
+}
