From c8e199177c31419b985d31fd9dc00cff1e1dc5d8 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Tue, 27 Feb 2018 15:31:26 +1300 Subject: [PATCH] #1294 - NPE when delete cascade from bean with SoftDelete to imported OneToOne without SoftDelete --- .../server/deploy/BeanPropertyAssoc.java | 7 ++ .../server/persist/DefaultPersister.java | 20 +++--- .../tests/model/softdelete/ESoftDelOneA.java | 71 +++++++++++++++++++ .../tests/model/softdelete/ESoftDelOneB.java | 57 +++++++++++++++ .../softdelete/TestSoftDeleteOtoCascade.java | 38 ++++++++++ 5 files changed, 185 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/tests/model/softdelete/ESoftDelOneA.java create mode 100644 src/test/java/org/tests/model/softdelete/ESoftDelOneB.java create mode 100644 src/test/java/org/tests/softdelete/TestSoftDeleteOtoCascade.java diff --git a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java index 499276dce..656f5d0bc 100644 --- a/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java +++ b/src/main/java/io/ebeaninternal/server/deploy/BeanPropertyAssoc.java @@ -186,6 +186,13 @@ public abstract class BeanPropertyAssoc extends BeanProperty { return targetDescriptor; } + /** + * Return true if the target side has soft delete. + */ + public boolean isTargetSoftDelete() { + return targetDescriptor.isSoftDelete(); + } + /** * Return true if REFRESH should cascade. */ diff --git a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java index 7343a8ac5..a825c1653 100644 --- a/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java +++ b/src/main/java/io/ebeaninternal/server/persist/DefaultPersister.java @@ -1080,7 +1080,7 @@ public final class DefaultPersister implements Persister { DeleteUnloadedForeignKeys unloaded = null; for (BeanPropertyAssocOne prop : expOnes) { // for soft delete check cascade type also supports soft delete - if (!softDelete || prop.getTargetDescriptor().isSoftDelete()) { + if (!softDelete || prop.isTargetSoftDelete()) { if (request.isLoadedProperty(prop)) { Object detailBean = prop.getValue(parentBean); if (detailBean != null) { @@ -1113,7 +1113,7 @@ public final class DefaultPersister implements Persister { if (ModifyListenMode.REMOVALS == many.getModifyListenMode()) { // PrivateOwned ... // if soft delete then check target also supports soft delete - if (!softDelete || many.getTargetDescriptor().isSoftDelete()) { + if (!softDelete || many.isTargetSoftDelete()) { Object details = many.getValue(parentBean); if (details instanceof BeanCollection) { Set modifyRemovals = ((BeanCollection) details).getModifyRemovals(); @@ -1257,14 +1257,18 @@ public final class DefaultPersister implements Persister { */ private void deleteAssocOne(PersistRequestBean request) { + boolean softDelete = request.isSoftDelete(); + BeanPropertyAssocOne[] ones = request.getBeanDescriptor().propertiesOneImportedDelete(); for (BeanPropertyAssocOne prop : ones) { - if (request.isLoadedProperty(prop)) { - Object detailBean = prop.getValue(request.getEntityBean()); - if (detailBean != null) { - EntityBean detail = (EntityBean) detailBean; - if (prop.hasId(detail)) { - deleteRecurse(detail, request.getTransaction(), request.isSoftDelete()); + if (!softDelete || prop.isTargetSoftDelete()) { + if (request.isLoadedProperty(prop)) { + Object detailBean = prop.getValue(request.getEntityBean()); + if (detailBean != null) { + EntityBean detail = (EntityBean) detailBean; + if (prop.hasId(detail)) { + deleteRecurse(detail, request.getTransaction(), request.isSoftDelete()); + } } } } diff --git a/src/test/java/org/tests/model/softdelete/ESoftDelOneA.java b/src/test/java/org/tests/model/softdelete/ESoftDelOneA.java new file mode 100644 index 000000000..3ecd9b044 --- /dev/null +++ b/src/test/java/org/tests/model/softdelete/ESoftDelOneA.java @@ -0,0 +1,71 @@ +package org.tests.model.softdelete; + +import io.ebean.annotation.SoftDelete; + +import javax.persistence.CascadeType; +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToOne; +import javax.persistence.Version; + +@Entity +public class ESoftDelOneA { + + @Id + long id; + + String name; + + @OneToOne(cascade = CascadeType.ALL) + ESoftDelOneB oneb; + + @SoftDelete + boolean deleted; + + @Version + long version; + + public ESoftDelOneA(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 ESoftDelOneB getOneb() { + return oneb; + } + + public void setOneb(ESoftDelOneB oneb) { + this.oneb = oneb; + } + + public boolean isDeleted() { + return deleted; + } + + public void setDeleted(boolean deleted) { + this.deleted = deleted; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/model/softdelete/ESoftDelOneB.java b/src/test/java/org/tests/model/softdelete/ESoftDelOneB.java new file mode 100644 index 000000000..b89192b31 --- /dev/null +++ b/src/test/java/org/tests/model/softdelete/ESoftDelOneB.java @@ -0,0 +1,57 @@ +package org.tests.model.softdelete; + +import javax.persistence.Entity; +import javax.persistence.Id; +import javax.persistence.OneToOne; +import javax.persistence.Version; + +@Entity +public class ESoftDelOneB { + + @Id + long id; + + String name; + + @OneToOne(mappedBy = "oneb") + ESoftDelOneA onea; + + @Version + long version; + + public ESoftDelOneB(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 ESoftDelOneA getOnea() { + return onea; + } + + public void setOnea(ESoftDelOneA onea) { + this.onea = onea; + } + + public long getVersion() { + return version; + } + + public void setVersion(long version) { + this.version = version; + } +} diff --git a/src/test/java/org/tests/softdelete/TestSoftDeleteOtoCascade.java b/src/test/java/org/tests/softdelete/TestSoftDeleteOtoCascade.java new file mode 100644 index 000000000..bad534614 --- /dev/null +++ b/src/test/java/org/tests/softdelete/TestSoftDeleteOtoCascade.java @@ -0,0 +1,38 @@ +package org.tests.softdelete; + +import io.ebean.BaseTestCase; +import io.ebean.Ebean; +import org.ebeantest.LoggedSqlCollector; +import org.junit.Test; +import org.tests.model.softdelete.ESoftDelOneA; +import org.tests.model.softdelete.ESoftDelOneB; + +import java.util.List; + +import static org.assertj.core.api.Assertions.assertThat; + +public class TestSoftDeleteOtoCascade extends BaseTestCase { + + @Test + public void test() { + + ESoftDelOneA a = new ESoftDelOneA("a1"); + ESoftDelOneB b = new ESoftDelOneB("b1"); + + a.setOneb(b); + b.setOnea(a); + + Ebean.save(a); + + LoggedSqlCollector.start(); + + // delete doesn't cascade to ESoftDelOneB as this is a + // soft delete and ESoftDelOneB doesn't have soft delete flag + Ebean.delete(a); + + List sql = LoggedSqlCollector.stop(); + assertThat(sql).hasSize(1); + assertThat(sql.get(0)).contains("update esoft_del_one_a set deleted=?, version=? where id=? and version=?"); + + } +}