From 13e0be47de23435e45c4457c1bd619fd4d6dca12 Mon Sep 17 00:00:00 2001 From: Christian Beikov Date: Tue, 18 Dec 2018 00:30:59 +0100 Subject: [PATCH] Allow re-add to map, handle flat view dirty only, make empty embeddable in non-indexed collection null --- .../view/impl/collection/RecordingMap.java | 15 ++++- .../view/impl/mapper/ViewMapper.java | 1 + .../ViewTypeObjectBuilderTemplate.java | 4 +- .../flush/AbstractPluralAttributeFlusher.java | 58 ++++++++++++++++++- .../flush/CollectionAttributeFlusher.java | 2 +- .../impl/update/flush/FusedMapActions.java | 11 +++- .../flush/IndexedListAttributeFlusher.java | 22 ++++--- .../update/flush/MapAttributeFlusher.java | 50 +++++++--------- 8 files changed, 118 insertions(+), 45 deletions(-) diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/collection/RecordingMap.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/collection/RecordingMap.java index 039c364dd8..da516dd0ce 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/collection/RecordingMap.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/collection/RecordingMap.java @@ -629,7 +629,6 @@ protected final void addAction(MapAction action) { this.removedElements = new IdentityHashMap<>(); } - // addAction optimizes actions by figuring converting to physical changes if (optimize) { action.addAction(actions, addedKeys, removedKeys, addedElements, removedElements); @@ -642,7 +641,12 @@ protected final void addAction(MapAction action) { if (this.removedKeys.remove(o) == null) { if (this.addedKeys.put((K) o, (K) o) == null) { if (parent != null && o instanceof BasicDirtyTracker) { - ((BasicDirtyTracker) o).$$_setParent(this, 1); + // Check if it was replaced by itself + if (removedKeys.remove(o)) { + this.addedKeys.remove(o); + } else { + ((BasicDirtyTracker) o).$$_setParent(this, 1); + } } } } else { @@ -670,7 +674,12 @@ protected final void addAction(MapAction action) { if (this.removedElements.remove(o) == null) { if (this.addedElements.put((V) o, (V) o) == null) { if (parent != null && o instanceof BasicDirtyTracker) { - ((BasicDirtyTracker) o).$$_setParent(this, 2); + // Check if it was replaced by itself + if (removedElements.remove(o)) { + this.addedElements.remove(o); + } else { + ((BasicDirtyTracker) o).$$_setParent(this, 2); + } } } } else { diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/mapper/ViewMapper.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/mapper/ViewMapper.java index 210232dc55..f3cf776596 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/mapper/ViewMapper.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/mapper/ViewMapper.java @@ -190,6 +190,7 @@ public T map(S source) { } } T result = objectInstantiator.newInstance(tuple); + // TODO: copy initial state, on create new, mark everything as dirty if (dirtyMapping != null && source instanceof DirtyTracker) { DirtyTracker oldDirtyTracker = (DirtyTracker) source; DirtyTracker dirtyTracker = (DirtyTracker) result; diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/objectbuilder/ViewTypeObjectBuilderTemplate.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/objectbuilder/ViewTypeObjectBuilderTemplate.java index 3df20f6c5f..c5c34c1de4 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/objectbuilder/ViewTypeObjectBuilderTemplate.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/objectbuilder/ViewTypeObjectBuilderTemplate.java @@ -526,7 +526,9 @@ private void applyMapping(AbstractAttribute attribute, String parentAttrib } else { MappingAttribute mappingAttribute = (MappingAttribute) attribute; ManagedViewTypeImplementor managedViewType = (ManagedViewTypeImplementor) pluralAttribute.getElementType(); - applySubviewMapping(mappingAttribute, attributePath, newTupleIdDescriptor, managedViewType, mapperBuilder, embeddingViewJpqlMacro, false, managedViewType instanceof ViewType); + // Obviously, we produce null if the object type is identifiable i.e. a ViewType and it is empty = null id + // Additionally, we also consider empty embeddables as null when we have a non-indexed collection so we can filter out these elements + applySubviewMapping(mappingAttribute, attributePath, newTupleIdDescriptor, managedViewType, mapperBuilder, embeddingViewJpqlMacro, false, managedViewType instanceof ViewType || !listKey && !mapKey); } } else if (mapKey) { MappingAttribute mappingAttribute = (MappingAttribute) attribute; diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/AbstractPluralAttributeFlusher.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/AbstractPluralAttributeFlusher.java index 550f7bfc0f..16dffb5698 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/AbstractPluralAttributeFlusher.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/AbstractPluralAttributeFlusher.java @@ -29,6 +29,7 @@ import javax.persistence.EntityManager; import javax.persistence.Query; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; import java.util.List; @@ -363,7 +364,17 @@ protected final void persistIfNeeded(EntityManager em, Object object, BasicUserT protected abstract boolean isIndexed(); - protected abstract void addElementFlushActions(UpdateContext context, TypeDescriptor elementDescriptor, List actions, V current); + protected abstract void addFlatViewElementFlushActions(UpdateContext context, TypeDescriptor elementDescriptor, List actions, V current); + + protected static boolean identityContains(Collection addedElements, MutableStateTrackable element) { + for (Object addedElement : addedElements) { + if (addedElement == element) { + return true; + } + } + + return false; + } @SuppressWarnings("unchecked") protected final DirtyAttributeFlusher getElementOnlyFlusher(UpdateContext context, V current) { @@ -476,7 +487,7 @@ protected final boolean determineElementFlushers(UpdateContext context, TypeDesc } } else { if (typeDescriptor.supportsDirtyCheck() && !typeDescriptor.isIdentifiable() && isIndexed()) { - addElementFlushActions(context, typeDescriptor, (List) actions, current); + addFlatViewElementFlushActions(context, typeDescriptor, (List) actions, current); } else { for (Object o : values) { if (o instanceof MutableStateTrackable) { @@ -529,6 +540,49 @@ protected final boolean determineElementFlushers(UpdateContext context, TypeDesc protected abstract AbstractPluralAttributeFlusher partialFlusher(boolean fetch, PluralFlushOperation operation, List collectionActions, List> elementFlushers); + /** + * @author Christian Beikov + * @since 1.4.0 + */ + protected static enum EntryState { + EXISTED { + @Override + EntryState onAdd() { + return EXISTED; + } + + @Override + EntryState onRemove() { + return REMOVED; + } + }, + ADDED { + @Override + EntryState onAdd() { + return ADDED; + } + + @Override + EntryState onRemove() { + return EXISTED; + } + }, + REMOVED { + @Override + EntryState onAdd() { + return EXISTED; + } + + @Override + EntryState onRemove() { + return REMOVED; + } + }; + + abstract EntryState onAdd(); + abstract EntryState onRemove(); + } + /** * @author Christian Beikov * @since 1.2.0 diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/CollectionAttributeFlusher.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/CollectionAttributeFlusher.java index b2d809791c..7128feb7ec 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/CollectionAttributeFlusher.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/CollectionAttributeFlusher.java @@ -497,7 +497,7 @@ protected boolean isIndexed() { } @Override - protected void addElementFlushActions(UpdateContext context, TypeDescriptor elementDescriptor, List> actions, V current) { + protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescriptor elementDescriptor, List> actions, V current) { throw new UnsupportedOperationException("Not indexed!"); } diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/FusedMapActions.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/FusedMapActions.java index 5f82dbc1e0..2b9db2bbe3 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/FusedMapActions.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/FusedMapActions.java @@ -18,6 +18,7 @@ import com.blazebit.persistence.view.impl.collection.MapAction; import com.blazebit.persistence.view.impl.entity.ViewToEntityMapper; +import com.blazebit.persistence.view.impl.proxy.DirtyTracker; import com.blazebit.persistence.view.impl.update.UpdateContext; import java.util.ArrayList; @@ -69,9 +70,13 @@ public FusedMapActions(ViewToEntityMapper keyViewToEntityMapper, List> actions, V current) { + protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescriptor typeDescriptor, List> actions, V current) { final ViewToEntityMapper mapper = typeDescriptor.getViewToEntityMapper(); - OUTER: for (int i = 0; i < current.size(); i++) { + for (int i = 0; i < current.size(); i++) { Object o = current.get(i); if (o instanceof MutableStateTrackable) { MutableStateTrackable element = (MutableStateTrackable) o; @SuppressWarnings("unchecked") DirtyAttributeFlusher flusher = (DirtyAttributeFlusher) (DirtyAttributeFlusher) mapper.getNestedDirtyFlusher(context, element, (DirtyAttributeFlusher) null); if (flusher != null) { - // Skip the element flusher if the element was already added before + EntryState state = EntryState.EXISTED; for (CollectionAction action : actions) { - if (action.getAddedObjects().contains(element)) { - continue OUTER; + if (identityContains(action.getRemovedObjects(), element)) { + state = state.onRemove(); + if (identityContains(action.getAddedObjects(), element)) { + state = state.onAdd(); + } + } else if (identityContains(action.getAddedObjects(), element)) { + state = state.onAdd(); } } - // 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)); + // If the element was added, we don't have to introduce a set action for flushing it + if (state != EntryState.ADDED) { + // 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)); + } } } } diff --git a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/MapAttributeFlusher.java b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/MapAttributeFlusher.java index 3cc0edb6ab..5a3881584e 100644 --- a/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/MapAttributeFlusher.java +++ b/entity-view/impl/src/main/java/com/blazebit/persistence/view/impl/update/flush/MapAttributeFlusher.java @@ -48,7 +48,6 @@ import java.util.AbstractMap; import java.util.ArrayList; import java.util.Arrays; -import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.IdentityHashMap; @@ -196,9 +195,9 @@ protected boolean isIndexed() { } @Override - protected void addElementFlushActions(UpdateContext context, TypeDescriptor typeDescriptor, List> actions, V current) { + protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescriptor typeDescriptor, List> actions, V current) { final ViewToEntityMapper mapper = typeDescriptor.getViewToEntityMapper(); - OUTER: for (Map.Entry entry : current.entrySet()) { + for (Map.Entry entry : current.entrySet()) { Object o = entry.getValue(); if (o instanceof MutableStateTrackable) { MutableStateTrackable element = (MutableStateTrackable) o; @@ -206,19 +205,20 @@ protected void addElementFlushActions(UpdateContext context, TypeDescriptor type DirtyAttributeFlusher flusher = (DirtyAttributeFlusher) (DirtyAttributeFlusher) mapper.getNestedDirtyFlusher(context, element, (DirtyAttributeFlusher) null); if (flusher != null) { // Skip the element flusher if the key was already added before - boolean wasElementAdded = false; + EntryState state = EntryState.EXISTED; for (MapAction action : actions) { - if (action.getAddedKeys().contains(entry.getKey())) { - continue OUTER; - } - if (identityContains(action.getAddedElements(), element)) { - wasElementAdded = true; + if (identityContains(action.getRemovedElements(), element)) { + state = state.onRemove(); + if (identityContains(action.getAddedElements(), element)) { + state = state.onAdd(); + } + } else if (identityContains(action.getAddedElements(), element)) { + state = state.onAdd(); } } - if (wasElementAdded) { - actions.add(new MapPutAction(entry.getKey(), element, Collections.emptyMap())); - } else { + // If the element was added, we don't have to introduce a put action for flushing it + if (state != EntryState.ADDED) { actions.add(new MapPutAction(entry.getKey(), element, current)); } } @@ -226,16 +226,6 @@ protected void addElementFlushActions(UpdateContext context, TypeDescriptor type } } - private boolean identityContains(Collection addedElements, MutableStateTrackable element) { - for (Object addedElement : addedElements) { - if (addedElement == element) { - return true; - } - } - - return false; - } - @Override protected void invokeCollectionAction(UpdateContext context, Object ownerView, Object view, V targetCollection, Object value, List> collectionActions) { if (mapping == null) { @@ -1603,12 +1593,16 @@ protected DirtyAttributeFlusher, E, V> determineDirtyF List>> collectionActions = determineCollectionActions(context, initial, current, equalityChecker); // If nothing changed in the collection and no changes should be flushed, we are done - if (collectionActions.size() == 0 && (!keyDescriptor.shouldFlushMutations() || keyDescriptor.supportsDirtyCheck()) && (!elementDescriptor.shouldFlushMutations() || elementDescriptor.supportsDirtyCheck())) { - // Always reset the actions as that indicates changes + if (collectionActions.size() == 0) { + List> elementFlushers = getElementFlushers(context, current, collectionActions); if (current instanceof RecordingMap) { ((RecordingMap) current).resetActions(context); } - return null; + // A "null" element flusher list is given when a fetch and compare is more appropriate + if (elementFlushers == null) { + return this; + } + return getReplayAndElementFlusher(context, initial, current, collectionActions, elementFlushers); } if (collectionActions.size() > current.size()) { @@ -1632,13 +1626,13 @@ protected DirtyAttributeFlusher, E, V> determineDirtyF // If we determine possible collection actions, we try to apply them, but if not if (elementDescriptor.shouldFlushMutations()) { List> elementFlushers = getElementFlushers(context, current, collectionActions); + if (current instanceof RecordingMap) { + ((RecordingMap) current).resetActions(context); + } // A "null" element flusher list is given when a fetch and compare is more appropriate if (elementFlushers == null) { return this; } - if (current instanceof RecordingMap) { - ((RecordingMap) current).resetActions(context); - } return getReplayAndElementFlusher(context, initial, current, collectionActions, elementFlushers); } else { if (current instanceof RecordingMap) {