[#1395] Fix flushing of creatable subtypes for singular attributes wi…

…th read-only declared types
Blazebit · Dec 27, 2021 · 5e27fa3 · 5e27fa3
1 parent e0476e4
commit 5e27fa3
Show file tree

Hide file tree

Showing 14 changed files with 163 additions and 113 deletions.
diff --git a/...src/main/java/com/blazebit/persistence/view/impl/entity/CreateOnlyViewToEntityMapper.java b/...src/main/java/com/blazebit/persistence/view/impl/entity/CreateOnlyViewToEntityMapper.java
diff --git a/...w/impl/src/main/java/com/blazebit/persistence/view/impl/update/EntityViewUpdaterImpl.java b/...w/impl/src/main/java/com/blazebit/persistence/view/impl/update/EntityViewUpdaterImpl.java
@@ -39,7 +39,6 @@
 import com.blazebit.persistence.view.impl.collection.MapInstantiatorImplementor;
 import com.blazebit.persistence.view.impl.collection.RecordingList;
 import com.blazebit.persistence.view.impl.collection.RecordingMap;
-import com.blazebit.persistence.view.impl.entity.CreateOnlyViewToEntityMapper;
 import com.blazebit.persistence.view.impl.entity.EmbeddableUpdaterBasedViewToEntityMapper;
 import com.blazebit.persistence.view.impl.entity.EntityIdLoader;
 import com.blazebit.persistence.view.impl.entity.EntityLoader;
@@ -1232,23 +1231,7 @@ public void remove(UpdateContext context, Object viewId) {
                                 ownerMapping,
                                 localCache
                             );
-                        } else if (shouldFlushPersists) {
-                            viewToEntityMapper = new CreateOnlyViewToEntityMapper(
-                                    attributeLocation,
-                                    evm,
-                                    subviewType.getJavaType(),
-                                    readOnlyAllowedSubtypes,
-                                    persistAllowedSubtypes,
-                                    updateAllowedSubtypes,
-                                    null,
-                                    null,
-                                    entityIdAccessor,
-                                    shouldFlushPersists,
-                                    owner,
-                                    ownerMapping,
-                                    localCache
-                            );
-                        } else if (shouldPassThrough(evm, viewType, attribute)) {
+                        } else if (!shouldFlushPersists && shouldPassThrough(evm, viewType, attribute)) {
                             viewToEntityMapper = new LoadOnlyViewToEntityMapper(
                                 EntityLoaders.referenceLoaderForAttribute(evm, localCache, subviewType, attribute),
                                 subviewIdAccessor,
@@ -1263,7 +1246,7 @@ public void remove(UpdateContext context, Object viewId) {
                                 persistAllowedSubtypes,
                                 updateAllowedSubtypes,
                                 EntityLoaders.referenceLoaderForAttribute(evm, localCache, subviewType, attribute),
-                                null,
+                                subviewIdAccessor,
                                 entityIdAccessor,
                                 shouldFlushPersists,
                                 owner,

diff --git a/.../java/com/blazebit/persistence/view/impl/update/flush/AbstractPluralAttributeFlusher.java b/.../java/com/blazebit/persistence/view/impl/update/flush/AbstractPluralAttributeFlusher.java
@@ -425,9 +425,15 @@ protected final DirtyAttributeFlusher<X, E, V> getElementOnlyFlusher(UpdateConte
         // Except when we use a query strategy here, we'd rather use update queries to do the update
         // We don't fetch if the force entity mode is active because that means, an entity is already given
         if (flushStrategy == FlushStrategy.ENTITY && !context.isForceEntity()) {
-            return partialFlusher(true, PluralFlushOperation.ELEMENT_ONLY, Collections.EMPTY_LIST, elementFlushers);
+            if (actions.isEmpty()) {
+                return partialFlusher(true, PluralFlushOperation.ELEMENT_ONLY, Collections.EMPTY_LIST, elementFlushers);
+            }
+            return partialFlusher(true, PluralFlushOperation.COLLECTION_REPLAY_AND_ELEMENT, actions, elementFlushers);
         } else {
-            return partialFlusher(false, PluralFlushOperation.ELEMENT_ONLY, Collections.EMPTY_LIST, elementFlushers);
+            if (actions.isEmpty()) {
+                return partialFlusher(false, PluralFlushOperation.ELEMENT_ONLY, Collections.EMPTY_LIST, elementFlushers);
+            }
+            return partialFlusher(false, PluralFlushOperation.COLLECTION_REPLAY_AND_ELEMENT, actions, elementFlushers);
         }
     }
 

diff --git a/.../main/java/com/blazebit/persistence/view/impl/update/flush/CompositeAttributeFlusher.java b/.../main/java/com/blazebit/persistence/view/impl/update/flush/CompositeAttributeFlusher.java
@@ -432,7 +432,7 @@ public Query flushQuery(UpdateContext context, String parameterPrefix, UpdateQue
             Object[] initialState = ((DirtyStateTrackable) value).$$_getInitialState();
             context.getInitialStateResetter().addState(initialState, initialState.clone());
 
-            if (query == null) {
+            if (query == null && queryFactory != null) {
                 query = queryFactory.createUpdateQuery(context, (MutableStateTrackable) ownerView, ownerFlusher);
             }
             for (int i = 0; i < state.length; i++) {

diff --git a/...ain/java/com/blazebit/persistence/view/impl/update/flush/FusedCollectionIndexActions.java b/...ain/java/com/blazebit/persistence/view/impl/update/flush/FusedCollectionIndexActions.java
@@ -18,6 +18,7 @@
 
 import com.blazebit.persistence.view.impl.collection.ListAction;
 import com.blazebit.persistence.view.impl.update.UpdateContext;
+import com.blazebit.persistence.view.spi.type.DirtyTracker;
 
 import java.util.ArrayList;
 import java.util.Collection;
@@ -82,7 +83,7 @@ public FusedCollectionIndexActions(List<? extends ListAction<?>> collectionActio
                     appendedObjectMap.add(entry);
                 } else {
                     ReplaceOperation replaceOperation = new ReplaceOperation(index, entry.getKey());
-                    if (!addTranslateOperation(translateOperations, index, Integer.MAX_VALUE, 1, null, replaceOperation)) {
+                    if (addTranslateOperation(translateOperations, index, Integer.MAX_VALUE, 1, null, replaceOperation)) {
                         replaceOperations.add(replaceOperation);
                     }
                 }
@@ -194,7 +195,8 @@ private static boolean addTranslateOperation(List<IndexTranslateOperation> trans
                         // A remove followed an insert or the other way round allow to remove the translation operation
                         translateOperations.remove(i);
                         replaceOperation.oldObject = indexTranslateOperation.removeOperations.get(0).removedObject;
-                        return replaceOperation.oldObject == replaceOperation.newObject;
+                        Object value = replaceOperation.newObject;
+                        return replaceOperation.oldObject != value || value instanceof DirtyTracker && ((DirtyTracker) value).$$_isDirty();
                     } else if (indexDiff == 1 && indexTranslateOperation.endIndex == endIndex) {
                         // Neighbouring index translations with the same endIndex are merged
                         List<RemoveOperation> newList;
@@ -206,18 +208,18 @@ private static boolean addTranslateOperation(List<IndexTranslateOperation> trans
                             newList.add(removeOperation);
                         }
                         translateOperations.set(i, new IndexTranslateOperation(Math.min(indexTranslateOperation.startIndex, startIndex), endIndex, indexTranslateOperation.offset + offset, newList));
-                        return false;
+                        return true;
                     } else {
                         translateOperations.set(i, new IndexTranslateOperation(indexTranslateOperation.startIndex, startIndex, indexTranslateOperation.offset, indexTranslateOperation.removeOperations));
                         translateOperations.add(i + 1, new IndexTranslateOperation(startIndex, endIndex, offset, removeOperation == null ? new ArrayList<RemoveOperation>() : new ArrayList<>(Collections.singletonList(removeOperation))));
-                        return false;
+                        return true;
                     }
                 }
             }
 
             translateOperations.add(new IndexTranslateOperation(startIndex, endIndex, offset, removeOperation == null ? new ArrayList<RemoveOperation>() : new ArrayList<>(Collections.singletonList(removeOperation))));
         }
-        return false;
+        return true;
     }
 
     private static int applyIndexTranslations(List<IndexTranslateOperation> indexTranslateOperations, int index) {

diff --git a/...ain/java/com/blazebit/persistence/view/impl/update/flush/IndexedListAttributeFlusher.java b/...ain/java/com/blazebit/persistence/view/impl/update/flush/IndexedListAttributeFlusher.java
@@ -348,13 +348,13 @@ protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescrip
                     // By default, since the view is dirty, we start with the state UPDATED and go through state transitions
                     // based on the containment of the view in the added/removed objects collections of the actions
                     EntryState state = EntryState.UPDATED;
-                    Object replacedObject = null;
+                    Object replacedObject = element;
                     for (CollectionAction<?> action : actions) {
                         Collection<Object> removedObjects = action.getRemovedObjects();
                         if (identityContains(removedObjects, element)) {
                             if (identityContains(action.getAddedObjects(), element)) {
                                 // This is a ListSetAction where the old and new object are the same instances
-                                replacedObject = null;
+                                replacedObject = element;
                                 state = EntryState.UPDATED;
                             } else {
                                 state = state.onRemove();
@@ -373,9 +373,9 @@ protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescrip
                     // If the element was UPDATED and there is no replacedObject,
                     // this means that this really was just a mutation of the view
                     // and there is no action that would flush the object changes already
-                    if (state == EntryState.UPDATED && replacedObject == null) {
+                    if (state == EntryState.UPDATED && replacedObject == element) {
                         // Using last = false is intentional to actually get a proper update instead of a delete and insert
-                        actions.add(new ListSetAction<>(i, false, element, null));
+                        actions.add(new ListSetAction<>(i, false, element, element));
                     }
                 }
             }

diff --git a/...rc/main/java/com/blazebit/persistence/view/impl/update/flush/SubviewAttributeFlusher.java b/...rc/main/java/com/blazebit/persistence/view/impl/update/flush/SubviewAttributeFlusher.java
@@ -604,7 +604,11 @@ public DirtyAttributeFlusher<SubviewAttributeFlusher<E, V>, E, V> getDirtyFlushe
             boolean needsUpdate = update && !viewIdEqual(initial, current);
             // If the reference changed, we don't need to load the old reference
             if (initial != current && needsUpdate) {
-                return new SubviewAttributeFlusher<>(this, false, (V) current, needsUpdate, null);
+                DirtyAttributeFlusher<?, E, V> flusher = null;
+                if (current instanceof MutableStateTrackable) {
+                    flusher = (DirtyAttributeFlusher<?, E, V>) viewToEntityMapper.getNestedDirtyFlusher(context, (MutableStateTrackable) current, this);
+                }
+                return new SubviewAttributeFlusher<>(this, false, (V) current, needsUpdate, flusher);
             }
 
             // If the initial and current reference are null, no need to do anything further

diff --git a/.../update/flatview/simple/mutable/EntityViewUpdateSimpleMutableFlatViewCollectionsTest.java b/.../update/flatview/simple/mutable/EntityViewUpdateSimpleMutableFlatViewCollectionsTest.java
@@ -20,11 +20,14 @@
 import com.blazebit.persistence.testsuite.base.jpa.category.NoDatanucleus;
 import com.blazebit.persistence.testsuite.base.jpa.category.NoEclipselink;
 import com.blazebit.persistence.testsuite.entity.Document;
+import com.blazebit.persistence.testsuite.entity.IntIdEntity;
 import com.blazebit.persistence.testsuite.entity.NameObject;
 import com.blazebit.persistence.view.FlushMode;
 import com.blazebit.persistence.view.FlushStrategy;
 import com.blazebit.persistence.view.spi.EntityViewConfiguration;
 import com.blazebit.persistence.view.testsuite.update.AbstractEntityViewUpdateDocumentTest;
+import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.IntIdEntityCreateView;
+import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.IntIdEntityIdView;
 import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.UpdatableDocumentWithCollectionsView;
 import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.UpdatableNameObjectView;
 import org.junit.Assert;
@@ -61,6 +64,8 @@ public static Object[][] combinations() {
     @Override
     protected void registerViewTypes(EntityViewConfiguration cfg) {
         cfg.addEntityView(UpdatableNameObjectView.class);
+        cfg.addEntityView(IntIdEntityIdView.class);
+        cfg.addEntityView(IntIdEntityCreateView.class);
     }
 
     @Override
@@ -172,6 +177,43 @@ public void testUpdateRemoveNull() {
         assertSubviewEquals(doc1.getNames(), docView.getNames());
     }
 
+    @Test
+    public void testAddCollectionElementWithNewObject() {
+        // Given
+        final UpdatableDocumentWithCollectionsView docView = getDoc1View();
+        clearQueries();
+
+        // When
+        IntIdEntityCreateView intIdEntity = evm.create(IntIdEntityCreateView.class);
+        intIdEntity.setName("test");
+        docView.getNames().get(0).setIntIdEntity(intIdEntity);
+        update(docView);
+
+        // Then
+        // Assert that the document and the people are loaded i.e. a full fetch
+        // Finally the person is updated because the intIdEntity changed
+        AssertStatementBuilder builder = assertUnorderedQuerySequence();
+
+        if (!isQueryStrategy()) {
+            fullFetch(builder);
+        }
+
+        if (version || isFullMode() && isQueryStrategy()) {
+            builder.update(Document.class);
+        }
+        if (isFullMode() || !isQueryStrategy() && !supportsIndexedInplaceUpdate()) {
+            builder.delete(Document.class, "names")
+                    .insert(Document.class, "names");
+        } else {
+            builder.update(Document.class, "names");
+        }
+        builder.insert(IntIdEntity.class);
+        builder.validate();
+
+        assertNoUpdateAndReload(docView);
+        assertEquals(intIdEntity.getId(), doc1.getNames().get(0).getIntIdEntity().getId());
+    }
+
     public static void assertSubviewEquals(Collection<NameObject> persons, Collection<? extends UpdatableNameObjectView> personSubviews) {
         if (persons == null) {
             assertNull(personSubviews);
@@ -197,10 +239,14 @@ public static void assertSubviewEquals(Collection<NameObject> persons, Collectio
 
     @Override
     protected AssertStatementBuilder fullFetch(AssertStatementBuilder builder) {
-        return builder.assertSelect()
+        builder.assertSelect()
                 .fetching(Document.class)
                 .fetching(Document.class, "names")
                 .and();
+        if (doc1.getNames().get(0).getIntIdEntity() != null) {
+            builder.select(IntIdEntity.class);
+        }
+        return builder;
     }
 
     @Override

diff --git a/...stsuite/update/flatview/simple/mutable/EntityViewUpdateSimpleMutableFlatViewMapsTest.java b/...stsuite/update/flatview/simple/mutable/EntityViewUpdateSimpleMutableFlatViewMapsTest.java
@@ -25,6 +25,8 @@
 import com.blazebit.persistence.view.FlushStrategy;
 import com.blazebit.persistence.view.spi.EntityViewConfiguration;
 import com.blazebit.persistence.view.testsuite.update.AbstractEntityViewUpdateDocumentTest;
+import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.IntIdEntityCreateView;
+import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.IntIdEntityIdView;
 import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.UpdatableDocumentWithMapsView;
 import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.UpdatableNameObjectView;
 import org.junit.Assert;
@@ -60,6 +62,8 @@ public static Object[][] combinations() {
     @Override
     protected void registerViewTypes(EntityViewConfiguration cfg) {
         cfg.addEntityView(UpdatableNameObjectView.class);
+        cfg.addEntityView(IntIdEntityIdView.class);
+        cfg.addEntityView(IntIdEntityCreateView.class);
     }
 
     @Override

diff --git a/...w/testsuite/update/flatview/simple/mutable/EntityViewUpdateSimpleMutableFlatViewTest.java b/...w/testsuite/update/flatview/simple/mutable/EntityViewUpdateSimpleMutableFlatViewTest.java
@@ -24,6 +24,8 @@
 import com.blazebit.persistence.view.FlushStrategy;
 import com.blazebit.persistence.view.spi.EntityViewConfiguration;
 import com.blazebit.persistence.view.testsuite.update.AbstractEntityViewUpdateDocumentTest;
+import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.IntIdEntityCreateView;
+import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.IntIdEntityIdView;
 import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.UpdatableDocumentView;
 import com.blazebit.persistence.view.testsuite.update.flatview.simple.mutable.model.UpdatableNameObjectView;
 import org.junit.Assert;
@@ -54,6 +56,8 @@ public static Object[][] combinations() {
     @Override
     protected void registerViewTypes(EntityViewConfiguration cfg) {
         cfg.addEntityView(UpdatableNameObjectView.class);
+        cfg.addEntityView(IntIdEntityIdView.class);
+        cfg.addEntityView(IntIdEntityCreateView.class);
     }
 
     @Test