From 33031d1c698e8a81d1e7a9b5cd66d22fd62778ad Mon Sep 17 00:00:00 2001 From: rob bygrave Date: Sun, 6 May 2018 20:21:10 +1200 Subject: [PATCH] #1369 - BeanListener and/or BeanPersistAdapter not fired on delete of children when cascaded --- .../server/deploy/BeanDescriptor.java | 10 +++- .../server/persist/DefaultPersister.java | 18 ++++-- src/test/java/org/tests/delete/DcDetail.java | 51 ++++++++++++++++ .../java/org/tests/delete/DcListener.java | 28 +++++++++ src/test/java/org/tests/delete/DcMaster.java | 59 +++++++++++++++++++ .../delete/TestDeleteCascadeWithListener.java | 51 ++++++++++++++++ 6 files changed, 211 insertions(+), 6 deletions(-) create mode 100644 src/test/java/org/tests/delete/DcDetail.java create mode 100644 src/test/java/org/tests/delete/DcListener.java create mode 100644 src/test/java/org/tests/delete/DcMaster.java create mode 100644 src/test/java/org/tests/delete/TestDeleteCascadeWithListener.java diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java index 1bc379e7e..e34020a10 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanDescriptor.java @@ -1723,7 +1723,15 @@ public class BeanDescriptor implements BeanType, STreeType { * associated L2 bean caching. */ public boolean isDeleteByStatement() { - return deleteRecurseSkippable && !isBeanCaching(); + return persistListener == null + && persistController == null + && deleteRecurseSkippable && !isBeanCaching(); + } + + public boolean isDeleteByBulk() { + return persistListener == null + && persistController == null + && propertiesManyToMany.length == 0; } /** diff --git a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index c3cb2dcda..f7688d7a5 100644 --- a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -588,10 +588,18 @@ public final class DefaultPersister implements Persister { } } - private void deleteList(List beanList, Transaction t, boolean softDelete) { + private void deleteList(List beanList, SpiTransaction t, boolean softDelete, boolean children) { + if (children) { + t.depth(-1); + t.checkBatchEscalationOnCollection(); + } for (Object aBeanList : beanList) { deleteRecurse((EntityBean) aBeanList, t, softDelete); } + if (children) { + t.flushBatchOnCollection(); + t.depth(+1); + } } /** @@ -675,7 +683,7 @@ public final class DefaultPersister implements Persister { t.logSummary("-- DeleteById of " + descriptor.getName() + " ids[" + idList + "] requires fetch of foreign key values"); } List beanList = server.findList(q, t); - deleteList(beanList, t, softDelete); + deleteList(beanList, t, softDelete, false); return beanList.size(); } else { @@ -1012,7 +1020,7 @@ public final class DefaultPersister implements Persister { // cascade delete the beans in the collection BeanDescriptor targetDesc = many.getTargetDescriptor(); if (!softDelete || targetDesc.isSoftDelete()) { - if (targetDesc.isDeleteRecurseSkippable() && !targetDesc.isBeanCaching()) { + if (targetDesc.isDeleteByStatement()) { // Just delete all the children with one statement IntersectionRow intRow = many.buildManyDeleteChildren(parentBean, excludeDetailIds); SqlUpdate sqlDelete = intRow.createDelete(server, softDelete); @@ -1037,13 +1045,13 @@ public final class DefaultPersister implements Persister { */ private void deleteChildrenById(SpiTransaction t, BeanDescriptor targetDesc, List childIds, boolean softDelete) { - if (targetDesc.propertiesManyToMany().length > 0) { + if (!targetDesc.isDeleteByBulk()) { // convert into a list of reference objects and perform delete by object List refList = new ArrayList<>(childIds.size()); for (Object id : childIds) { refList.add(targetDesc.createReference(id, null)); } - deleteList(refList, t, softDelete); + deleteList(refList, t, softDelete, true); } else { // perform delete by statement if possible diff --git a/src/test/java/org/tests/delete/DcDetail.java b/src/test/java/org/tests/delete/DcDetail.java new file mode 100644 index 000000000..e49cc58bc --- /dev/null +++ b/src/test/java/org/tests/delete/DcDetail.java @@ -0,0 +1,51 @@ +package org.tests.delete; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.ManyToOne; + +@Entity +public class DcDetail { + + @Id + long id; + + @ManyToOne + DcMaster master; + + String description; + + long version; + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public DcMaster getMaster() { + return master; + } + + public void setMaster(DcMaster master) { + this.master = master; + } + + public String getDescription() { + return description; + } + + public void setDescription(String description) { + this.description = description; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/delete/DcListener.java b/src/test/java/org/tests/delete/DcListener.java new file mode 100644 index 000000000..5e4191ab0 --- /dev/null +++ b/src/test/java/org/tests/delete/DcListener.java @@ -0,0 +1,28 @@ +package org.tests.delete; + +import io.ebean.event.AbstractBeanPersistListener; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; + +public class DcListener extends AbstractBeanPersistListener { + + static List deleted = Collections.synchronizedList(new ArrayList<>()); + + @Override + public boolean isRegisterFor(Class cls) { + return cls.equals(DcDetail.class); + } + + @Override + public void deleted(Object bean) { + deleted.add(bean); + } + + static List deletedBeans() { + List copy = new ArrayList<>(deleted); + deleted.clear(); + return copy; + } +} diff --git a/src/test/java/org/tests/delete/DcMaster.java b/src/test/java/org/tests/delete/DcMaster.java new file mode 100644 index 000000000..e1c8d0144 --- /dev/null +++ b/src/test/java/org/tests/delete/DcMaster.java @@ -0,0 +1,59 @@ +package org.tests.delete; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToMany; +import javax.persistence.Version; +import java.util.List; + +@Entity +public class DcMaster { + + @Id + long id; + + String name; + + @OneToMany(mappedBy = "master", cascade = CascadeType.ALL) + List details; + + @Version + long version; + + public DcMaster(String name) { + this.name = name; + } + + public long getId() { + return id; + } + + public void setId(long id) { + this.id = id; + } + + public String getName() { + return name; + } + + public void setName(String name) { + this.name = name; + } + + public List getDetails() { + return details; + } + + public void setDetails(List details) { + this.details = details; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/delete/TestDeleteCascadeWithListener.java b/src/test/java/org/tests/delete/TestDeleteCascadeWithListener.java new file mode 100644 index 000000000..56d47e5ea --- /dev/null +++ b/src/test/java/org/tests/delete/TestDeleteCascadeWithListener.java @@ -0,0 +1,51 @@ +package org.tests.delete; + +import io.ebean.BaseTestCase; +import io.ebean.Ebean; +import org.ebeantest.LoggedSqlCollector; +import org.junit.Test; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestDeleteCascadeWithListener extends BaseTestCase { + + @Test + public void test() { + + DcMaster m0 = new DcMaster("m0"); + m0.getDetails().add(new DcDetail()); + m0.getDetails().add(new DcDetail()); + m0.getDetails().add(new DcDetail()); + + Ebean.save(m0); + + DcMaster found = Ebean.find(DcMaster.class, m0.getId()); + + LoggedSqlCollector.start(); + Ebean.delete(found); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(5); + + assertThat(sql.get(0)).contains("select t0.id from dc_detail t0 where master_id=?"); + assertThat(sql.get(1)).contains("delete from dc_detail where id=?"); + assertThat(sql.get(3)).contains("delete from dc_detail where id=?"); + assertThat(sql.get(4)).contains("delete from dc_master where id=? and version=?"); + + awaitListenerPropagation(); + + List beans = DcListener.deletedBeans(); + assertThat(beans).hasSize(3); + } + + private void awaitListenerPropagation() { + try { + Thread.sleep(200); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException(e); + } + } +}