From 8301fd5e74e5784db53ccff863549eb698a7377c Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Mon, 13 Jun 2022 18:35:42 +1200 Subject: [PATCH] #2710 - JSONB fields are not considered dirty after modified in BeanPersistController preUpdate --- .../server/core/PersistRequestBean.java | 20 +++++---- .../event/BeanPersistControllerTest.java | 41 ++++++++++++++++--- .../java/org/tests/model/basic/UTMaster.java | 29 +++++++++++++ 3 files changed, 75 insertions(+), 15 deletions(-) diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java b/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java index f73fcb97d..b7545b07a 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java @@ -77,8 +77,7 @@ public final class PersistRequestBean extends PersistRequest implements BeanP */ private List> 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 updatedProperties; /** @@ -122,7 +121,7 @@ public final class PersistRequestBean 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; @@ -149,8 +148,10 @@ public final class PersistRequestBean 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 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 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); } diff --git a/ebean-test/src/test/java/io/ebean/xtest/event/BeanPersistControllerTest.java b/ebean-test/src/test/java/io/ebean/xtest/event/BeanPersistControllerTest.java index e584bc40b..b991e07be 100644 --- a/ebean-test/src/test/java/io/ebean/xtest/event/BeanPersistControllerTest.java +++ b/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; } diff --git a/ebean-test/src/test/java/org/tests/model/basic/UTMaster.java b/ebean-test/src/test/java/org/tests/model/basic/UTMaster.java index 07c5fec39..4c1b2998c 100644 --- a/ebean-test/src/test/java/org/tests/model/basic/UTMaster.java +++ b/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 details; + /** + * Mutating content persisted as JSON. + */ + public static class Journal { + private List entries = new ArrayList<>(); + public List getEntries() { + return entries; + } + public void setEntries(List 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 getDetails() { return details; }