Merge branch '2.3.x' of github.com:grails/grails-core into 2.3.x

Author: graemerocher

grails-plugin-controllers/src/main/groovy/org/codehaus/groovy/grails/compiler/web/ControllerActionTransformer.java

@@ -206,10 +206,12 @@ public void performInjectionOnAnnotatedClass(SourceUnit source, ClassNode classN
 
     private boolean isExceptionHandlingMethod(MethodNode methodNode) {
         boolean isExceptionHandler = false;
-        Parameter[] parameters = methodNode.getParameters();
-        if(parameters.length == 1) {
-            ClassNode parameterTypeClassNode = parameters[0].getType();
-            isExceptionHandler = parameterTypeClassNode.isDerivedFrom(new ClassNode(Exception.class));
+        if(!methodNode.isPrivate()) {
+            Parameter[] parameters = methodNode.getParameters();
+            if(parameters.length == 1) {
+                ClassNode parameterTypeClassNode = parameters[0].getType();
+                isExceptionHandler = parameterTypeClassNode.isDerivedFrom(new ClassNode(Exception.class));
+            }
         }
         return isExceptionHandler;
     }

grails-plugin-validation/src/main/groovy/org/codehaus/groovy/grails/web/plugins/support/ValidationSupport.groovy

@@ -56,8 +56,11 @@ class ValidationSupport {
             }
             for (prop in constraints.values()) {
                 if (fieldsToValidate == null || fieldsToValidate.contains(prop.propertyName)) {
-                    prop.messageSource = messageSource
-                    prop.validate(object, object.getProperty(prop.propertyName), localErrors)
+                    def fieldError = originalErrors.getFieldError(prop.propertyName)
+                    if(fieldError == null || !fieldError.bindingFailure) {
+                        prop.messageSource = messageSource
+                        prop.validate(object, object.getProperty(prop.propertyName), localErrors)
+                    }
                 }
             }
             object.errors = localErrors

grails-test-suite-persistence/src/test/groovy/org/codehaus/groovy/grails/orm/GrailsWebDataBinderBindingXmlSpec.groovy

@@ -0,0 +1,87 @@
+package org.codehaus.groovy.grails.orm
+
+import grails.persistence.Entity
+import grails.test.mixin.Mock
+import grails.test.mixin.TestMixin
+import grails.test.mixin.domain.DomainClassUnitTestMixin
+
+import org.codehaus.groovy.grails.web.binding.GrailsWebDataBinder
+
+import spock.lang.Issue
+import spock.lang.Specification
+
+@TestMixin(DomainClassUnitTestMixin)
+@Mock([Writer, Book])
+class GrailsWebDataBinderBindingXmlSpec extends Specification {
+
+    @Issue('GRAILS-10868')
+    void 'Test binding a single XML child element to a List'() {
+        given:
+        def binder = new GrailsWebDataBinder(grailsApplication)
+        def writer = new Writer(name: 'Writer One')
+        
+        when:
+        def xml = new XmlSlurper().parseText("""
+  <writer>
+    <name>Writer One</name>
+    <books>
+        <book><title>Book One</title><publisher>Publisher One</publisher></book>
+    </books>
+  </writer>
+""")
+        binder.bind writer, xml
+        
+        then:
+        writer.name == 'Writer One'
+        writer.books.size() == 1
+        writer.books[0].title == 'Book One'
+        writer.books[0].publisher == 'Publisher One'
+    }
+    
+    @Issue('GRAILS-10868')
+    void 'Test adding an existing element to a List by id'() {
+        given:
+        def binder = new GrailsWebDataBinder(grailsApplication)
+        def writer = new Writer(name: 'Writer One')
+        def originalBook = new Book(title: 'Book One', publisher: 'Publisher One')
+        
+        when:
+        originalBook = originalBook.save()
+        
+        then:
+        originalBook
+        
+        when:
+        def xml = new XmlSlurper().parseText("""
+  <writer>
+    <name>Writer Two</name>
+    <books>
+        <book><id>${originalBook.id}</id><title>Updated Book One</title></book>
+        <book><id>2</id><title>Book Two</title><publisher>Publisher Two</publisher></book>
+    </books>
+  </writer>
+""")
+        binder.bind writer, xml
+        
+        then:
+        writer.name == 'Writer Two'
+        writer.books.size() == 2
+        writer.books[0].title == 'Updated Book One'
+        writer.books[0].publisher == 'Publisher One'
+        writer.books[1].publisher == 'Publisher Two'
+        writer.books[1].title == 'Book Two'
+    }
+}
+
+@Entity
+class Writer {
+    String name
+    List books
+    static hasMany = [books: Book]
+}
+
+@Entity
+class Book {
+    String publisher
+    String title
+}

grails-test-suite-uber/src/test/groovy/grails/validation/ValidateableSpec.groovy

@@ -2,6 +2,10 @@ package grails.validation
 
 import grails.test.mixin.TestMixin
 import grails.test.mixin.support.GrailsUnitTestMixin
+
+import org.springframework.validation.FieldError
+
+import spock.lang.Issue
 import spock.lang.Specification
 
 @TestMixin(GrailsUnitTestMixin)
@@ -77,6 +81,31 @@ class ValidateableSpec extends Specification {
         !validateable.hasErrors()
         validateable.errors.errorCount == 0
     }
+    
+    @Issue('GRAILS-10871')
+    void 'Test that binding failures are retained during validation and that the corresponding property is not validated'() {
+        given:
+        def validateable = new MyValidateable()
+        
+        when:
+        def fieldError = new FieldError(MyValidateable.name, 'age', 'forty two', true, null, null, null)
+        validateable.errors.addError fieldError
+       
+        then:
+        validateable.hasErrors()
+        validateable.errors.errorCount == 1
+        validateable.errors.getFieldError('age').rejectedValue == 'forty two'
+        
+        when:
+        validateable.name = 'lower case'
+        
+        then:
+        !validateable.validate()
+        validateable.hasErrors()
+        validateable.errors.errorCount == 2
+        validateable.errors.getFieldError('age').rejectedValue == 'forty two'
+        validateable.errors.getFieldError('name').rejectedValue == 'lower case'
+    }
 }
 
 @Validateable

grails-test-suite-web/src/test/groovy/org/codehaus/groovy/grails/web/controllers/ControllerExceptionHandlerSpec.groovy

@@ -6,6 +6,8 @@ import grails.test.mixin.TestFor
 import java.sql.BatchUpdateException
 import java.sql.SQLException
 
+import javax.xml.soap.SOAPException
+
 import spock.lang.Issue
 import spock.lang.Specification
 
@@ -93,6 +95,24 @@ class ControllerExceptionHandlerSpec extends Specification {
         model.problemDescription == 'A Number Was Invalid'
     }
 
+    void 'Test throwing an exception that does not have a handler'() {
+        when:
+        params.exceptionToThrow = 'javax.xml.soap.SOAPException'
+        def model = controller.testActionWithNonCommandObjectParameter()
+
+        then:
+        thrown SOAPException
+    }
+
+    void 'Test throwing an exception that does not have a handler and does match a private method in the parent controller'() {
+        when: 'a controller action throws an exception which matches an inherited private method which should not be treated as an exception handler'
+        params.exceptionToThrow = 'java.io.IOException'
+        def model = controller.testActionWithNonCommandObjectParameter()
+
+        then: 'the method is ignored and the exception is thrown'
+        thrown IOException
+    }
+
     void 'Test action throws an exception that does not have a corresponding error handler'() {
         when:
         params.exceptionToThrow = 'java.lang.UnsupportedOperationException'
@@ -132,7 +152,14 @@ class ControllerExceptionHandlerSpec extends Specification {
 }
 
 @Artefact('Controller')
-class ErrorHandlersController {
+abstract class SomeAbstractController {
+    
+    private somePrivateMethodWhichIsNotAnExceptionHandler(IOException e) {
+    }
+}
+
+@Artefact('Controller')
+class ErrorHandlersController extends SomeAbstractController {
 
     def testAction() {
         def exceptionClass = Class.forName(params.exceptionToThrow)

grails-web/src/main/groovy/org/codehaus/groovy/grails/web/binding/GrailsWebDataBinder.groovy

@@ -279,33 +279,47 @@ class GrailsWebDataBinder extends SimpleDataBinder {
                 }
             } else if(Collection.isAssignableFrom(metaProperty.type)) {
                 def referencedType = getReferencedTypeForCollection(propName, obj)
-                if(referencedType && isDomainClass(referencedType) && val instanceof List) {
-                    needsBinding = false
-                    initializeCollection obj, metaProperty.name, metaProperty.type
-                    def itemsWhichNeedBinding = []
-                    val.each { item ->
-                        def persistentInstance
-                        if(item instanceof Map || item instanceof DataBindingSource) {
-                            def idValue = getIdentifierValueFrom(item)
-                            if(idValue != null) {
-                                persistentInstance = getPersistentInstance(referencedType, idValue)
-                                if(persistentInstance != null) {
-                                    bind persistentInstance, new SimpleMapDataBindingSource(item), listener
-                                    itemsWhichNeedBinding << persistentInstance
-                                }
-                            }
-                        }
-                        if(persistentInstance == null) {
-                            itemsWhichNeedBinding << item
+                if(referencedType && isDomainClass(referencedType)) {
+                    def listValue
+                    if(val instanceof List) {
+                        listValue = (List)val
+                    } else if(val instanceof GPathResultMap && val.size() == 1) {
+                        def mapValue = (GPathResultMap)val
+                        def valueInMap = mapValue[mapValue.keySet()[0]]
+                        if(valueInMap instanceof List) {
+                            listValue = (List)valueInMap
+                        } else if(valueInMap instanceof GPathResultMap) {
+                            listValue = [valueInMap]
                         }
                     }
-                    if(itemsWhichNeedBinding) {
-                        def coll = obj[metaProperty.name]
-                        if(coll instanceof Collection) {
-                            coll.clear()
+                    if(listValue != null) {
+                        needsBinding = false
+                        initializeCollection obj, metaProperty.name, metaProperty.type
+                        def itemsWhichNeedBinding = []
+                        listValue.each { item ->
+                            def persistentInstance
+                            if(item instanceof Map || item instanceof DataBindingSource) {
+                                def idValue = getIdentifierValueFrom(item)
+                                if(idValue != null) {
+                                    persistentInstance = getPersistentInstance(referencedType, idValue)
+                                    if(persistentInstance != null) {
+                                        bind persistentInstance, new SimpleMapDataBindingSource(item), listener
+                                        itemsWhichNeedBinding << persistentInstance
+                                    }
+                                }
+                            }
+                            if(persistentInstance == null) {
+                                itemsWhichNeedBinding << item
+                            }
                         }
-                        for(item in itemsWhichNeedBinding) {
-                            addElementToCollection obj, metaProperty.name, metaProperty.type, item, false
+                        if(itemsWhichNeedBinding) {
+                            def coll = obj[metaProperty.name]
+                            if(coll instanceof Collection) {
+                                coll.clear()
+                            }
+                            for(item in itemsWhichNeedBinding) {
+                                addElementToCollection obj, metaProperty.name, metaProperty.type, item, false
+                            }
                         }
                     }
                 }