diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java index f6f2f241c..1d1008765 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/persist/SaveManyBeans.java @@ -24,7 +24,7 @@ import static io.ebeaninternal.server.persist.DmlUtil.isNullOrZero; /** * Saves the details for a OneToMany or ManyToMany relationship (entity beans). */ -public final class SaveManyBeans extends SaveManyBase { +final class SaveManyBeans extends SaveManyBase { private final boolean cascade; private final boolean publish; @@ -34,7 +34,9 @@ public final class SaveManyBeans extends SaveManyBase { private final DeleteMode deleteMode; private final boolean untouchedBeanCollection; private final Collection collection; + private final boolean hasOrderColumn; private int sortOrder; + private boolean insertAllChildren; SaveManyBeans(DefaultPersister persister, boolean insertedParent, BeanPropertyAssocMany many, EntityBean parentBean, PersistRequestBean request) { super(persister, insertedParent, many, parentBean, request); @@ -46,6 +48,7 @@ public final class SaveManyBeans extends SaveManyBase { this.deleteMode = targetDescriptor.isSoftDelete() ? DeleteMode.SOFT : DeleteMode.HARD; this.untouchedBeanCollection = untouchedBeanCollection(); this.collection = cascade ? BeanCollectionUtil.getActualEntries(value) : null; + this.hasOrderColumn = many.hasOrderColumn(); } /** @@ -73,7 +76,7 @@ public final class SaveManyBeans extends SaveManyBase { resetModifyState(); } } else { - if (isModifyListenMode() || many.hasOrderColumn()) { + if (isModifyListenMode() || hasOrderColumn) { // delete any removed beans / orphans removeAssocManyOrphans(); } @@ -90,6 +93,7 @@ public final class SaveManyBeans extends SaveManyBase { private boolean isSaveIntersection() { if (!many.isManyToMany()) { + // OneToMany JoinTable return true; } return transaction.isSaveAssocManyIntersection(many.intersectionTableJoin().getTable(), many.descriptor().rootName()); @@ -112,26 +116,22 @@ public final class SaveManyBeans extends SaveManyBase { private void processDetails() { BeanProperty orderColumn = null; - boolean hasOrderColumn = many.hasOrderColumn(); if (hasOrderColumn) { if (!insertedParent && canSkipForOrderColumn() && saveRecurseSkippable) { return; } orderColumn = targetDescriptor.orderColumn(); } - if (insertedParent) { // performance optimisation for large collections targetDescriptor.preAllocateIds(collection.size()); } - if (!insertedParent && many.isOrphanRemoval() && request.isForcedUpdate()) { // collect the Id's (to exclude from deleteManyDetails) List detailIds = collectIds(collection, targetDescriptor, isMap); // deleting missing children - children not in our collected detailIds persister.deleteManyDetails(transaction, many.descriptor(), parentBean, many, detailIds, deleteMode); } - transaction.depth(+1); saveAllBeans(orderColumn); if (hasOrderColumn) { @@ -140,17 +140,14 @@ public final class SaveManyBeans extends SaveManyBase { transaction.depth(-1); } - private void saveAllBeans(BeanProperty orderColumn) { - // if a map, then we get the key value and - // set it to the appropriate property on the - // detail bean before we save it + private void saveAllBeans(final BeanProperty orderColumn) { Object mapKeyValue = null; boolean skipSavingThisBean; - + boolean clearedParent = false; for (Object detailBean : collection) { sortOrder++; if (isMap) { - // its a map so need the key and value + // a map so need the key and value Map.Entry entry = (Map.Entry) detailBean; mapKeyValue = entry.getKey(); detailBean = entry.getValue(); @@ -169,25 +166,28 @@ public final class SaveManyBeans extends SaveManyBase { ebi.setDirty(true); } } - if (targetDescriptor.isReference(ebi) && originalOrder == 0) { + if (originalOrder == 0 && targetDescriptor.isReference(ebi)) { // we can skip this one skipSavingThisBean = true; } else if (ebi.isNewOrDirty()) { skipSavingThisBean = false; // set the parent bean to detailBean many.setJoinValuesToChild(parentBean, detail, mapKeyValue); + } else if (insertAllChildren) { + ebi.setNew(); + skipSavingThisBean = false; + many.setJoinValuesToChild(parentBean, detail, mapKeyValue); } else { - // unmodified so skip depending on prop.isSaveRecurseSkippable(); skipSavingThisBean = saveRecurseSkippable; } } - if (!skipSavingThisBean) { persister.saveRecurse(detail, transaction, parentBean, request.flags()); - if (many.hasOrderColumn()) { - // Clear the bean from the PersistenceContext (L1 cache), because the order of referenced beans might have changed + if (hasOrderColumn && !clearedParent) { + // Clear the parent bean from the PersistenceContext (L1 cache), because the order of referenced beans might have changed final BeanDescriptor beanDescriptor = many.descriptor(); beanDescriptor.contextClear(transaction.getPersistenceContext(), beanDescriptor.getId(parentBean)); + clearedParent = true; } } } @@ -342,6 +342,7 @@ public final class SaveManyBeans extends SaveManyBase { if (!(value instanceof BeanCollection)) { if (!insertedParent && cascade && isChangedProperty()) { persister.addToFlushQueue(many.deleteByParentId(request.beanId(), null), transaction, 0); + insertAllChildren = true; } } else { BeanCollection c = (BeanCollection) value; @@ -351,7 +352,7 @@ public final class SaveManyBeans extends SaveManyBase { c.setModifyListening(many.modifyListenMode()); } // We must not reset when we still have to update other entities in the collection and set their new orderColumn value - if (!many.hasOrderColumn()) { + if (!hasOrderColumn) { c.modifyReset(); } if (modifyRemovals != null && !modifyRemovals.isEmpty()) { diff --git a/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java b/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java new file mode 100644 index 000000000..8dbedc9b3 --- /dev/null +++ b/ebean-test/src/test/java/org/tests/cascade/TestOrphanCollectionReplacement.java @@ -0,0 +1,51 @@ +package org.tests.cascade; + +import io.ebean.DB; +import io.ebean.xtest.BaseTestCase; +import org.junit.jupiter.api.Test; + +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; + +import static java.util.Objects.requireNonNull; +import static org.junit.jupiter.api.Assertions.assertEquals; + +class TestOrphanCollectionReplacement extends BaseTestCase { + + @Test + void replaceCollection_whenOrphan_expect_forcedInsert() { + long parentId; + { // setup + List children = new ArrayList<>(); + children.add(new COOneMany("c0")); + children.add(new COOneMany("c1")); + + COOne parent = new COOne("p0"); + parent.setChildren(children); + + DB.save(parent); + parentId = parent.getId(); + } + + { // act + COOne fetchedParent = DB.find(COOne.class, parentId); + assert fetchedParent != null; + + COOneMany role = new COOneMany("c2"); + + List filtered = fetchedParent.getChildren().stream().filter(r -> "c0".equals(r.getName())).collect(Collectors.toList()); + + List updatedRoles = new ArrayList<>(); + updatedRoles.addAll(filtered); + updatedRoles.addAll(List.of(role)); + fetchedParent.setChildren(updatedRoles); + + DB.save(fetchedParent); + } + + COOne fetchedUser2 = DB.find(COOne.class, parentId); + requireNonNull(fetchedUser2); + assertEquals(2, fetchedUser2.getChildren().size()); + } +} diff --git a/ebean-test/src/test/java/org/tests/order/TestOrderColumn.java b/ebean-test/src/test/java/org/tests/order/TestOrderColumn.java index a6e198735..54587376d 100644 --- a/ebean-test/src/test/java/org/tests/order/TestOrderColumn.java +++ b/ebean-test/src/test/java/org/tests/order/TestOrderColumn.java @@ -10,10 +10,10 @@ import java.util.List; import static org.assertj.core.api.Assertions.assertThat; -public class TestOrderColumn extends TransactionalTestCase { +class TestOrderColumn extends TransactionalTestCase { @Test - public void testOrderColumnInheritance() { + void testOrderColumnInheritance() { final OrderMaster master = new OrderMaster(); for (int i = 0; i < 5; i++) { @@ -33,7 +33,7 @@ public class TestOrderColumn extends TransactionalTestCase { } @Test - public void testOrderColumnSortChange() { + void testOrderColumnSortChange() { final OrderMaster master = new OrderMaster(); for (int i = 0; i < 5; i++) { @@ -62,7 +62,7 @@ public class TestOrderColumn extends TransactionalTestCase { } @Test - public void testModifyTree() { + void testModifyTree() { final OrderMaster master = new OrderMaster(); for (int i = 0; i < 5; i++) { @@ -108,7 +108,7 @@ public class TestOrderColumn extends TransactionalTestCase { } @Test - public void testRemoveElement() { + void testRemoveElement() { final OrderMaster master = new OrderMaster(); for (int i = 0; i < 5; i++) {