From 79368055959b586fa822250af4870a90020eb193 Mon Sep 17 00:00:00 2001 From: Jeff Scott Brown Date: Wed, 16 Jul 2014 06:23:16 -0500 Subject: [PATCH 1/2] Fix eclipse configuration to be compatible with Gradle 2 --- grails-web-jsp/build.gradle | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/grails-web-jsp/build.gradle b/grails-web-jsp/build.gradle index 911f599ef98..fa547661824 100644 --- a/grails-web-jsp/build.gradle +++ b/grails-web-jsp/build.gradle @@ -29,7 +29,7 @@ jar { eclipse { classpath { - plusConfigurations += configurations.jsp21 + plusConfigurations += [configurations.jsp21] file { whenMerged { classpath -> // move jsp-api-2.1 to the last one in entries so that it appears after -2.0 From 5dd77fdb393d344718ffabf562edb096b2db0d0d Mon Sep 17 00:00:00 2001 From: Jeff Scott Brown Date: Wed, 16 Jul 2014 07:53:42 -0500 Subject: [PATCH 2/2] GRAILS-11585 - improve allowedMethods for command object actions Functional tests at https://github.com/grails/grails-functional-tests/commit/9f353a07f6458f4f8318bbda9d1369df2140f0e2 There are 2 parts to this. When a command object is a domain object and an error occurs retrieving the instance from the data store, instead of throwing that exception the framework should catch the exception, add an error to the controller and invoke the action with a null value for the command object. When a controller action accepts a command object argument the compiler creates a no-arg version of the method which creates an instance of the command object, initializes it and passes it as an argument to the method which accepts the command object. The allowedMethods handling code was being added to the method which accepts the argument. This means that the allowedMethods check is happening after the command object is initialized. This commit changes that so the allowedMethods check is added to the no-arg method and is done before the command object is initialialized. The allowedMethods checking code is still in the method which accepts the argument as well, which allows unit tests which explicitly pass a command object argument to still take advantage of allowedMethods checks. The code which imposes the allowedMethods check sets a request attribute indicating that the work has been done so when the no-arg method invokes the method which accepts the argument the check is actually only done once. With this change, the following unit tests will all pass. A controller which accepts a domain class command object: // grails-app/controllers/grails11585/RegisterController.groovy package grails11585 class RegisterController { static allowedMethods = [register: 'POST'] // Person is a domain class def register(Person p) { if(p) { render "Name: ${p.name}" } else { render 'p is null' } } } A unit test: // test/unit/grails11585/RegisterControllerSpec.groovy package grails11585 import grails.test.mixin.TestFor import spock.lang.Specification import grails.test.mixin.hibernate.HibernateTestMixin import grails.test.mixin.gorm.Domain @TestFor(RegisterController) @Domain(Person) @TestMixin(HibernateTestMixin) class RegisterControllerSpec extends Specification { void "test register with valid request method"() { when: 'the id is of the wrong type and the request method is valid' params.id = 'bogus' params.name = 'Zack' request.method = 'POST' controller.register() then: 'the response code is 200' response.status == 200 and: 'the command object is null' response.contentAsString == 'p is null' and: 'an error is added to the controller' controller.hasErrors() controller.errors.errorCount == 1 controller.errors.allErrors[0].code == 'grails11585.RegisterController.commandObject.p.error' controller.errors.allErrors[0].defaultMessage == 'Provided id of the wrong type for class grails11585.Person. Expected: class java.lang.Long, got class java.lang.String; nested exception is org.hibernate.TypeMismatchException: Provided id of the wrong type for class grails11585.Person. Expected: class java.lang.Long, got class java.lang.String' } void "test register with invalid request method"() { when: 'the id is of the wrong type and the request method is invalid' params.id = 'bogus' params.name = 'Zack' request.method = 'GET' controller.register() then: 'the response code is 405' response.status == 405 } void "test register with explicit command object argument and a valid request method"() { when: 'request method is valid' request.method = 'POST' controller.register(new Person(name: 'Zack')) then: 'the response code is 200' response.status == 200 and: 'the response content is correct' response.contentAsString == 'Name: Zack' } void "test register with explicit command object argument and invalid request method"() { when: 'request method is invalid' request.method = 'GET' controller.register(new Person(name: 'Zack')) then: 'the response code is 405' response.status == 405 } } Note that this unit test is using @HibernateTestMixin because the default in memory GORM implementation does not throw an exception for type mismatch of the id. --- .../web/ControllerActionTransformer.java | 22 +++++++++++++------ .../plugins/web/api/ControllersApi.java | 9 +++++++- 2 files changed, 23 insertions(+), 8 deletions(-) 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 index a38b5517774..4d943be0aec 100644 --- 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 @@ -359,13 +359,22 @@ private MethodNode convertToMethodAction(ClassNode classNode, MethodNode methodN MethodNode method = null; if (methodNode.getParameters().length > 0) { + final BlockStatement methodCode = new BlockStatement(); + + final BlockStatement codeToHandleAllowedMethods = getCodeToHandleAllowedMethods(classNode, methodNode.getName()); + final Statement codeToCallOriginalMethod = addOriginalMethodCall(methodNode, initializeActionParameters( + classNode, methodNode, methodNode.getName(), parameters, source, context)); + + methodCode.addStatement(codeToHandleAllowedMethods); + methodCode.addStatement(codeToCallOriginalMethod); + method = new MethodNode( methodNode.getName(), Modifier.PUBLIC, returnType, ZERO_PARAMETERS, EMPTY_CLASS_ARRAY, - addOriginalMethodCall(methodNode, initializeActionParameters( - classNode, methodNode, methodNode.getName(), parameters, source, context))); + methodCode); + GrailsASTUtils.copyAnnotations(methodNode, method); annotateActionMethod(classNode, parameters, method); } else { @@ -469,7 +478,7 @@ protected void annotateActionMethod(ClassNode controllerClassNode, final Paramet } } - protected BlockStatement getCodeToHandleAllowedMethods(ClassNode controllerClass, MethodNode methodNode) { + protected BlockStatement getCodeToHandleAllowedMethods(ClassNode controllerClass, String methodName) { GrailsASTUtils.addEnhancedAnnotation(controllerClass, DefaultGrailsControllerClass.ALLOWED_HTTP_METHODS_PROPERTY); final BlockStatement checkAllowedMethodsBlock = new BlockStatement(); @@ -483,7 +492,6 @@ protected BlockStatement getCodeToHandleAllowedMethods(ClassNode controllerClass final List allowedMethodNames = new ArrayList(); final MapExpression allowedMethodsMapExpression = (MapExpression) initialAllowedMethodsExpression; final List allowedMethodsMapEntryExpressions = allowedMethodsMapExpression.getMapEntryExpressions(); - final String methodName = methodNode.getName(); for(MapEntryExpression allowedMethodsMapEntryExpression : allowedMethodsMapEntryExpressions) { final Expression allowedMethodsMapEntryKeyExpression = allowedMethodsMapEntryExpression.getKeyExpression(); if(allowedMethodsMapEntryKeyExpression instanceof ConstantExpression) { @@ -536,7 +544,7 @@ protected BlockStatement getCodeToHandleAllowedMethods(ClassNode controllerClass final ArgumentListExpression argumentListExpression = new ArgumentListExpression(); argumentListExpression.addExpression(new ConstantExpression(ALLOWED_METHODS_HANDLED_ATTRIBUTE_NAME)); - argumentListExpression.addExpression(new ConstantExpression(methodNode.getName())); + argumentListExpression.addExpression(new ConstantExpression(methodName)); final Expression setAttributeMethodCall = new MethodCallExpression(requestPropertyExpression, "setAttribute", argumentListExpression); @@ -599,7 +607,7 @@ protected void wrapMethodBodyWithExceptionHandling(final ClassNode controllerCla final Statement methodBody = methodNode.getCode(); BlockStatement tryBlock = new BlockStatement(); - BlockStatement codeToHandleAllowedMethods = getCodeToHandleAllowedMethods(controllerClassNode, methodNode); + BlockStatement codeToHandleAllowedMethods = getCodeToHandleAllowedMethods(controllerClassNode, methodNode.getName()); tryBlock.addStatement(codeToHandleAllowedMethods); tryBlock.addStatement(methodBody); @@ -809,7 +817,7 @@ protected void initializeCommandObjectParameter(final BlockStatement wrapper, final TryCatchStatement tryCatchStatement = new TryCatchStatement(tryBlock, new EmptyStatement()); tryCatchStatement.addCatch(new CatchStatement(new Parameter(new ClassNode(DataBindingSourceCreationException.class), "$dataBindingSourceInitializationException"), catchBlock)); - + wrapper.addStatement(tryCatchStatement); } diff --git a/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/plugins/web/api/ControllersApi.java b/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/plugins/web/api/ControllersApi.java index b08daa01531..883b69acd63 100644 --- a/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/plugins/web/api/ControllersApi.java +++ b/grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/plugins/web/api/ControllersApi.java @@ -442,7 +442,14 @@ public Object initializeCommandObject(final Object controllerInstance, final Cla final HttpMethod requestMethod = HttpMethod.valueOf(request.getMethod()); if(entityIdentifierValue != null) { - commandObjectInstance = InvokerHelper.invokeStaticMethod(type, "get", entityIdentifierValue); + try { + commandObjectInstance = InvokerHelper.invokeStaticMethod(type, "get", entityIdentifierValue); + } catch (Exception e) { + final Errors errors = getErrors(controllerInstance); + if(errors != null) { + errors.reject(controllerInstance.getClass().getName() + ".commandObject." + commandObjectParameterName + ".error", e.getMessage()); + } + } } else if(requestMethod == HttpMethod.POST || !isDomainClass){ commandObjectInstance = type.newInstance(); }