#1294 Update DirtyCheckable.groovy

* Cleanup and remove redundant code.
* Update implementation of markDirty to consider EntityProxy and update the comparison operation accordingly.

Author: puneetbehl

grails-datastore-core/src/main/groovy/org/grails/datastore/mapping/dirty/checking/DirtyCheckable.groovy

@@ -78,12 +78,7 @@ trait DirtyCheckable {
     void markDirty(String propertyName, newValue) {
         if( $changedProperties != null && !$changedProperties.containsKey(propertyName))  {
             def oldValue = ((GroovyObject) this).getProperty(propertyName)
-             boolean isNull = newValue == null
-            if ((isNull && oldValue != null) ||
-                (!isNull && oldValue == null) ||
-                (!isNull && !newValue.equals(oldValue))) {
-                $changedProperties.put propertyName, oldValue
-            }
+            markDirty(propertyName, newValue, oldValue)
         }
     }
 
@@ -97,12 +92,17 @@ trait DirtyCheckable {
             boolean isNull = newValue == null
             if ((isNull && oldValue != null) ||
                 (!isNull && oldValue == null) ||
-                (!isNull && !newValue.equals(oldValue))) {
+                (!isNull && (newOldOrOldValueIsProxy(newValue, oldValue) && newValue.getAt("id") != oldValue.getAt("id")
+                        || (!newOldOrOldValueIsProxy(newValue, oldValue) && !newValue.equals(oldValue))))) {
                 $changedProperties.put propertyName, oldValue
             }
         }
     }
 
+    boolean newOldOrOldValueIsProxy(newValue, oldValue) {
+        (newValue instanceof EntityProxy || oldValue instanceof EntityProxy)
+    }
+
     /**
      * @return A list of the dirty property names
      */
@@ -132,4 +132,4 @@ trait DirtyCheckable {
             return null
         }
     }
-}
+}

grails-datastore-gorm-test/src/test/groovy/org/grails/datastore/gorm/DirtyCheckingSpec.groovy

@@ -110,26 +110,62 @@ class DirtyCheckingSpec extends GormDatastoreSpec {
         TestBook.deleteAll()
         TestAuthor.deleteAll()
     }
+
+    void "test relationships are marked dirty when proxies are used but different"() {
+        given:
+        Long bookId = new TestBook(title: 'Martin Fierro', author: new TestAuthor(name: 'Jose Hernandez'))
+                .save(flush: true, failOnError: true)
+                .id
+        Long otherAuthorId = new TestAuthor(name: "JD").save(flush: true, failOnError: true).id
+        session.flush()
+        session.clear()
+
+        when:
+        TestBook book = TestBook.get(bookId)
+        book.author = TestAuthor.load(otherAuthorId)
+
+        then:
+        proxyHandler.isProxy(book.author)
+        book.isDirty('author')
+        book.isDirty()
+
+        cleanup:
+        TestBook.deleteAll()
+        TestAuthor.deleteAll()
+    }
+
+    void "test relationships marked dirty when domain objects are used and changed"() {
+
+        given:
+        Long bookId = new TestBook(title: 'Martin Fierro', author: new TestAuthor(name: 'Jose Hernandez'))
+                .save(flush: true, failOnError: true)
+                .id
+        Long otherAuthorId = new TestAuthor(name: "JD").save(flush: true, failOnError: true).id
+        session.flush()
+        session.clear()
+
+        when:
+        TestBook book = TestBook.get(bookId)
+        book.author = TestAuthor.get(otherAuthorId)
+
+        then:
+        !proxyHandler.isProxy(book.author)
+        book.isDirty('author')
+        book.isDirty()
+
+        cleanup:
+        TestBook.deleteAll()
+        TestAuthor.deleteAll()
+    }
 }
 
 @Entity
 class TestAuthor {
     String name
-    long version
-
-    @Override
-    boolean equals(o) {
-        if (!(o instanceof TestAuthor)) return false
-        if (this.is(o)) return true
-        TestAuthor that = (TestAuthor) o
-        if (id !=null && that.id !=null) return id == that.id
-        return false
-    }
 }
 
 @Entity
 class TestBook {
     String title
-    long version
     TestAuthor author
 }