Change how command objects are initialized

jeffscottbrown · jeffscottbrown · commit 3827c7ae2aef · 2016-02-25T16:17:17.000-06:00
This change will cause the compiler to turn this: class SomeController { @transactional def someAction(SomeDomain sd) { // application code here } } Into this: class SomeController { def someAction() { someAction(null) } def someAction(SomeDomain sd) { withTransaction { status -> $tt__someAction(sd, status) } } def $tt_someAction(SomeDomain sd, TransactionStatus status) { if(sd == null) { sd = // initialize sd } // original application code here } } The initialization code has been moved into the transaction handling method and the null check is there to accommodate unit tests which explicitly pass a parameter in to the action method. This is related to apache/grails-data-mapping#580.
diff --git a/grails-core/src/main/groovy/org/codehaus/groovy/grails/compiler/injection/GrailsASTUtils.java b/grails-core/src/main/groovy/org/codehaus/groovy/grails/compiler/injection/GrailsASTUtils.java
@@ -873,6 +873,16 @@ public static boolean hasAnnotation(final ClassNode classNode, final Class<? ext
         return !classNode.getAnnotations(new ClassNode(annotationClass)).isEmpty();
     }
 
+    /**
+     * Returns true if classNode is marked with annotationClass
+     * @param methodNode A MethodNode to inspect
+     * @param annotationClass an annotation to look for
+     * @return true if methodNode is marked with annotationClass, otherwise false
+     */
+    public static boolean hasAnnotation(final MethodNode methodNode, final Class<? extends Annotation> annotationClass) {
+        return !methodNode.getAnnotations(new ClassNode(annotationClass)).isEmpty();
+    }
+
     /**
      * @param classNode a ClassNode to search
      * @param annotationsToLookFor Annotations to look for
diff --git a/grails-core/src/main/groovy/org/codehaus/groovy/grails/transaction/transform/TransactionalTransform.groovy b/grails-core/src/main/groovy/org/codehaus/groovy/grails/transaction/transform/TransactionalTransform.groovy
@@ -259,7 +259,7 @@ class TransactionalTransform implements ASTTransformation{
     }
 
     protected MethodCallExpression moveOriginalCodeToNewMethod(SourceUnit source, ClassNode classNode, MethodNode methodNode) {
-        String renamedMethodName = '$tt__' + methodNode.getName()
+        String renamedMethodName = getTransactionHandlingMethodName(methodNode)
         final transactionStatusParameter = new Parameter(ClassHelper.make(TransactionStatus), "transactionStatus")
         def newParameters = methodNode.getParameters() ? (copyParameters(((methodNode.getParameters() as List) + [transactionStatusParameter]) as Parameter[])) : [transactionStatusParameter] as Parameter[]
 
@@ -289,6 +289,10 @@ class TransactionalTransform implements ASTTransformation{
         originalMethodCall
     }
 
+    static String getTransactionHandlingMethodName(MethodNode methodNode) {
+        '$tt__' + methodNode.getName()
+    }
+
     protected void weaveTransactionManagerAware(SourceUnit source, ClassNode declaringClassNode) {
         ClassNode transactionManagerAwareInterface = ClassHelper.make(TransactionManagerAware)
 
diff --git a/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/compiler/web/ControllerActionTransformer.java b/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/compiler/web/ControllerActionTransformer.java
@@ -21,6 +21,7 @@
 import static org.codehaus.groovy.grails.compiler.injection.GrailsASTUtils.buildGetPropertyExpression;
 import static org.codehaus.groovy.grails.compiler.injection.GrailsASTUtils.buildSetPropertyExpression;
 import grails.artefact.Artefact;
+import grails.transaction.Transactional;
 import grails.util.BuildSettings;
 import grails.util.CollectionUtils;
 import grails.validation.ASTValidateableHelper;
@@ -86,12 +87,14 @@
 import org.codehaus.groovy.grails.compiler.injection.AstTransformer;
 import org.codehaus.groovy.grails.compiler.injection.GrailsASTUtils;
 import org.codehaus.groovy.grails.compiler.injection.GrailsArtefactClassInjector;
+import org.codehaus.groovy.grails.transaction.transform.TransactionalTransform;
 import org.codehaus.groovy.grails.web.binding.DefaultASTDatabindingHelper;
 import org.codehaus.groovy.grails.web.controllers.DefaultControllerExceptionHandlerMetaData;
 import org.codehaus.groovy.grails.web.util.TypeConvertingMap;
 import org.codehaus.groovy.runtime.DefaultGroovyMethods;
 import org.codehaus.groovy.syntax.Token;
 import org.codehaus.groovy.syntax.Types;
+import org.springframework.transaction.TransactionStatus;
 import org.springframework.validation.Errors;
 import org.springframework.validation.MapBindingResult;
 
@@ -357,6 +360,7 @@ private MethodNode convertToMethodAction(ClassNode classNode, MethodNode methodN
 
         MethodNode method = null;
         if (methodNode.getParameters().length > 0) {
+            populateMethodWithCommandObjectInitializationCode(classNode, methodNode, source, context);
             final BlockStatement methodCode = new BlockStatement();
             
             final BlockStatement codeToHandleAllowedMethods = getCodeToHandleAllowedMethods(classNode, methodNode.getName());
@@ -385,6 +389,54 @@ private MethodNode convertToMethodAction(ClassNode classNode, MethodNode methodN
         return method;
     }
 
+    protected void populateMethodWithCommandObjectInitializationCode(final ClassNode classNode,
+                                                                     final MethodNode methodNode,
+                                                                     final SourceUnit source,
+                                                                     final GeneratorContext context) {
+        final MethodNode methodToUse = getMethodToIncludeCommandObjectInitializationCode(methodNode);
+        final BlockStatement newCodeForExistingMethod = new BlockStatement();
+        final BlockStatement codeToInitializeCommandObjects = getCodeToInitializeCommandObjects(methodToUse, methodToUse.getName(), methodToUse.getParameters(), classNode, source, context);
+        newCodeForExistingMethod.addStatement(codeToInitializeCommandObjects);
+        newCodeForExistingMethod.addStatement(methodToUse.getCode());
+        methodToUse.setCode(newCodeForExistingMethod);
+    }
+
+    protected MethodNode getMethodToIncludeCommandObjectInitializationCode(final MethodNode methodNode) {
+        MethodNode node = methodNode;
+        if(GrailsASTUtils.hasAnnotation(methodNode, Transactional.class)) {
+            final ClassNode declaringClass = methodNode.getDeclaringClass();
+            final String txHandlingMethodName = TransactionalTransform.getTransactionHandlingMethodName(methodNode);
+            final Parameter[] originalMethodParameters = methodNode.getParameters();
+            final Parameter[] txHandlingMethodParams = new Parameter[originalMethodParameters.length + 1];
+            System.arraycopy(originalMethodParameters, 0, txHandlingMethodParams, 0, originalMethodParameters.length);
+            txHandlingMethodParams[txHandlingMethodParams.length-1] = new Parameter(ClassHelper.make(TransactionStatus.class), "transactionStatus");
+            if(declaringClass.hasMethod(txHandlingMethodName, txHandlingMethodParams)) {
+                node = declaringClass.getMethod(txHandlingMethodName, txHandlingMethodParams);
+            }
+        }
+        return node;
+    }
+
+    protected BlockStatement getCodeToInitializeCommandObjects(final ASTNode actionNode,
+                                                               final String actionName,
+                                                               final Parameter[] parameters,
+                                                               final ClassNode controllerNode,
+                                                               final SourceUnit source,
+                                                               final GeneratorContext context) {
+        final BlockStatement body = new BlockStatement();
+        for(Parameter p : parameters) {
+            final String paramName = p.getName();
+            final ClassNode paramTypeClassNode = p.getType();
+            if (!(PRIMITIVE_CLASS_NODES.contains(paramTypeClassNode) ||
+                    TYPE_WRAPPER_CLASS_TO_CONVERSION_METHOD_NAME.containsKey(paramTypeClassNode)) &&
+            !paramTypeClassNode.equals(new ClassNode(String.class)) &&
+            !paramTypeClassNode.equals(OBJECT_CLASS)) {
+                initializeAndValidateCommandObjectParameter(body, controllerNode, paramTypeClassNode, actionNode, actionName, paramName, source, context);
+            }
+        }
+        return body;
+    }
+
     private Statement addOriginalMethodCall(MethodNode methodNode, BlockStatement blockStatement) {
 
         if (blockStatement == null) {
@@ -438,7 +490,16 @@ protected void addMethodToInvokeClosure(ClassNode controllerClassNode,
 
         MethodNode method = controllerClassNode.getMethod(closureProperty.getName(), ZERO_PARAMETERS);
         if (method == null || !method.getDeclaringClass().equals(controllerClassNode)) {
-            ClosureExpression closureExpression = (ClosureExpression) closureProperty.getInitialExpression();
+            final ClosureExpression closureExpression = (ClosureExpression) closureProperty.getInitialExpression();
+            final BlockStatement newClosureCode = new BlockStatement();
+            final Statement originalClosureCode = closureExpression.getCode();
+            final BlockStatement codeToInitializeCommandObjects = getCodeToInitializeCommandObjects(closureProperty, closureProperty.getName(), closureExpression.getParameters(), controllerClassNode, source, context);
+
+            newClosureCode.addStatement(codeToInitializeCommandObjects);
+            newClosureCode.addStatement(originalClosureCode);
+
+            closureExpression.setCode(newClosureCode);
+
             final Parameter[] parameters = closureExpression.getParameters();
             final BlockStatement newMethodCode = initializeActionParameters(
                     controllerClassNode, closureProperty, closureProperty.getName(),
@@ -705,18 +766,16 @@ protected void initializeMethodParameter(final ClassNode classNode, final BlockS
         } else if (paramTypeClassNode.equals(new ClassNode(String.class))) {
             initializeStringParameter(classNode, wrapper, param, requestParameterName);
         } else if (!paramTypeClassNode.equals(OBJECT_CLASS)) {
-            initializeAndValidateCommandObjectParameter(wrapper, classNode, paramTypeClassNode,
-                    actionNode, actionName, paramName, source, context);
+            final DeclarationExpression declareCoExpression = new DeclarationExpression(
+                    new VariableExpression(paramName, paramTypeClassNode), Token.newSymbol(Types.EQUALS, 0, 0), new ConstantExpression(null));
+            wrapper.addStatement(new ExpressionStatement(declareCoExpression));
         }
     }
 
     protected void initializeAndValidateCommandObjectParameter(final BlockStatement wrapper,
             final ClassNode controllerNode, final ClassNode commandObjectNode,
             final ASTNode actionNode, final String actionName, final String paramName,
             final SourceUnit source, final GeneratorContext context) {
-        final DeclarationExpression declareCoExpression = new DeclarationExpression(
-                new VariableExpression(paramName, commandObjectNode), Token.newSymbol(Types.EQUALS, 0, 0), new EmptyExpression());
-        wrapper.addStatement(new ExpressionStatement(declareCoExpression));
 
         if(commandObjectNode.isInterface() || Modifier.isAbstract(commandObjectNode.getModifiers())) {
             final String warningMessage = "The [" + actionName + "] action in [" +
@@ -754,10 +813,12 @@ protected void initializeAndValidateCommandObjectParameter(final BlockStatement
             if (argumentIsValidateable) {
                 final MethodCallExpression validateMethodCallExpression =
                         new MethodCallExpression(new VariableExpression(paramName), "validate", EMPTY_TUPLE);
+
                 final MethodNode validateMethod =
                         commandObjectNode.getMethod("validate", new Parameter[0]);
                 if (validateMethod != null) {
                     validateMethodCallExpression.setMethodTarget(validateMethod);
+                    validateMethodCallExpression.setImplicitThis(false);
                 }
                 final Statement ifCommandObjectIsNotNullThenValidate = new IfStatement(new BooleanExpression(new VariableExpression(paramName)), new ExpressionStatement(validateMethodCallExpression), new ExpressionStatement(new EmptyExpression()));
                 wrapper.addStatement(ifCommandObjectIsNotNullThenValidate);
@@ -804,8 +865,10 @@ protected void initializeCommandObjectParameter(final BlockStatement wrapper,
         applyDefaultMethodTarget(initializeCommandObjectMethodCall, commandObjectNode);
         
         final Expression assignCommandObjectToParameter = new BinaryExpression(new VariableExpression(paramName), Token.newSymbol(Types.EQUALS, 0, 0), initializeCommandObjectMethodCall);
-        
-        wrapper.addStatement(new ExpressionStatement(assignCommandObjectToParameter));
+
+        final BinaryExpression isParamNullExression = new BinaryExpression(new VariableExpression(paramName), Token.newSymbol(Types.COMPARE_EQUAL, 0, 0), new ConstantExpression(null));
+        final Statement initializeCommandObjectIfNull = new IfStatement(new BooleanExpression(isParamNullExression), new ExpressionStatement(assignCommandObjectToParameter), new EmptyStatement());
+        wrapper.addStatement(initializeCommandObjectIfNull);
     }
 
     /**
