#2710 - JSONB fields are not considered dirty after modified in BeanPersistController preUpdate

ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java

@@ -77,8 +77,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
    */
   private List<BeanPropertyAssocMany<?>> updatedManys;
   /**
-   * Need to get and store the updated properties because the persist listener is notified
-   * later on a different thread and the bean has been reset at that point.
+   * Store the updated properties to notify persist listener.
    */
   private Set<String> updatedProperties;
   /**
@@ -122,7 +121,7 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
    */
   private boolean complete;
   /**
-   * Many to many intersection table changes that are held for later batch processing.
+   * Many-to-many intersection table changes that are held for later batch processing.
    */
   private List<SaveMany> saveMany;
 
@@ -149,8 +148,10 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
         intercept.setNewBeanForUpdate();
         statelessUpdate = true;
       }
-      // Mark Mutable scalar properties (like Hstore) as dirty where necessary
-      beanDescriptor.checkMutableProperties(intercept);
+      if (!intercept.isDirty()) {
+        // check if mutable scalar properties are dirty
+        beanDescriptor.checkMutableProperties(intercept);
+      }
     }
     this.concurrencyMode = beanDescriptor.concurrencyMode(intercept);
     this.publish = Flags.isPublish(flags);
@@ -726,10 +727,6 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
         executeInsert();
         return -1;
       case UPDATE:
-        if (beanPersistListener != null) {
-          // store the updated properties for sending later
-          updatedProperties = updatedProperties();
-        }
         executeUpdate();
         return -1;
       case DELETE_SOFT:
@@ -1208,6 +1205,11 @@ public final class PersistRequestBean<T> extends PersistRequest implements BeanP
   private void executeUpdate() {
     setTenantId();
     if (controller == null || controller.preUpdate(this)) {
+      // check dirty state for mutable scalar properties (like DbJson, Hstore)
+      beanDescriptor.checkMutableProperties(intercept);
+      if (beanPersistListener != null) {
+        updatedProperties = updatedProperties();
+      }
       postControllerPrepareUpdate();
       beanManager.getBeanPersister().update(this);
     }

ebean-test/src/test/java/io/ebean/xtest/event/BeanPersistControllerTest.java

@@ -4,6 +4,7 @@ package io.ebean.xtest.event;
 import io.ebean.Database;
 import io.ebean.DatabaseFactory;
 import io.ebean.Transaction;
+import io.ebean.ValuePair;
 import io.ebean.config.DatabaseConfig;
 import io.ebean.event.BeanDeleteIdRequest;
 import io.ebean.event.BeanPersistAdapter;
@@ -13,9 +14,7 @@ import org.tests.model.basic.EBasicVer;
 import org.tests.model.basic.UTDetail;
 import org.tests.model.basic.UTMaster;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
+import java.util.*;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -26,8 +25,30 @@ public class BeanPersistControllerTest {
   private final PersistAdapter stopPersistingAdapter = new PersistAdapter(false);
 
   @Test
-  public void issue_1341() {
+  public void issued1() {
+    Database db = getDatabase(continuePersistingAdapter);
 
+    UTMaster bean0 = new UTMaster("m0");
+    bean0.setJournal(new UTMaster.Journal());
+    db.save(bean0);
+
+    UTMaster change0 = db.find(UTMaster.class, bean0.getId());
+    change0.setName("change0");
+    db.save(change0);
+
+    UTMaster change1 = db.find(UTMaster.class, bean0.getId());
+    change1.setName("change1");
+    db.save(change1);
+
+    UTMaster again = db.find(UTMaster.class, bean0.getId());
+    UTMaster.Journal journal = again.getJournal();
+    assertThat(journal.getEntries()).hasSize(2);
+
+    db.shutdown();
+  }
+
+  @Test
+  public void issue_1341() {
     Database db = getDatabase(continuePersistingAdapter);
 
     UTMaster bean0 = new UTMaster("one0");
@@ -118,7 +139,6 @@ public class BeanPersistControllerTest {
   }
 
   private Database getDatabase(PersistAdapter persistAdapter) {
-
     DatabaseConfig config = new DatabaseConfig();
     config.setName("h2ebasicver");
     config.loadFromProperties();
@@ -133,7 +153,6 @@ public class BeanPersistControllerTest {
     config.getClasses().add(UTDetail.class);
 
     config.add(persistAdapter);
-
     return DatabaseFactory.create(config);
   }
 
@@ -177,6 +196,16 @@ public class BeanPersistControllerTest {
         // invoke lazy loading ... which invoke the flush of the jdbc batch
         detail.setQty(42);
       }
+      if (bean instanceof UTMaster) {
+        UTMaster master = (UTMaster)bean;
+        UTMaster.Journal journal = master.getJournal();
+        if (journal == null) {
+          journal = new UTMaster.Journal();
+          master.setJournal(journal);
+        }
+        // modify a "mutable scalar type" in preUpdate, should be included in update
+        journal.addEntry();
+      }
 
       return continueDefaultPersisting;
     }

ebean-test/src/test/java/org/tests/model/basic/UTMaster.java

@@ -1,9 +1,11 @@
 package org.tests.model.basic;
 
 import io.ebean.Model;
+import io.ebean.annotation.DbJsonB;
 
 import javax.persistence.*;
 import java.time.LocalDate;
+import java.time.LocalDateTime;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -20,12 +22,31 @@ public class UTMaster extends Model {
 
   LocalDate eventDate;
 
+  @DbJsonB
+  Journal journal;
+
   @Version
   Integer version;
 
   @OneToMany(cascade = CascadeType.ALL)
   List<UTDetail> details;
 
+  /**
+   * Mutating content persisted as JSON.
+   */
+  public static class Journal {
+    private List<String> entries = new ArrayList<>();
+    public List<String> getEntries() {
+      return entries;
+    }
+    public void setEntries(List<String> entries) {
+      this.entries = entries;
+    }
+    public void addEntry() {
+      entries.add(LocalDateTime.now().toString());
+    }
+  }
+
   public UTMaster() {
 
   }
@@ -74,6 +95,14 @@ public class UTMaster extends Model {
     this.version = version;
   }
 
+  public Journal getJournal() {
+    return journal;
+  }
+
+  public void setJournal(Journal journal) {
+    this.journal = journal;
+  }
+
   public List<UTDetail> getDetails() {
     return details;
   }