grails / gorm-mongodb

GORM for MongoDB
Apache License 2.0
69 stars 31 forks source link

Entities inside embedded collections don't track changes #148

Open valentingoebel opened 4 years ago

valentingoebel commented 4 years ago

Here is an example project:

https://github.com/valentingoebel/grails-dirtycheckingbug/

application.yml requires following env variables so make sure you fill them.

grails:
    mongodb:
        host: ${MONGODB_HOST}
        port: 27017
        username: ${MONGODB_USER}
        password: ${MONGODB_PASS}
        databaseName: grailsdirtycheckingbug

Here is a summary of the code:

class DBList {

    static constraints = {
    }

    List<DBListEntry> entries = []
    static embedded = ['entries']

    static hasMany = [
        entries: DBListEntry
    ]
}

class DBListEntry {

    String value

    static constraints = {
    }
}

class DBListController {

    def index(Integer max) {
        def dblist = DBList.list()[0]

        dblist.entries.each { it ->
            it.value = it.value + "1"
            render "DBListEntry: This should be dirty but it isn't: ${it.listDirtyPropertyNames()}<br>"
        }

        render "DBList: This is not dirty (optional bug): ${dblist.listDirtyPropertyNames()}<br><br>"

        dblist.entries.each { it ->
            it.trackChanges()
        }

        dblist.entries.each { it ->
            it.value = it.value + "2"
            render "DBListEntry: It works!: ${it.listDirtyPropertyNames()}<br>"
        }

        render "DBList: This is not dirty (optional bug): ${dblist.listDirtyPropertyNames()}<br><br>"

        render "Output: ${dblist.entries*.value}<br>"
    }
}
  1. Calling trackChanges() on each entity inside the embedded list results in the desired outcome. I am already using this workaround in my projects but it required substantial changes in my code because using databinding in controller parameters like def show(DBList list) will fill each field before you even get an opportunity to call trackChanges().

  2. Even with trackChanges() there is still the issue that the list itself does not get marked as dirty. This is fine in the non embedded case because changing an entity would mark all its associated entities dirty which is certainly not what we want but for embedded entities it could make sense because an embedded entity is always contained inside a parent entity. Therefore changing the child could be interpreted as a change in the parent.

I only need the first bug fixed because it prevents me from using databinding in controller parameters. The second one is up for discussion and can be rejected if it doesn't make sense. Marking a list dirty when at least one of its elements has changed is very easy so I'm not really insisting on that change.

taplar commented 4 years ago

Same issues, same fix. Wrote logic to call trackChanges on the nested entities in lists to make them not think they are dirty. Also wrote a custom isDirty method that recursively goes down an entity tree and "bubbles" the dirty state of nested elements all the way up the tree. The OP included logic to show their version of the issue. Here is mine that is a reduced form of things similar to how we have things structured and configured.

import com.mongodb.WriteConcern

class Contact { String firstName List

addresses

static embedded = [ 'addresses' ]
static mapWith = 'mongo'

static mapping = {
    version false
    collection 'contacts'
    writeConcern WriteConcern.ACKNOWLEDGED
}

static constraints = {
    firstName blank: false
}

}

class Address { String address1 String address2

static constraints = {
    address1 blank: false
}

}


* Controller `TestController.groovy`

package myapp.test

class TestController { def test () { def contact = Contact.findByFirstName( 'TEST' )

    if ( contact ) {
        println "Existing Contact is dirty? ${contact.isDirty()}" // returns false [EXPECTED]
        contact.delete()
        contact.discard()
    }

    contact = new Contact( firstName: 'TEST', addresses: [] )
    contact.addresses << new Address( address1: 'TEST STREET 1', address2: 'TEST STREET 1.2' )
    contact.addresses << new Address( address1: 'TEST STREET 2', address2: 'TEST STREET 2.2' )

    println "New Contact is dirty? ${contact.isDirty()}" // returns true [EXPECTED]

    contact.save( flush: true )

    println "Saved Contact is dirty? ${contact.isDirty()}" // returns false [EXPECTED]

    contact.discard()

    contact = Contact.findByFirstName( 'TEST' )

    println "Existing Contact is dirty? ${contact.isDirty()}" // returns false [EXPECTED]
    println "Addresses list is dirty? ${contact.isDirty('addresses')}" // returns false [EXPECTED]
    println "Address 1 is dirty? ${contact.addresses[0].isDirty()}" // returns true [NOT EXPECTED]
    println "Address 2 is dirty? ${contact.addresses[1].isDirty()}" // returns true [NOT EXPECTED]

    render status: response.SC_OK, contentType: 'application/json', text: '{}'
}

}



I would not expect anything on a freshly pulled domain record to be marked as dirty.  And changing anything on an Entity I would expect to mark all parent structures as dirty as well.

I did see the note on https://gorm.grails.org/latest/hibernate/manual/index.html#domainClasses under `6.10.1. isDirty` that says "isDirty() does not currently check collection associations, but it does check all other persistent properties and associations." so I assume that the "bubble" up logic isn't currently written to work like that with this sort of situation.  *However*, the children being dirty on a fresh pull, to me, feels like a different thing and should not be happening.