GRAILS-10868 - improve binding XML to persistent collections

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

@@ -0,0 +1,63 @@
+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 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-web/src/main/groovy/org/codehaus/groovy/grails/web/binding/GrailsWebDataBinder.groovy

@@ -279,33 +279,45 @@ 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
                         }
                     }
-                    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
+                            }
                         }
                     }
                 }