From 0fd0d094b3841df9aa8322b7840949a1d6fa1eb7 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Sat, 22 Dec 2018 13:22:02 +0100 Subject: [PATCH] Add equivalence strategy to delta application Also changed the default for adding values from REAL_VALUE to IGNORE_METADATA_CONSIDER_DIFFERENT_IDS i.e. the changes are: - different non-null PCV IDs make values inequal - different element names make values inequal (this is to be rethought). The strategy is now in sync with the default strategy used for diff(..). --- .../com/evolveum/midpoint/prism/Item.java | 10 +++- .../midpoint/prism/delta/ItemDelta.java | 5 +- .../prism/delta/ItemDeltaCollectionsUtil.java | 3 +- .../midpoint/prism/delta/ObjectDelta.java | 3 ++ .../midpoint/prism/impl/ItemImpl.java | 17 +++++-- .../prism/impl/delta/ItemDeltaImpl.java | 47 ++++++++++++------- .../prism/impl/delta/ObjectDeltaImpl.java | 17 ++++--- .../model/impl/lens/ChangeExecutor.java | 3 +- .../model/impl/lens/IvwoConsolidator.java | 3 +- .../model/impl/visualizer/Visualizer.java | 5 +- .../impl/lens/TestAssignmentProcessor.java | 2 +- ...y-delete-assignment-account-dummy-attr.xml | 2 +- 12 files changed, 81 insertions(+), 36 deletions(-) diff --git a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Item.java b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Item.java index 75922a7077c..702563fd686 100644 --- a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Item.java +++ b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/Item.java @@ -278,6 +278,14 @@ public interface Item extends It */ boolean addAll(Collection newValues) throws SchemaException; + /** + * Adds given values, with the same semantics as repeated add(..) calls. + * For equality testing uses give strategy. + * + * @return true if this item changed as a result of the call (i.e. if at least one value was really added) + */ + boolean addAll(Collection newValues, EquivalenceStrategy strategy) throws SchemaException; + /** * Removes given value from the item. * @@ -313,7 +321,7 @@ public interface Item extends It /** * Replaces all values of the item by given values. */ - void replaceAll(Collection newValues) throws SchemaException; + void replaceAll(Collection newValues, EquivalenceStrategy strategy) throws SchemaException; /** * Replaces all values of the item by given value. diff --git a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDelta.java b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDelta.java index 98e7e08bdd6..104e04184bf 100644 --- a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDelta.java +++ b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDelta.java @@ -17,6 +17,7 @@ import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.ItemName; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.util.DebugDumpable; @@ -256,13 +257,15 @@ public interface ItemDelta extend void simplify(); void applyTo(PrismContainerValue containerValue) throws SchemaException; + void applyTo(PrismContainerValue containerValue, ParameterizedEquivalenceStrategy strategy) throws SchemaException; void applyTo(Item item) throws SchemaException; + void applyTo(Item item, ParameterizedEquivalenceStrategy strategy) throws SchemaException; /** * Applies delta to item were path of the delta and path of the item matches (skips path checks). */ - void applyToMatchingPath(Item item) throws SchemaException; + void applyToMatchingPath(Item item, ParameterizedEquivalenceStrategy strategy) throws SchemaException; ItemDelta getSubDelta(ItemPath path); diff --git a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDeltaCollectionsUtil.java b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDeltaCollectionsUtil.java index c0733165829..ed7bb8dc0c2 100644 --- a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDeltaCollectionsUtil.java +++ b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ItemDeltaCollectionsUtil.java @@ -17,6 +17,7 @@ package com.evolveum.midpoint.prism.delta; import com.evolveum.midpoint.prism.*; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.ItemName; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.util.exception.SchemaException; @@ -155,7 +156,7 @@ public static void applyTo(Collection deltas, PrismContaine public static void applyToMatchingPath(Collection deltas, PrismContainer propertyContainer) throws SchemaException { for (ItemDelta delta : deltas) { - delta.applyToMatchingPath(propertyContainer); + delta.applyToMatchingPath(propertyContainer, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); } } diff --git a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java index 29625dbd3c4..4571640bceb 100644 --- a/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java +++ b/infra/prism-api/src/main/java/com/evolveum/midpoint/prism/delta/ObjectDelta.java @@ -17,6 +17,7 @@ import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.util.DebugDumpable; import com.evolveum.midpoint.util.exception.SchemaException; @@ -198,6 +199,8 @@ Collection targetObject) throws SchemaException; + void applyTo(PrismObject targetObject, ParameterizedEquivalenceStrategy strategy) throws SchemaException; + /** * Applies this object delta to specified object, returns updated object. * It leaves the original object unchanged. diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/ItemImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/ItemImpl.java index 6500512324a..bf3c660fb18 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/ItemImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/ItemImpl.java @@ -401,7 +401,7 @@ public int size() { } public boolean addAll(Collection newValues) throws SchemaException { - checkMutability(); // TODO consider weaker condition, like testing if there's a real change + checkMutability(); boolean changed = false; for (V val: newValues) { if (add(val)) { @@ -411,6 +411,17 @@ public boolean addAll(Collection newValues) throws SchemaException { return changed; } + public boolean addAll(Collection newValues, EquivalenceStrategy strategy) throws SchemaException { + checkMutability(); + boolean changed = false; + for (V val: newValues) { + if (add(val, strategy)) { + changed = true; + } + } + return changed; + } + public boolean add(@NotNull V newValue) throws SchemaException { return add(newValue, true, getEqualsHashCodeStrategy()); } @@ -496,10 +507,10 @@ public V remove(int index) { } @Override - public void replaceAll(Collection newValues) throws SchemaException { + public void replaceAll(Collection newValues, EquivalenceStrategy strategy) throws SchemaException { checkMutability(); clear(); - addAll(newValues); + addAll(newValues, strategy); } @Override diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ItemDeltaImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ItemDeltaImpl.java index 15732e5f314..704e6982979 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ItemDeltaImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ItemDeltaImpl.java @@ -21,6 +21,7 @@ import com.evolveum.midpoint.prism.delta.PlusMinusZero; import com.evolveum.midpoint.prism.delta.PrismValueDeltaSetTriple; import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.ItemName; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.util.*; @@ -1155,14 +1156,12 @@ private void cleanupAllTheWayUp(Item item) { if (item.isEmpty()) { PrismValue itemParent = item.getParent(); if (itemParent != null) { - if (itemParent instanceof PrismContainerValue) { - ((PrismContainerValue)itemParent).remove(item); - if (itemParent.isEmpty()) { - Itemable itemGrandparent = itemParent.getParent(); - if (itemGrandparent != null) { - if (itemGrandparent instanceof Item) { - cleanupAllTheWayUp((Item)itemGrandparent); - } + ((PrismContainerValue)itemParent).remove(item); + if (itemParent.isEmpty()) { + Itemable itemGrandparent = itemParent.getParent(); + if (itemGrandparent != null) { + if (itemGrandparent instanceof Item) { + cleanupAllTheWayUp((Item)itemGrandparent); } } } @@ -1171,27 +1170,35 @@ private void cleanupAllTheWayUp(Item item) { } public void applyTo(PrismContainerValue containerValue) throws SchemaException { + applyTo(containerValue, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); + } + + public void applyTo(PrismContainerValue containerValue, ParameterizedEquivalenceStrategy strategy) throws SchemaException { ItemPath deltaPath = getPath(); if (ItemPath.isEmpty(deltaPath)) { throw new IllegalArgumentException("Cannot apply empty-path delta " + this + " directly to a PrismContainerValue " + containerValue); } Item subItem = containerValue.findOrCreateItem(deltaPath, getItemClass(), getDefinition()); - applyToMatchingPath(subItem); + applyToMatchingPath(subItem, strategy); } public void applyTo(Item item) throws SchemaException { + applyTo(item, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); + } + + public void applyTo(Item item, ParameterizedEquivalenceStrategy strategy) throws SchemaException { ItemPath itemPath = item.getPath(); ItemPath deltaPath = getPath(); CompareResult compareComplex = itemPath.compareComplex(deltaPath); if (compareComplex == CompareResult.EQUIVALENT) { - applyToMatchingPath(item); + applyToMatchingPath(item, strategy); cleanupAllTheWayUp(item); } else if (compareComplex == CompareResult.SUBPATH) { if (item instanceof PrismContainer) { PrismContainer container = (PrismContainer)item; ItemPath remainderPath = deltaPath.remainder(itemPath); Item subItem = container.findOrCreateItem(remainderPath, getItemClass(), getDefinition()); - applyToMatchingPath(subItem); + applyToMatchingPath(subItem, strategy); } else { throw new SchemaException("Cannot apply delta "+this+" to "+item+" as delta path is below the item path and the item is not a container"); } @@ -1205,29 +1212,35 @@ public void applyTo(Item item) throws SchemaException { /** * Applies delta to item were path of the delta and path of the item matches (skips path checks). */ - public void applyToMatchingPath(Item item) throws SchemaException { + public void applyToMatchingPath(Item item, ParameterizedEquivalenceStrategy strategy) throws SchemaException { if (item == null) { return; } if (item.getDefinition() == null && getDefinition() != null){ + //noinspection unchecked item.applyDefinition(getDefinition()); } if (!getItemClass().isAssignableFrom(item.getClass())) { throw new SchemaException("Cannot apply delta "+this+" to "+item+" because the deltas is applicable only to "+getItemClass().getSimpleName()); } if (valuesToReplace != null) { - item.replaceAll(PrismValueCollectionsUtil.cloneCollection(valuesToReplace)); + //noinspection unchecked + item.replaceAll(PrismValueCollectionsUtil.cloneCollection(valuesToReplace), strategy); } else { if (valuesToDelete != null) { + //noinspection unchecked item.removeAll(valuesToDelete); } if (valuesToAdd != null) { if (item.getDefinition() != null && item.getDefinition().isSingleValue()) { - item.replaceAll(PrismValueCollectionsUtil.cloneCollection(valuesToAdd)); + //noinspection unchecked + item.replaceAll(PrismValueCollectionsUtil.cloneCollection(valuesToAdd), strategy); } else { for (V valueToAdd : valuesToAdd) { - if (!item.contains(valueToAdd, EquivalenceStrategy.REAL_VALUE)) { - item.add(valueToAdd.clone()); + //noinspection unchecked + if (!item.contains(valueToAdd, strategy)) { + //noinspection unchecked + item.add(valueToAdd.clone(), false); } } } @@ -1297,7 +1310,7 @@ public Item getItemNewMatchingPath(Item itemOld) throws SchemaExceptio } else { itemNew = itemOld.clone(); } - applyToMatchingPath(itemNew); + applyToMatchingPath(itemNew, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); return itemNew; } diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ObjectDeltaImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ObjectDeltaImpl.java index 268a433fba3..124bfea1e85 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ObjectDeltaImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/delta/ObjectDeltaImpl.java @@ -18,6 +18,7 @@ import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.delta.*; import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.*; import com.evolveum.midpoint.util.DebugUtil; import com.evolveum.midpoint.util.MiscUtil; @@ -569,11 +570,11 @@ public void mergeModification(ItemDelta modificationToMerge) throws SchemaE } - /** - * Applies this object delta to specified object, returns updated object. - * It modifies the provided object. - */ public void applyTo(PrismObject targetObject) throws SchemaException { + applyTo(targetObject, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); + } + + public void applyTo(PrismObject targetObject, ParameterizedEquivalenceStrategy strategy) throws SchemaException { if (isEmpty()) { // nothing to do return; @@ -581,12 +582,14 @@ public void applyTo(PrismObject targetObject) throws SchemaException { if (changeType != ChangeType.MODIFY) { throw new IllegalStateException("Can apply only MODIFY delta to object, got " + changeType + " delta"); } - applyTo(targetObject, modifications); + applyTo(targetObject, modifications, strategy); } - private static void applyTo(PrismObject targetObject, Collection> modifications) throws SchemaException { + private static void applyTo(PrismObject targetObject, + Collection> modifications, + ParameterizedEquivalenceStrategy strategy) throws SchemaException { for (ItemDelta itemDelta : modifications) { - itemDelta.applyTo(targetObject); + itemDelta.applyTo(targetObject, strategy); } } diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ChangeExecutor.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ChangeExecutor.java index 866970e3e56..fc2441c7c9a 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ChangeExecutor.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/ChangeExecutor.java @@ -552,7 +552,8 @@ private boolean isEquivalentModifyDelta(Collection> mo .findItemDeltasSubPath(modifications1, ShadowType.F_ATTRIBUTES); Collection> attrDeltas2 = ItemDeltaCollectionsUtil .findItemDeltasSubPath(modifications2, ShadowType.F_ATTRIBUTES); - return MiscUtil.unorderedCollectionEquals(attrDeltas1, attrDeltas2); + //noinspection unchecked,RedundantCast + return MiscUtil.unorderedCollectionEquals((Collection) attrDeltas1, (Collection) attrDeltas2); } private boolean isEquivalentAddDelta(PrismObject object1, PrismObject object2) { diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/IvwoConsolidator.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/IvwoConsolidator.java index a9fd91c1749..0d79f6b79a7 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/IvwoConsolidator.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/IvwoConsolidator.java @@ -25,6 +25,7 @@ import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.equivalence.EquivalenceStrategy; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.ItemPath; import org.apache.commons.lang.mutable.MutableBoolean; import org.jetbrains.annotations.NotNull; @@ -514,7 +515,7 @@ private boolean hasValue(Item item, ItemDelta itemDelta) throws Schema return true; } else { Item clonedItem = item.clone(); - itemDelta.applyToMatchingPath(clonedItem); + itemDelta.applyToMatchingPath(clonedItem, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); return !clonedItem.isEmpty(); } } diff --git a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/visualizer/Visualizer.java b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/visualizer/Visualizer.java index 23afbb7d009..126c9446585 100644 --- a/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/visualizer/Visualizer.java +++ b/model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/visualizer/Visualizer.java @@ -20,6 +20,7 @@ import com.evolveum.midpoint.model.impl.visualizer.output.*; import com.evolveum.midpoint.prism.*; import com.evolveum.midpoint.prism.delta.*; +import com.evolveum.midpoint.prism.equivalence.ParameterizedEquivalenceStrategy; import com.evolveum.midpoint.prism.path.ItemPath; import com.evolveum.midpoint.prism.polystring.PolyString; import com.evolveum.midpoint.prism.util.CloneUtil; @@ -673,7 +674,7 @@ private SceneDeltaItemImpl createSceneDeltaItem(PropertyDelta delta, SceneImp property.addValues(CloneUtil.cloneCollectionMembers(delta.getEstimatedOldValues())); } try { - delta.applyToMatchingPath(property); + delta.applyToMatchingPath(property, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); } catch (SchemaException e) { throw new SystemException("Couldn't visualize property delta: " + delta + ": " + e.getMessage(), e); } @@ -778,7 +779,7 @@ private SceneDeltaItemImpl createSceneDeltaItem(ReferenceDelta delta, SceneImpl if (delta.getEstimatedOldValues() != null) { reference.addAll(CloneUtil.cloneCollectionMembers(delta.getEstimatedOldValues())); } - delta.applyToMatchingPath(reference); + delta.applyToMatchingPath(reference, ParameterizedEquivalenceStrategy.DEFAULT_FOR_DELTA_APPLICATION); } catch (SchemaException e) { throw new SystemException("Couldn't visualize reference delta: " + delta + ": " + e.getMessage(), e); } diff --git a/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/TestAssignmentProcessor.java b/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/TestAssignmentProcessor.java index 31db295b9be..1fb6f039df5 100644 --- a/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/TestAssignmentProcessor.java +++ b/model/model-impl/src/test/java/com/evolveum/midpoint/model/impl/lens/TestAssignmentProcessor.java @@ -390,7 +390,7 @@ public void test031DeleteAssignmentModifyAccount() throws Exception { display("Input context", context); PrismObject userNew = context.getFocusContext().getObjectNew(); - assertEquals("Unexpected number of assignemnts in userNew after recompute", 1, userNew.asObjectable().getAssignment().size()); + assertEquals("Unexpected number of assignments in userNew after recompute", 1, userNew.asObjectable().getAssignment().size()); assertFocusModificationSanity(context); diff --git a/model/model-impl/src/test/resources/lens/user-barbossa-modify-delete-assignment-account-dummy-attr.xml b/model/model-impl/src/test/resources/lens/user-barbossa-modify-delete-assignment-account-dummy-attr.xml index b1d62467229..886da010a0b 100644 --- a/model/model-impl/src/test/resources/lens/user-barbossa-modify-delete-assignment-account-dummy-attr.xml +++ b/model/model-impl/src/test/resources/lens/user-barbossa-modify-delete-assignment-account-dummy-attr.xml @@ -25,7 +25,7 @@ delete assignment - + Undead monkey account construction