Skip to content

Commit d5df2f6

Browse files
joemccall86graemerocher
authored andcommitted
Use equals method for dirty checking (#1232)
* Use equals method for dirty checking The `==` method will always call `compareTo` if the property implements `Comparable`, which is not always desirable. Call the equals method directly instead. Include spec illustrating the new behavior. * Handle case where the new or old values are null Fixes test failures
1 parent 4bb003a commit d5df2f6

File tree

2 files changed

+53
-2
lines changed

2 files changed

+53
-2
lines changed

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

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,9 @@ trait DirtyCheckable {
7878
void markDirty(String propertyName, newValue) {
7979
if( $changedProperties != null && !$changedProperties.containsKey(propertyName)) {
8080
def oldValue = ((GroovyObject) this).getProperty(propertyName)
81-
if(newValue != oldValue) {
81+
if ((newValue == null && oldValue != null) ||
82+
(newValue != null && oldValue == null) ||
83+
(newValue && !newValue.equals(oldValue))) {
8284
$changedProperties.put propertyName, oldValue
8385
}
8486
}
@@ -91,7 +93,9 @@ trait DirtyCheckable {
9193
*/
9294
void markDirty(String propertyName, newValue, oldValue) {
9395
if( $changedProperties != null && !$changedProperties.containsKey(propertyName)) {
94-
if(newValue != oldValue) {
96+
if ((newValue == null && oldValue != null) ||
97+
(newValue != null && oldValue == null) ||
98+
(newValue && !newValue.equals(oldValue))) {
9599
$changedProperties.put propertyName, oldValue
96100
}
97101
}
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
package org.grails.datastore.mapping.dirty.checking
2+
3+
4+
import groovy.transform.Sortable
5+
import spock.lang.Issue
6+
import spock.lang.Specification
7+
8+
class DirtyCheckableSpec extends Specification {
9+
10+
@Issue('https://github.com/grails/grails-data-mapping/issues/1231')
11+
def 'setting a field that implements Comparable dirty checks properly'() {
12+
given: 'a person'
13+
def person = new Person(name: 'John Doe')
14+
15+
and: 'a blog post'
16+
def blogPost = new BlogPost(title: 'Hello World', content: 'some content')
17+
person.lastViewedPost = blogPost
18+
19+
and: 'a second blog post is made'
20+
def anotherBlogPost = new BlogPost(title: blogPost.title, content: 'different content, same title')
21+
22+
and: 'change list is reset'
23+
person.trackChanges()
24+
25+
when: 'the lastViewedPost is set to something different'
26+
person.lastViewedPost = anotherBlogPost
27+
28+
and: 'markDirty is called (normally would be weaved in by a DirtyCheckingTransformer)'
29+
person.markDirty('lastViewedPost', anotherBlogPost, blogPost)
30+
31+
then:
32+
person.hasChanged()
33+
person.hasChanged('lastViewedPost')
34+
person.getOriginalValue('lastViewedPost').equals(blogPost)
35+
}
36+
}
37+
38+
class Person implements DirtyCheckable {
39+
String name
40+
BlogPost lastViewedPost
41+
}
42+
43+
@Sortable(includes = ['title'])
44+
class BlogPost {
45+
String title
46+
String content
47+
}

0 commit comments

Comments
 (0)