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 57b18f312..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,6 +34,7 @@ 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; @@ -47,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(); } /** @@ -74,7 +76,7 @@ public final class SaveManyBeans extends SaveManyBase { resetModifyState(); } } else { - if (isModifyListenMode() || many.hasOrderColumn()) { + if (isModifyListenMode() || hasOrderColumn) { // delete any removed beans / orphans removeAssocManyOrphans(); } @@ -91,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()); @@ -113,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) { @@ -141,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(); @@ -170,7 +166,7 @@ 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()) { @@ -185,13 +181,13 @@ public final class SaveManyBeans extends SaveManyBase { 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; } } } @@ -356,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/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++) {