7.0.0-RC2 BUG FIX - for properties during PreUpdateEvents not being marked dirty when using mongodb

grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/engine/MongoCodecEntityPersister.groovy

@@ -47,6 +47,7 @@ import org.grails.datastore.mapping.dirty.checking.DirtyCheckableCollection
 import org.grails.datastore.mapping.dirty.checking.DirtyCheckingCollection
 import org.grails.datastore.mapping.dirty.checking.DirtyCheckingSupport
 import org.grails.datastore.mapping.engine.EntityAccess
+import org.grails.datastore.mapping.engine.ModificationTrackingEntityAccess
 import org.grails.datastore.mapping.engine.ThirdPartyCacheEntityPersister
 import org.grails.datastore.mapping.engine.internal.MappingUtils
 import org.grails.datastore.mapping.model.ClassMapping
@@ -201,6 +202,7 @@ class MongoCodecEntityPersister extends ThirdPartyCacheEntityPersister<Object> {
             return (Serializable) id
         } else {
             final EntityAccess entityAccess = createEntityAccess(entity, obj)
+
             boolean isAssigned = isAssignedId(entity)
             if (!isAssigned && idIsNull) {
                 id = generateIdentifier(entity)
@@ -237,10 +239,19 @@ class MongoCodecEntityPersister extends ThirdPartyCacheEntityPersister<Object> {
                     }
                 })
             } else {
-                mongoCodecSession.addPendingUpdate(new PendingUpdateAdapter(entity, id, obj, entityAccess) {
+                // Updates during PreUpdateEvents are lost due to not being marked dirty. Track access then mark dirty.
+                final ModificationTrackingEntityAccess trackedAccess = new ModificationTrackingEntityAccess(entityAccess)
+                mongoCodecSession.addPendingUpdate(new PendingUpdateAdapter(entity, id, obj, trackedAccess) {
                     @Override
                     void run() {
-                        if (!cancelUpdate(entity, entityAccess)) {
+                        if (!cancelUpdate(entity, trackedAccess)) {
+                            if (obj instanceof DirtyCheckable && !trackedAccess.modifiedProperties.isEmpty()) {
+                                DirtyCheckable dirtyCheckable = (DirtyCheckable) obj
+                                trackedAccess.modifiedProperties.each { String propertyName, Object value ->
+                                    dirtyCheckable.markDirty(propertyName)
+                                }
+                            }
+
                             updateCaches(entity, obj, id)
                             addCascadeOperation(new PendingOperationAdapter(entity, id, obj) {
                                 @Override

grails-data-mongodb/core/src/main/groovy/org/grails/datastore/mapping/mongo/engine/codecs/PersistentEntityCodec.groovy

@@ -64,7 +64,6 @@ import org.grails.datastore.mapping.engine.internal.MappingUtils
 import org.grails.datastore.mapping.model.EmbeddedPersistentEntity
 import org.grails.datastore.mapping.model.PersistentEntity
 import org.grails.datastore.mapping.model.PersistentProperty
-import org.grails.datastore.mapping.model.config.GormProperties
 import org.grails.datastore.mapping.model.types.Association
 import org.grails.datastore.mapping.model.types.Embedded
 import org.grails.datastore.mapping.model.types.EmbeddedCollection
@@ -249,12 +248,6 @@ class PersistentEntityCodec extends BsonPersistentEntityCodec {
                 }
 
             }
-            else {
-                // schedule lastUpdated if necessary
-                if (entity.getPropertyByName(GormProperties.LAST_UPDATED) != null) {
-                    dirtyProperties.add(GormProperties.LAST_UPDATED)
-                }
-            }
 
             for (propertyName in dirtyProperties) {
                 def prop = entity.getPropertyByName(propertyName)

grails-data-mongodb/core/src/test/groovy/org/grails/datastore/gorm/mongo/BeforeUpdatePropertyPersistenceSpec.groovy

@@ -0,0 +1,194 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one
+ *  or more contributor license agreements.  See the NOTICE file
+ *  distributed with this work for additional information
+ *  regarding copyright ownership.  The ASF licenses this file
+ *  to you under the Apache License, Version 2.0 (the
+ *  "License"); you may not use this file except in compliance
+ *  with the License.  You may obtain a copy of the License at
+ *
+ *    https://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing,
+ *  software distributed under the License is distributed on an
+ *  "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ *  KIND, either express or implied.  See the License for the
+ *  specific language governing permissions and limitations
+ *  under the License.
+ */
+package org.grails.datastore.gorm.mongo
+
+import grails.persistence.Entity
+import org.apache.grails.data.mongo.core.GrailsDataMongoTckManager
+import org.apache.grails.data.testing.tck.base.GrailsDataTckSpec
+import org.bson.types.ObjectId
+import spock.lang.Issue
+
+/**
+ * Tests that properties set in beforeUpdate() are actually persisted to MongoDB.
+ * This specifically tests the scenario where a property is set in beforeUpdate()
+ * but was NOT explicitly modified by the user code.
+ */
+class BeforeUpdatePropertyPersistenceSpec extends GrailsDataTckSpec<GrailsDataMongoTckManager> {
+
+    void setupSpec() {
+        manager.domainClasses.addAll([UserWithBeforeUpdate, UserWithBeforeUpdateAndAutoTimestamp])
+    }
+
+    void "Test that properties set in beforeUpdate are persisted"() {
+        given: "A user is created"
+        def user = new UserWithBeforeUpdate(name: "Fred")
+        def saved = user.save(flush: true)
+
+        expect: "The user was saved successfully"
+        saved != null
+        user.id != null
+        user.errors.allErrors.size() == 0
+
+        when: "The session is cleared and user is retrieved"
+        manager.session.clear()
+        user = UserWithBeforeUpdate.get(user.id)
+
+        then: "beforeInsert was called and random was set"
+        user != null
+        user.name == "Fred"
+        user.random == "Not Updated"
+
+        when: "The user's name is updated (but random is not explicitly modified)"
+        def previousRandom = user.random
+        user.name = "Bob"
+        user.save(flush: true)
+        manager.session.clear()
+        user = UserWithBeforeUpdate.get(user.id)
+
+        then: "beforeUpdate was called and random was changed and persisted"
+        user != null
+        user.name == "Bob"
+        user.random != previousRandom
+        user.random != "Not Updated"
+        user.random.length() == 5 // UUID substring [0..4]
+    }
+
+    void "Test that beforeUpdate is called even when no properties are explicitly modified"() {
+        given: "A user is created"
+        def user = new UserWithBeforeUpdate(name: "Fred")
+        user.save(flush: true)
+        manager.session.clear()
+        user = UserWithBeforeUpdate.get(user.id)
+        def previousRandom = user.random
+
+        when: "The user is saved without any explicit property changes"
+        user.save(flush: true)
+        manager.session.clear()
+        user = UserWithBeforeUpdate.get(user.id)
+
+        then: "beforeUpdate was called and random was updated"
+        user != null
+        user.random != previousRandom
+        user.random != "Not Updated"
+        user.random.length() == 5
+    }
+
+    void "Test that multiple updates continue to trigger beforeUpdate"() {
+        given: "A user is created"
+        def user = new UserWithBeforeUpdate(name: "Fred")
+        user.save(flush: true)
+        manager.session.clear()
+
+        when: "The user is updated multiple times"
+        def randomValues = []
+        3.times {
+            user = UserWithBeforeUpdate.get(user.id)
+            randomValues << user.random
+            user.name = "Name${it}"
+            user.save(flush: true)
+            manager.session.clear()
+        }
+        user = UserWithBeforeUpdate.get(user.id)
+
+        then: "Each update generated a new random value"
+        randomValues.size() == 3
+        randomValues[0] == "Not Updated" // from beforeInsert
+        randomValues[1] != "Not Updated" // first update
+        randomValues[2] != randomValues[1] // second update
+        user.random != randomValues[2] // third update
+        user.random.length() == 5
+    }
+
+    @Issue('GPMONGODB-XXX')
+    void "Test that properties set in beforeUpdate with AutoTimestamp are persisted"() {
+        given: "A user with auto timestamp is created"
+        def user = new UserWithBeforeUpdateAndAutoTimestamp(name: "Fred")
+        user.save(flush: true)
+        manager.session.clear()
+
+        when: "The user is retrieved"
+        user = UserWithBeforeUpdateAndAutoTimestamp.get(user.id)
+
+        then: "beforeInsert was called and random was set"
+        user != null
+        user.name == "Fred"
+        user.random == "Not Updated"
+        user.dateCreated != null
+        user.lastUpdated != null
+
+        when: "The user's name is updated"
+        sleep 100 // ensure lastUpdated differs
+        def previousRandom = user.random
+        def previousLastUpdated = user.lastUpdated
+        user.name = "Bob"
+        user.save(flush: true)
+        manager.session.clear()
+        user = UserWithBeforeUpdateAndAutoTimestamp.get(user.id)
+
+        then: "beforeUpdate was called, random was changed, and lastUpdated was updated"
+        user != null
+        user.name == "Bob"
+        user.random != previousRandom
+        user.random != "Not Updated"
+        user.random.length() == 5
+        user.lastUpdated > previousLastUpdated
+    }
+}
+
+@Entity
+class UserWithBeforeUpdate {
+
+    ObjectId id
+    String name
+    String random
+
+    static constraints = {
+        random nullable: true
+    }
+
+    def beforeInsert() {
+        this.setRandom("Not Updated")
+    }
+
+    def beforeUpdate() {
+        this.setRandom(UUID.randomUUID().toString()[0..4])
+    }
+}
+
+@Entity
+class UserWithBeforeUpdateAndAutoTimestamp {
+
+    ObjectId id
+    String name
+    String random
+    Date dateCreated
+    Date lastUpdated
+
+    static constraints = {
+        random nullable: true
+    }
+
+    def beforeInsert() {
+        this.setRandom("Not Updated")
+    }
+
+    def beforeUpdate() {
+        this.setRandom(UUID.randomUUID().toString()[0..4])
+    }
+}