Skip to content

Commit

Permalink
[#1396] Fix support for flushing flat view collections with read-only…
Browse files Browse the repository at this point in the history
… declared and updatable subview types
  • Loading branch information
beikov committed Dec 27, 2021
1 parent 117e4bd commit e0476e4
Show file tree
Hide file tree
Showing 6 changed files with 118 additions and 29 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ Not yet released
* Fix typo in QueryDSL `GROUP_CONCAT` rendering leading to a syntax error
* Add `getById` implementation as needed by Spring Data 2.5 to the integration
* Ensure stream/iteration processing of results works with entity views containing no collections
* Fix support for flushing flat view collections with read-only declared and updatable subview types

### Backwards-incompatible changes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
import com.blazebit.persistence.view.impl.EntityViewManagerImpl;
import com.blazebit.persistence.view.impl.accessor.AttributeAccessor;
import com.blazebit.persistence.view.impl.mapper.Mapper;
import com.blazebit.persistence.view.spi.type.MutableStateTrackable;
import com.blazebit.persistence.view.impl.update.EntityViewUpdater;
import com.blazebit.persistence.view.impl.update.EntityViewUpdaterImpl;
import com.blazebit.persistence.view.impl.update.UpdateContext;
import com.blazebit.persistence.view.impl.update.flush.CompositeAttributeFlusher;
import com.blazebit.persistence.view.impl.update.flush.DirtyAttributeFlusher;
import com.blazebit.persistence.view.metamodel.Type;
import com.blazebit.persistence.view.spi.type.MutableStateTrackable;

import java.util.List;
import java.util.Map;
Expand Down Expand Up @@ -82,11 +83,18 @@ public Object applyToEntity(UpdateContext context, Object entity, Object view) {
Class<?> viewTypeClass = getViewTypeClass(view);
EntityViewUpdater updater = persistUpdater.get(viewTypeClass);
if (updater == null) {
// Currently we don't handle read only flat views, but not sure this is a problem
if (persistUpdater.isEmpty()) {
throw new IllegalStateException("Couldn't update object for attribute '" + attributeLocation + "'. No allowed types for updates found, maybe you forgot to annotate '" + viewTypeClass.getName() + "' with @UpdatableEntityView?");
if (view instanceof MutableStateTrackable) {
if (persistUpdater.isEmpty()) {
throw new IllegalStateException("Couldn't update object for attribute '" + attributeLocation + "'. No allowed types for updates found, maybe you forgot to annotate '" + viewTypeClass.getName() + "' with @UpdatableEntityView?");
} else {
throw new IllegalStateException("Couldn't update object for attribute '" + attributeLocation + "'. Expected subviews of the types " + persistUpdater.keySet() + " but got: " + view);
}
} else {
throw new IllegalStateException("Couldn't update object for attribute '" + attributeLocation + "'. Expected subviews of the types " + persistUpdater.keySet() + " but got: " + view);
if (entity == null) {
entity = entityLoader.toEntity(context, view, null);
}
((CompositeAttributeFlusher) defaultUpdater.getFullGraphNode()).flushEntity(context, entity, null, view, view, null);
return entity;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -565,10 +565,10 @@ protected final boolean determineElementFlushers(UpdateContext context, TypeDesc
* @since 1.4.0
*/
protected static enum EntryState {
EXISTED {
UPDATED {
@Override
EntryState onAdd() {
return EXISTED;
return ADDED;
}

@Override
Expand All @@ -584,13 +584,13 @@ EntryState onAdd() {

@Override
EntryState onRemove() {
return EXISTED;
return UPDATED;
}
},
REMOVED {
@Override
EntryState onAdd() {
return EXISTED;
return UPDATED;
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -336,36 +336,44 @@ protected boolean isIndexed() {
@Override
protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescriptor typeDescriptor, List<CollectionAction<?>> actions, V current) {
final ViewToEntityMapper mapper = typeDescriptor.getViewToEntityMapper();
int initialSize = -1;
for (int i = 0; i < current.size(); i++) {
Object o = current.get(i);
if (o instanceof MutableStateTrackable) {
MutableStateTrackable element = (MutableStateTrackable) o;
@SuppressWarnings("unchecked")
DirtyAttributeFlusher<?, E, V> flusher = (DirtyAttributeFlusher<?, E, V>) (DirtyAttributeFlusher) mapper.getNestedDirtyFlusher(context, element, (DirtyAttributeFlusher) null);
if (flusher != null) {
// If the element is dirty, we have to register a ListSetAction if the element was not added
// If it were added, we would already have an action for that object
EntryState state = EntryState.EXISTED;
// At this point, we have to check the collection actions to determine if the view was added through actions somehow
// We will register a special ListSetAction if the view was not added through actions to issue an UPDATE statement
// 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;
for (CollectionAction<?> action : actions) {
Collection<Object> removedObjects = action.getRemovedObjects();
if (identityContains(removedObjects, element)) {
state = state.onRemove();
if (identityContains(action.getAddedObjects(), element)) {
state = state.onAdd();
// This is a ListSetAction where the old and new object are the same instances
replacedObject = null;
state = EntryState.UPDATED;
} else {
state = state.onRemove();
}
} else if (identityContains(action.getAddedObjects(), element)) {
if (removedObjects.isEmpty()) {
state = state.onAdd();
} else {
// This is like replacing an existing entry
state = EntryState.ADDED;
// This is a ListSetAction which has only a single element, so this is safe
replacedObject = removedObjects.iterator().next();
state = EntryState.UPDATED;
}
}
}

// If the element was added, we don't have to introduce a set action for flushing it
if (state != EntryState.ADDED) {
// 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) {
// 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));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
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;
Expand Down Expand Up @@ -206,22 +207,53 @@ protected void addFlatViewElementFlushActions(UpdateContext context, TypeDescrip
@SuppressWarnings("unchecked")
DirtyAttributeFlusher<?, E, V> flusher = (DirtyAttributeFlusher<?, E, V>) (DirtyAttributeFlusher) mapper.getNestedDirtyFlusher(context, element, (DirtyAttributeFlusher) null);
if (flusher != null) {
// Skip the element flusher if the key was already added before
EntryState state = EntryState.EXISTED;
// At this point, we have to check the collection actions to determine if the element was added through actions somehow
// We will register a special MapPutAction if the element was not added through actions to issue an UPDATE statement
// By default, since the element is dirty, we start with the state UPDATED and go through state transitions
// based on the containment of the element in the added/removed objects collections of the actions
EntryState state = EntryState.UPDATED;
Object replacedObject = element;
for (MapAction<?> action : actions) {
if (identityContains(action.getRemovedElements(), element)) {
state = state.onRemove();
if (identityContains(action.getAddedElements(), element)) {
Collection<Object> removedElements = action.getRemovedElements();
Collection<Object> addedElements = action.getAddedElements();
if (removedElements.isEmpty()) {
if (identityContains(addedElements, element)) {
state = state.onAdd();
}
} else if (identityContains(action.getAddedElements(), element)) {
state = state.onAdd();
} else if (addedElements.isEmpty()) {
if (identityContains(removedElements, element)) {
state = state.onRemove();
}
} else {
// Here we have a MapPutAction or MapPutAllAction where added/removed have the same cardinality
Iterator<Object> addedIter = addedElements.iterator();
Iterator<Object> removedIter = removedElements.iterator();
while (addedIter.hasNext()) {
Object added = addedIter.next();
Object removed = removedIter.next();
if (removed == element) {
if (added == element) {
// This is a MapPutAction or MapPutAllAction where the old and new object are the same instances
replacedObject = element;
state = EntryState.UPDATED;
} else {
state = state.onRemove();
}
break;
} else if (added == element) {
replacedObject = removed;
state = EntryState.UPDATED;
break;
}
}
}
}

// 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));
// If the element was UPDATED and the replacedObject is the element itself,
// 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 == element) {
actions.add(new MapPutAction<>(entry.getKey(), element, element));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,43 @@ public void testUpdateCollectionElement() {
assertSubviewEquals(doc1.getNames(), docView.getNames());
}

@Test
public void testAddCollectionElement() {
// Given
final UpdatableDocumentWithCollectionsView docView = getDoc1View();
clearQueries();

// When

UpdatableNameObjectView updatableNameObjectView = evm.create(UpdatableNameObjectView.class);
updatableNameObjectView.setPrimaryName("newPers");
docView.getNames().add(updatableNameObjectView);
update(docView);

// Then
// Assert that the document and the people are loaded i.e. a full fetch
// Finally the person is added because the primary name 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");
}
builder.insert(Document.class, "names");
builder.validate();

assertNoUpdateAndReload(docView);
assertEquals("newPers", doc1.getNames().get(1).getPrimaryName());
assertSubviewEquals(doc1.getNames(), docView.getNames());
}

public static void assertSubviewEquals(Collection<NameObject> persons, Collection<? extends ReadonlyNameObjectView> personSubviews) {
if (persons == null) {
assertNull(personSubviews);
Expand Down Expand Up @@ -141,6 +178,9 @@ protected AssertStatementBuilder fullFetch(AssertStatementBuilder builder) {
protected AssertStatementBuilder fullUpdate(AssertStatementBuilder builder) {
builder.delete(Document.class, "names")
.insert(Document.class, "names");
if ( doc1.getNames().size() > 1 ) {
builder.insert(Document.class, "names");
}
builder.update(Document.class);
return builder;
}
Expand Down

0 comments on commit e0476e4

Please sign in to comment.