Merge branch '6.1.x'

Author: graemerocher

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

@@ -78,7 +78,9 @@ trait DirtyCheckable {
     void markDirty(String propertyName, newValue) {
         if( $changedProperties != null && !$changedProperties.containsKey(propertyName))  {
             def oldValue = ((GroovyObject) this).getProperty(propertyName)
-            if(newValue != oldValue) {
+            if ((newValue == null && oldValue != null) ||
+                (newValue != null && oldValue == null) ||
+                (newValue && !newValue.equals(oldValue))) {
                 $changedProperties.put propertyName, oldValue
             }
         }
@@ -91,7 +93,9 @@ trait DirtyCheckable {
      */
     void markDirty(String propertyName, newValue, oldValue) {
         if( $changedProperties != null && !$changedProperties.containsKey(propertyName))  {
-            if(newValue != oldValue) {
+            if ((newValue == null && oldValue != null) ||
+                (newValue != null && oldValue == null) ||
+                (newValue && !newValue.equals(oldValue))) {
                 $changedProperties.put propertyName, oldValue
             }
         }

grails-datastore-core/src/test/groovy/org/grails/datastore/mapping/dirty/checking/DirtyCheckableSpec.groovy

@@ -0,0 +1,47 @@
+package org.grails.datastore.mapping.dirty.checking
+
+
+import groovy.transform.Sortable
+import spock.lang.Issue
+import spock.lang.Specification
+
+class DirtyCheckableSpec extends Specification {
+
+    @Issue('https://github.com/grails/grails-data-mapping/issues/1231')
+    def 'setting a field that implements Comparable dirty checks properly'() {
+        given: 'a person'
+        def person = new Person(name: 'John Doe')
+
+        and: 'a blog post'
+        def blogPost = new BlogPost(title: 'Hello World', content: 'some content')
+        person.lastViewedPost = blogPost
+
+        and: 'a second blog post is made'
+        def anotherBlogPost = new BlogPost(title: blogPost.title, content: 'different content, same title')
+
+        and: 'change list is reset'
+        person.trackChanges()
+
+        when: 'the lastViewedPost is set to something different'
+        person.lastViewedPost = anotherBlogPost
+
+        and: 'markDirty is called (normally would be weaved in by a DirtyCheckingTransformer)'
+        person.markDirty('lastViewedPost', anotherBlogPost, blogPost)
+
+        then:
+        person.hasChanged()
+        person.hasChanged('lastViewedPost')
+        person.getOriginalValue('lastViewedPost').equals(blogPost)
+    }
+}
+
+class Person implements DirtyCheckable {
+    String name
+    BlogPost lastViewedPost
+}
+
+@Sortable(includes = ['title'])
+class BlogPost {
+    String title
+    String content
+}

grails-datastore-gorm-tck/src/main/groovy/grails/gorm/tests/NullValueEqualSpec.groovy

@@ -0,0 +1,19 @@
+package grails.gorm.tests
+
+class NullValueEqualSpec extends GormDatastoreSpec {
+
+  void "test null value in equal and not equal"() {
+    when:
+    new TestEntity(name:"Fred", age: null).save(failOnError: true)
+    new TestEntity(name:"Bob", age: 11).save(failOnError: true)
+    new TestEntity(name:"Jack", age: null).save(flush:true, failOnError: true)
+
+    then:
+    TestEntity.countByAge(11) == 1
+    TestEntity.findAllByAge(null).size() == 2
+    TestEntity.countByAge(null) == 2
+
+    TestEntity.countByAgeNotEqual(11) == 2
+    TestEntity.countByAgeNotEqual(null) == 1
+  }
+}

grails-datastore-gorm-tck/src/main/groovy/grails/gorm/tests/TestEntity.groovy

@@ -16,12 +16,13 @@ class TestEntity implements Serializable {
 
     static mapping = {
         name index:true
-        age index:true, nullable:true
-        child index:true, nullable:true
+        age index:true
+        child index:true
     }
 
     static constraints = {
         name blank:false
         child nullable:true
+        age nullable:true
     }
 }

grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/finders/DynamicFinder.java

@@ -765,11 +765,11 @@ private static String calcPropertyName(String queryParameter, String clause) {
      * @return the initialized expression
      */
     private MethodExpression getInitializedExpression(MethodExpression expression, Object[] arguments) {
-        if (expression instanceof Equal && arguments.length == 1 && arguments[0] == null) {
-            expression = new IsNull(expression.propertyName);
-        } else {
-            expression.setArguments(arguments);
-        }
+//        if (expression instanceof Equal && arguments.length == 1 && arguments[0] == null) { // logic moved directly to Equal.createCriterion
+//            expression = new IsNull(expression.propertyName);
+//        } else {
+        expression.setArguments(arguments);
+//        }
         return expression;
     }
 

grails-datastore-gorm/src/main/groovy/org/grails/datastore/gorm/finders/MethodExpression.java

@@ -422,7 +422,12 @@ public Equal(String propertyName) {
 
         @Override
         public Query.Criterion createCriterion() {
-            return Restrictions.eq(propertyName, arguments[0]);
+            Object argument = arguments[0];
+            if (argument != null) {
+                return Restrictions.eq(propertyName, argument);
+            } else {
+                return Restrictions.isNull(propertyName);
+            }
         }
 
     }
@@ -437,7 +442,12 @@ public NotEqual(String propertyName) {
 
         @Override
         public Query.Criterion createCriterion() {
-            return Restrictions.ne(propertyName, arguments[0]);
+            Object argument = arguments[0];
+            if (argument != null) {
+                return Restrictions.ne(propertyName, arguments[0]);
+            } else {
+                return Restrictions.isNotNull(propertyName);
+            }
         }
 
     }