From 07946cd5051508022e5b6b040b785a0690637cbe Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Wed, 28 Mar 2018 17:31:53 +1300 Subject: [PATCH] #115 - Mapping - Add support for @ElementCollection enhancement Add element collection notification - combine into preUpdate/postUpdate of the bean --- .../server/core/PersistRequestBean.java | 50 +++++++++++- .../deploy/BeanLifecycleAdapterFactory.java | 3 - .../server/persist/DefaultPersister.java | 7 +- .../persist/SaveManyElementCollection.java | 5 +- .../persist/SaveManyElementCollectionMap.java | 6 +- .../EcPersonPersistAdapter.java | 58 ++++++++++++++ .../TestElementCollectionBasic.java | 77 +++++++++++++++++++ 7 files changed, 193 insertions(+), 13 deletions(-) create mode 100644 src/test/java/org/tests/model/elementcollection/EcPersonPersistAdapter.java diff --git a/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java b/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java index 33f72c636..283d426bd 100644 --- a/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java +++ b/src/main/java/io/ebeaninternal/server/core/PersistRequestBean.java @@ -154,6 +154,11 @@ public final class PersistRequestBean extends PersistRequest implements BeanP */ private boolean getterCallback; + /** + * postUpdate notifications. Used to combine bean and element update updates into single postUpdate event. + */ + private int pendingPostUpdateNotify; + public PersistRequestBean(SpiEbeanServer server, T bean, Object parentBean, BeanManager mgr, SpiTransaction t, PersistExecute persistExecute, PersistRequest.Type type, int flags) { @@ -337,11 +342,16 @@ public final class PersistRequestBean extends PersistRequest implements BeanP @Override public void preGetterTrigger(int propertyIndex) { - if (propertyIndex < 0 || beanDescriptor.isGeneratedProperty(propertyIndex)) { + if (flushBatchOnGetter(propertyIndex)) { transaction.flushBatch(); } } + private boolean flushBatchOnGetter(int propertyIndex) { + // propertyIndex of -1 the Id property, no flush for get Id on UPDATE + return propertyIndex == -1 ? type == Type.INSERT : beanDescriptor.isGeneratedProperty(propertyIndex); + } + public void setSkipBatchForTopLevel() { skipBatchForTopLevel = true; } @@ -858,6 +868,16 @@ public final class PersistRequestBean extends PersistRequest implements BeanP } } + private void postUpdateNotify() { + if (pendingPostUpdateNotify > 0) { + // invoke the delayed postUpdate notification (combined with element collection update) + controller.postUpdate(this); + } else { + // batched update with no element collection, send postUpdate notification once it executes + pendingPostUpdateNotify = -1; + } + } + /** * Aggressive L1 and L2 cache cleanup for deletes. */ @@ -905,13 +925,38 @@ public final class PersistRequestBean extends PersistRequest implements BeanP } } + /** + * Ensure the preUpdate event fires (for case where only element collection has changed). + */ + public void preElementCollectionUpdate() { + if (controller != null && !dirty) { + // fire preUpdate notification when only element collection updated + controller.preUpdate(this); + } + } + + /** + * Combine with the beans postUpdate event notification. + */ + public void postElementCollectionUpdate() { + if (controller != null) { + pendingPostUpdateNotify += 2; + } + } + private void controllerPost() { switch (type) { case INSERT: controller.postInsert(this); break; case UPDATE: - controller.postUpdate(this); + if (pendingPostUpdateNotify == -1) { + // notify now - batched bean update with no element collection + controller.postUpdate(this); + } else { + // delay notify to combine with element collection update + pendingPostUpdateNotify++; + } break; case SOFT_DELETE: controller.postSoftDelete(this); @@ -1031,6 +1076,7 @@ public final class PersistRequestBean extends PersistRequest implements BeanP setNotifyCache(); addEvent(); } + postUpdateNotify(); } /** diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanLifecycleAdapterFactory.java b/src/main/java/io/ebeaninternal/server/deploy/BeanLifecycleAdapterFactory.java index ae6e670f0..f9bdcd224 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanLifecycleAdapterFactory.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanLifecycleAdapterFactory.java @@ -102,7 +102,6 @@ class BeanLifecycleAdapterFactory { postInserts.add(method); hasPersistMethods = true; } - if (method.isAnnotationPresent(PreUpdate.class)) { preUpdates.add(method); hasPersistMethods = true; @@ -111,7 +110,6 @@ class BeanLifecycleAdapterFactory { postUpdates.add(method); hasPersistMethods = true; } - if (method.isAnnotationPresent(PreRemove.class)) { preDeletes.add(method); hasPersistMethods = true; @@ -128,7 +126,6 @@ class BeanLifecycleAdapterFactory { postSoftDeletes.add(method); hasPersistMethods = true; } - if (method.isAnnotationPresent(PostLoad.class)) { postLoads.add(method); } diff --git a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index b16a0ced8..c3cb2dcda 100644 --- a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -510,11 +510,8 @@ public final class DefaultPersister implements Persister { if (request.isDirty()) { request.executeOrQueue(); - } else { - // skip validation on unchanged bean - if (logger.isDebugEnabled()) { - logger.debug(Message.msg("persist.update.skipped", request.getBean())); - } + } else if (logger.isDebugEnabled()) { + logger.debug(Message.msg("persist.update.skipped", request.getBean())); } if (request.isPersistCascade()) { diff --git a/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollection.java b/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollection.java index 1867421fa..402a5e80a 100644 --- a/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollection.java +++ b/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollection.java @@ -27,10 +27,10 @@ class SaveManyElementCollection extends SaveManyBase { } Object parentId = request.getBeanId(); - SpiEbeanServer server = request.getServer(); if (!insertedParent) { + request.preElementCollectionUpdate(); SqlUpdate sqlDelete = many.deleteByParentId(parentId, null); server.execute(sqlDelete, transaction); } @@ -49,5 +49,8 @@ class SaveManyElementCollection extends SaveManyBase { transaction.depth(-1); resetModifyState(); + if (!insertedParent) { + request.postElementCollectionUpdate(); + } } } diff --git a/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollectionMap.java b/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollectionMap.java index 3ba94d781..e19b7e314 100644 --- a/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollectionMap.java +++ b/src/main/java/io/ebeaninternal/server/persist/SaveManyElementCollectionMap.java @@ -29,10 +29,9 @@ class SaveManyElementCollectionMap extends SaveManyBase { } Object parentId = request.getBeanId(); - SpiEbeanServer server = request.getServer(); - if (!insertedParent) { + request.preElementCollectionUpdate(); SqlUpdate sqlDelete = many.deleteByParentId(parentId, null); server.execute(sqlDelete, transaction); } @@ -51,5 +50,8 @@ class SaveManyElementCollectionMap extends SaveManyBase { transaction.depth(-1); resetModifyState(); + if (!insertedParent) { + request.postElementCollectionUpdate(); + } } } diff --git a/src/test/java/org/tests/model/elementcollection/EcPersonPersistAdapter.java b/src/test/java/org/tests/model/elementcollection/EcPersonPersistAdapter.java new file mode 100644 index 000000000..02dd4c191 --- /dev/null +++ b/src/test/java/org/tests/model/elementcollection/EcPersonPersistAdapter.java @@ -0,0 +1,58 @@ +package org.tests.model.elementcollection; + +import io.ebean.event.BeanPersistAdapter; +import io.ebean.event.BeanPersistRequest; + +import java.util.ArrayList; +import java.util.List; + +public class EcPersonPersistAdapter extends BeanPersistAdapter { + + private static List LOG = new ArrayList<>(); + + static List eventLog() { + List copy = new ArrayList(LOG); + LOG.clear(); + return copy; + } + + @Override + public boolean isRegisterFor(Class cls) { + return cls.equals(EcPerson.class); + } + + + @Override + public boolean preInsert(BeanPersistRequest request) { + LOG.add("preInsert"); + return true; + } + + @Override + public void postInsert(BeanPersistRequest request) { + LOG.add("postInsert"); + } + + @Override + public boolean preUpdate(BeanPersistRequest request) { + LOG.add("preUpdate"); + return true; + } + + @Override + public void postUpdate(BeanPersistRequest request) { + LOG.add("postUpdate"); + } + + @Override + public boolean preDelete(BeanPersistRequest request) { + LOG.add("preDelete"); + return true; + } + + @Override + public void postDelete(BeanPersistRequest request) { + LOG.add("postDelete"); + } + +} diff --git a/src/test/java/org/tests/model/elementcollection/TestElementCollectionBasic.java b/src/test/java/org/tests/model/elementcollection/TestElementCollectionBasic.java index b55b3dba2..0dea97952 100644 --- a/src/test/java/org/tests/model/elementcollection/TestElementCollectionBasic.java +++ b/src/test/java/org/tests/model/elementcollection/TestElementCollectionBasic.java @@ -2,6 +2,7 @@ package org.tests.model.elementcollection; import io.ebean.BaseTestCase; import io.ebean.Ebean; +import io.ebean.Transaction; import org.ebeantest.LoggedSqlCollector; import org.junit.Test; @@ -11,6 +12,10 @@ import static org.assertj.core.api.Assertions.assertThat; public class TestElementCollectionBasic extends BaseTestCase { + private List eventLog() { + return EcPersonPersistAdapter.eventLog(); + } + @Test public void test() { @@ -21,6 +26,8 @@ public class TestElementCollectionBasic extends BaseTestCase { person.getPhoneNumbers().add("021 4321"); Ebean.save(person); + assertThat(eventLog()).containsExactly("preInsert", "postInsert"); + List sql = LoggedSqlCollector.current(); assertThat(sql).hasSize(2); assertThat(sql.get(0)).contains("insert into ec_person"); @@ -31,6 +38,8 @@ public class TestElementCollectionBasic extends BaseTestCase { person1.getPhoneNumbers().add("09 4321"); Ebean.save(person1); + assertThat(eventLog()).containsExactly("preInsert", "postInsert"); + LoggedSqlCollector.current(); List found = @@ -82,6 +91,26 @@ public class TestElementCollectionBasic extends BaseTestCase { assertThat(sql).hasSize(1); assertThat(sql.get(0)).contains("update ec_person"); + assertThat(eventLog()).containsExactly("preUpdate", "postUpdate"); + + updateBasicInBatch(bean); + } + + private void updateBasicInBatch(EcPerson bean) { + + try (Transaction txn = Ebean.beginTransaction()) { + txn.setBatchMode(true); + bean.setName("Fiona021-mod-0-batch"); + Ebean.save(bean); + txn.commit(); + } + + List sql = LoggedSqlCollector.current(); + assertThat(sql).hasSize(1); + assertThat(sql.get(0)).contains("update ec_person"); + + assertThat(eventLog()).containsExactly("preUpdate", "postUpdate"); + updateBoth(bean); } @@ -97,6 +126,29 @@ public class TestElementCollectionBasic extends BaseTestCase { assertThat(sql.get(1)).contains("delete from ec_person_phone where owner_id=?"); assertThat(sql.get(2)).contains("insert into ec_person_phone (owner_id,phone) values (?,?)"); + assertThat(eventLog()).containsExactly("preUpdate", "postUpdate"); + + updateBothInBatch(bean); + } + + private void updateBothInBatch(EcPerson bean) { + + try (Transaction txn = Ebean.beginTransaction()) { + txn.setBatchMode(true); + bean.setName("Fiona021-mod-both-batch"); + bean.getPhoneNumbers().add("01-22123"); + Ebean.save(bean); + txn.commit(); + } + + List sql = LoggedSqlCollector.current(); + assertThat(sql).hasSize(3); + assertThat(sql.get(0)).contains("update ec_person set name=?, version=? where id=? and version=?"); + assertThat(sql.get(1)).contains("delete from ec_person_phone where owner_id=?"); + assertThat(sql.get(2)).contains("insert into ec_person_phone (owner_id,phone) values (?,?)"); + + assertThat(eventLog()).containsExactly("preUpdate", "postUpdate"); + updateNothing(bean); } @@ -107,6 +159,27 @@ public class TestElementCollectionBasic extends BaseTestCase { List sql = LoggedSqlCollector.current(); assertThat(sql).hasSize(0); + assertThat(eventLog()).isEmpty(); + + updateOnlyCollectionInBatch(bean); + } + + private void updateOnlyCollectionInBatch(EcPerson bean) { + + try (Transaction txn = Ebean.beginTransaction()) { + txn.setBatchMode(true); + bean.getPhoneNumbers().add("01-4321"); + Ebean.save(bean); + txn.commit(); + } + + List sql = LoggedSqlCollector.current(); + assertThat(sql).hasSize(2); + assertThat(sql.get(0)).contains("delete from ec_person_phone where owner_id=?"); + assertThat(sql.get(1)).contains("insert into ec_person_phone (owner_id,phone) values (?,?)"); + + assertThat(eventLog()).containsExactly("preUpdate", "postUpdate"); + updateOnlyCollection(bean); } @@ -120,6 +193,8 @@ public class TestElementCollectionBasic extends BaseTestCase { assertThat(sql.get(0)).contains("delete from ec_person_phone where owner_id=?"); assertThat(sql.get(1)).contains("insert into ec_person_phone (owner_id,phone) values (?,?)"); + assertThat(eventLog()).containsExactly("preUpdate", "postUpdate"); + delete(bean); } @@ -131,6 +206,8 @@ public class TestElementCollectionBasic extends BaseTestCase { assertThat(sql).hasSize(2); assertThat(sql.get(0)).contains("delete from ec_person_phone where owner_id = ?"); assertThat(sql.get(1)).contains("delete from ec_person where id=? and version=?"); + + assertThat(eventLog()).containsExactly("preDelete", "postDelete"); } private void jsonToFrom(EcPerson foundFirst) {