From ee221dd4fa294a52e2e5684a1a1218338db618ca Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Mon, 19 Apr 2021 15:59:20 +0200 Subject: [PATCH 1/5] Added Object Size performance test Signed-off-by: Tony Tkacik --- .../AbstractSchemaPerformanceTest.java | 2 +- .../performance/PerfTestPrismObjectSize.java | 221 ++++++++++++++++++ 2 files changed, 222 insertions(+), 1 deletion(-) create mode 100644 infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java diff --git a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/AbstractSchemaPerformanceTest.java b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/AbstractSchemaPerformanceTest.java index d72640979f9..de0becca9b4 100644 --- a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/AbstractSchemaPerformanceTest.java +++ b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/AbstractSchemaPerformanceTest.java @@ -99,7 +99,7 @@ protected double measureSingle(String label, CheckedProducer producer, long e } @NotNull - public PrismObject getJack() throws SchemaException, IOException { + public static PrismObject getJack() throws SchemaException, IOException { return getPrismContext().parserFor(USER_JACK_FILE).parse(); } diff --git a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java new file mode 100644 index 00000000000..af367898796 --- /dev/null +++ b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java @@ -0,0 +1,221 @@ +package com.evolveum.midpoint.schema.performance; + +import static com.evolveum.midpoint.prism.util.PrismTestUtil.getPrismContext; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.UUID; + +import javax.xml.namespace.QName; + +import org.jetbrains.annotations.NotNull; +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +import com.evolveum.midpoint.prism.PrismObject; +import com.evolveum.midpoint.prism.PrismParserNoIO; +import com.evolveum.midpoint.prism.PrismSerializer; +import com.evolveum.midpoint.prism.impl.xnode.ListXNodeImpl; +import com.evolveum.midpoint.prism.impl.xnode.MapXNodeImpl; +import com.evolveum.midpoint.prism.impl.xnode.PrimitiveXNodeImpl; +import com.evolveum.midpoint.prism.impl.xnode.XNodeImpl; +import com.evolveum.midpoint.prism.xnode.RootXNode; +import com.evolveum.midpoint.util.CheckedProducer; +import com.evolveum.midpoint.util.exception.CommonException; +import com.evolveum.midpoint.util.exception.SchemaException; +import com.evolveum.midpoint.xml.ns._public.common.common_3.AssignmentType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.ConstructionType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectReferenceType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.ResourceType; +import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType; + +public class PerfTestPrismObjectSize extends AbstractSchemaPerformanceTest { + + public static final int EXECUTION = 3000; + public static final int REPEATS = 1; + + private static final QName CONTAINER_ID = new QName("id"); + + private static final String[] FILE_FORMATS = new String[] {"xml","json","yaml"}; + + private static final int[] CONTAINER_COUNTS = new int[] { 0, 10, 20, 50, 100, 200, 500, 1000}; + + + + + @Test(dataProvider = "combinations") + public void fromXnodeToFile(ContainerTestParams config) throws Exception { + RootXNode input = config.testObject(); + for(String format : FILE_FORMATS) { + PrismSerializer serializer = getPrismContext().serializerFor(format); + String monitorId = monitorName("serialize.xnode", config.monitorId() , format); + String note = "Measures serialization from xnode to data stream. Test parameters: " + config; + measure(monitorId, note, () -> serializer.serialize(input)); + } + } + + @Test(dataProvider = "combinationsConflicts") + public void fromXnodeToPrism(ContainerTestParams config) throws Exception { + RootXNode input = config.testObject(); + PrismParserNoIO parser = getPrismContext().parserFor(input); + String monitorId = monitorName("parse.prism", config.monitorId()); + String note = "Measures unmarshalling of Prism Objects from XNodes. Test parameters: " + config; + measure(monitorId, note, parser::parse); + } + + @Test(dataProvider = "combinations") + public void fromPrismToXnode(ContainerTestParams config) throws Exception { + PrismObject input = getPrismContext().parserFor(config.testObject()).parse(); + PrismSerializer serializer = getPrismContext().xnodeSerializer(); + String monitorId = monitorName("serialize.prism", config.monitorId()); + String note = "Measures unmarshalling of Prism Objects from XNodes. Test parameters: " + config; + measure(monitorId, note, () -> serializer.serialize(input)); + } + + @Test(dataProvider = "combinations") + public void fromFileToXNode(ContainerTestParams config) throws Exception { + for(String format : FILE_FORMATS) { + String input = getPrismContext().serializerFor(format).serialize(config.testObject()); + PrismParserNoIO parser = getPrismContext().parserFor(input); + String monitorId = monitorName("parse.xnode", config.monitorId(), format); + String note = "Measures parsing of JSON/XML/YAML to XNodes. Test parameters: " + config; + measure(monitorId, note, parser::parseToXNode); + } + } + + private static final List generateUUIDs(int count) { + List ret = new ArrayList<>(count); + for (int i = 0; i < count; i++) { + ret.add(newUuid()); + } + return ret; + } + + private static String newUuid() { + return UUID.randomUUID().toString(); + } + + @DataProvider(name = "combinations") + public Object[][] combinationsWithoutConflicts() { + return createCombinations(false); + } + + @DataProvider(name = "combinationsConflicts") + public Object[][] combinationsWithConflicts() { + return createCombinations(true); + } + + private static Object[][] createCombinations(boolean withConflicts) { + ArrayList configs = new ArrayList<>(); + for (int containerCount : CONTAINER_COUNTS) { + int conflicts = 2 * containerCount / 10; + configs.add(new Object[] { new ContainerTestParams(containerCount, 0, false)}); + configs.add(new Object[] { new ContainerTestParams(containerCount, 0, true)}); + if (withConflicts && conflicts > 0) { + configs.add(new Object[] { new ContainerTestParams(containerCount, conflicts, false)}); + configs.add(new Object[] { new ContainerTestParams(containerCount, conflicts, true)}); + } + } + return configs.toArray(new Object[configs.size()][]); + } + + @Override + protected void measure(String label, String note, CheckedProducer producer) throws CommonException, IOException { + super.measure(label, note, producer, EXECUTION, REPEATS); + } + + private static final RootXNode generateTestObject(int count, int conflicts, boolean withIds) throws SchemaException, IOException { + @NotNull + PrismObject baseObject = getJack(); + + @NotNull + RootXNode rootNode = getPrismContext().xnodeSerializer().serialize(baseObject); + MapXNodeImpl user = (MapXNodeImpl) rootNode.getSubnode(); + + ListXNodeImpl assignments = new ListXNodeImpl(); + + int unique = count - conflicts; + List uuids = generateUUIDs(unique); + // Lets insert unique + + for (int i = 0; i < unique; i++) { + assignments.add(createXNodeAssignment(uuids.get(i), withIds ? i : -1)); + } + for (int i = 0; i < conflicts; i++) { + int id = unique - 1 - i; + assignments.add(createXNodeAssignment(uuids.get(id), withIds ? id : -1)); + } + user.put(UserType.F_ASSIGNMENT, assignments); + return rootNode; + } + + + private static final MapXNodeImpl createXNodeAssignment(String uuid, int id) { + MapXNodeImpl assignment = new MapXNodeImpl(); + if(id > 0) { + assignment.put(CONTAINER_ID, attribute(Long.valueOf(id))); + } + MapXNodeImpl construction = new MapXNodeImpl(); + assignment.put(AssignmentType.F_CONSTRUCTION, construction); + MapXNodeImpl resourceRef = new MapXNodeImpl(); + construction.put(ConstructionType.F_RESOURCE_REF, resourceRef); + resourceRef.put(ObjectReferenceType.F_OID, attribute(uuid)); + resourceRef.put(ObjectReferenceType.F_TYPE, attribute(ResourceType.COMPLEX_TYPE)); + return assignment; + } + + private static @NotNull XNodeImpl attribute(Object value) { + PrimitiveXNodeImpl attr = new PrimitiveXNodeImpl<>(value); + attr.setAttribute(true); + return attr; + } + + + public static class ContainerTestParams { + + public ContainerTestParams(int count, int conflicts, boolean withIds) { + this.count = count; + this.conflicts = conflicts; + this.withIds = withIds; + } + + private final int count; + private final int conflicts; + private final boolean withIds; + + public RootXNode testObject() { + try { + return generateTestObject(count, conflicts, withIds); + } catch (Exception e) { + throw new IllegalStateException("Test object generation failed",e); + } + } + + public String monitorId() { + return new StringBuilder() + .append(count) + .append(".") + .append(conflicts) + .append(".") + .append(withIds ? "ids" : "noids") + .toString() + ; + } + + public String testNote() { + return new StringBuilder("container count: ") + .append(count) + .append(", conflicting content count: ") + .append(conflicts) + .append(" container ids: ") + .append(withIds ? "yes" : "no") + .toString() + ; + } + + @Override + public String toString() { + return "[count=" + count + ", conflicts=" + conflicts + ", withIds=" + withIds + "]"; + } + } +} From 12a5711a68a2c3b86d7e297365f8c99fc2045044 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Tue, 20 Apr 2021 21:45:58 +0200 Subject: [PATCH 2/5] Added combination performance test of deltas Signed-off-by: Tony Tkacik --- .../performance/PerfTestPrismObjectSize.java | 89 ++++++++++++++++++- 1 file changed, 86 insertions(+), 3 deletions(-) diff --git a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java index af367898796..32b6bf7d97d 100644 --- a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java +++ b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java @@ -5,16 +5,18 @@ import java.util.ArrayList; import java.util.List; import java.util.UUID; - import javax.xml.namespace.QName; import org.jetbrains.annotations.NotNull; import org.testng.annotations.DataProvider; import org.testng.annotations.Test; +import com.evolveum.axiom.concepts.CheckedFunction; import com.evolveum.midpoint.prism.PrismObject; import com.evolveum.midpoint.prism.PrismParserNoIO; import com.evolveum.midpoint.prism.PrismSerializer; +import com.evolveum.midpoint.prism.delta.ObjectDelta; +import com.evolveum.midpoint.prism.impl.delta.builder.DeltaBuilder; import com.evolveum.midpoint.prism.impl.xnode.ListXNodeImpl; import com.evolveum.midpoint.prism.impl.xnode.MapXNodeImpl; import com.evolveum.midpoint.prism.impl.xnode.PrimitiveXNodeImpl; @@ -38,9 +40,9 @@ public class PerfTestPrismObjectSize extends AbstractSchemaPerformanceTest { private static final String[] FILE_FORMATS = new String[] {"xml","json","yaml"}; - private static final int[] CONTAINER_COUNTS = new int[] { 0, 10, 20, 50, 100, 200, 500, 1000}; - + private static final int[] CONTAINER_COUNTS = new int[] { 10, 20, 50, 100, 200, 500, 1000}; + private static final int[] DELTA_PERCENTS = new int[] {10,20,30}; @Test(dataProvider = "combinations") @@ -83,6 +85,87 @@ public void fromFileToXNode(ContainerTestParams config) throws Exception { } } + private void measureDelta(ContainerTestParams config, String name, CheckedFunction,ObjectDelta,SchemaException> deltaFactory) throws SchemaException { + PrismObject xnodeObj = getPrismContext().parserFor(config.testObject()).parse(); + List existingAssignments = xnodeObj.asObjectable().getAssignment(); + ObjectDelta delta = deltaFactory.apply(existingAssignments); + String monitorId = monitorName("delta", name); + String note = "Application of delta " + name + " Test Params: " + config; + try { + measure(monitorId, note, () -> { + PrismObject objClone = xnodeObj.clone(); + delta.applyTo(objClone); + return objClone; + }); + } catch (CommonException | IOException e) { + throw new IllegalStateException(e); + } + } + + @Test(dataProvider = "combinations") + public void replaceDelta(ContainerTestParams config) throws SchemaException { + for (int percent : DELTA_PERCENTS) { + int opCount = config.count * percent / 100; + measureDelta(config, monitorName("replace",config.monitorId(), Integer.toString(opCount)), assignments -> { + DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); + for (int i = assignments.size() - 1 - opCount; i < assignments.size(); i++) { + AssignmentType assignment = assignments.get(i).clone(); + assignment.description("Modified"); + delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).replace(assignment.asPrismContainerValue()); + } + return delta.asObjectDelta(""); + }); + } + } + + @Test(dataProvider = "combinations") + public void addDelta(ContainerTestParams config) throws SchemaException { + for (int percent : DELTA_PERCENTS) { + int opCount = config.count * percent / 100; + measureDelta(config, monitorName("add",config.monitorId(), Integer.toString(opCount)), assignments -> { + DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); + for (int i = 0; i < opCount; i++) { + AssignmentType assignment = assignments.get(i).clone(); + assignment.getConstruction().resourceRef(newUuid(), assignment.getConstruction().getResourceRef().getType()); + assignment.setId(null); + delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).add(assignment.asPrismContainerValue()); + } + return delta.asObjectDelta(""); + }); + } + } + + @Test(dataProvider = "combinations") + public void deleteDelta(ContainerTestParams config) throws SchemaException { + for (int percent : DELTA_PERCENTS) { + int opCount = config.count * percent / 100; + measureDelta(config, monitorName("delete",config.monitorId(), Integer.toString(opCount)), assignments -> { + DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); + for (int i = assignments.size() - 1 - opCount; i < assignments.size(); i++) { + AssignmentType assignment = assignments.get(i).clone(); + delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).delete(assignment.asPrismContainerValue()); + } + return delta.asObjectDelta(""); + }); + } + } + + + @Test(dataProvider = "combinations") + public void deleteNoIdDelta(ContainerTestParams config) throws SchemaException { + for (int percent : DELTA_PERCENTS) { + int opCount = config.count * percent / 100; + measureDelta(config, monitorName("delete.no.id",config.monitorId(), Integer.toString(opCount)), assignments -> { + DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); + for (int i = assignments.size() - 1 - opCount; i < assignments.size(); i++) { + AssignmentType assignment = assignments.get(i).clone(); + delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).delete(assignment.asPrismContainerValue()); + } + return delta.asObjectDelta(""); + }); + } + } + private static final List generateUUIDs(int count) { List ret = new ArrayList<>(count); for (int i = 0; i < count; i++) { From 25c88d92831e7a896d0dbb32fcd5a26283f14bd2 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Tue, 27 Apr 2021 18:02:55 +0200 Subject: [PATCH 3/5] MID-7008: Added early exit to diff* methods in case they are used via equals In order to determine if objects are equals, we do not need full diff of objects only to know that they are different. Adds optional exit of diff methods on first occurence of different value. --- .../midpoint/prism/impl/ItemImpl.java | 23 ++- .../prism/impl/PrismContainerValueImpl.java | 142 ++++++++++++++++-- .../midpoint/prism/impl/PrismValueImpl.java | 27 +++- .../performance/PerfTestPrismObjectSize.java | 26 ++-- 4 files changed, 191 insertions(+), 27 deletions(-) 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 6a732b43a59..927ed55b9d3 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 @@ -586,8 +586,18 @@ public ItemDelta diff(Item other, @NotNull ParameterizedEquivalenceStr void diffInternal(Item other, Collection deltas, boolean rootValuesOnly, ParameterizedEquivalenceStrategy strategy) { + diffInternal(other, deltas, rootValuesOnly, strategy, false); + } + + boolean diffInternal(Item other, Collection deltas, boolean rootValuesOnly, + ParameterizedEquivalenceStrategy strategy, boolean exitOnDiff) { ItemDelta delta = createDelta(); if (other == null) { + // Early exit for equals use case + if (exitOnDiff) { + return true; + } + if (delta.getDefinition() == null && this.getDefinition() != null) { delta.setDefinition(this.getDefinition().clone()); } @@ -611,7 +621,11 @@ void diffInternal(Item other, Collection deltas, bool if (!rootValuesOnly && thisValue.representsSameValue(otherValue, true)) { found = true; // Matching IDs, look inside to figure out internal deltas - ((PrismValueImpl) thisValue).diffMatchingRepresentation(otherValue, deltas, strategy); + boolean different = ((PrismValueImpl) thisValue).diffMatchingRepresentation(otherValue, deltas, strategy, exitOnDiff); + if (exitOnDiff && different) { + return true; + } + // No need to process this value again iterator.remove(); break; @@ -624,12 +638,18 @@ void diffInternal(Item other, Collection deltas, bool } } if (!found) { + if (exitOnDiff) { + return true; + } // We have the value and the other does not, this is delete of the entire value delta.addValueToDelete(thisValue.clone()); } } // outstandingOtherValues are those values that the other has and we could not // match them to any of our values. These must be new values to add + if (exitOnDiff && !outstandingOtherValues.isEmpty()) { + return true; + } for (PrismValue outstandingOtherValue : outstandingOtherValues) { delta.addValueToAdd(outstandingOtherValue.clone()); } @@ -640,6 +660,7 @@ void diffInternal(Item other, Collection deltas, bool if (delta != null && !delta.isEmpty()) { ((Collection)deltas).add(delta); } + return !delta.isEmpty(); } protected ItemDelta fixupDelta(ItemDelta delta, Item other) { diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java index 1ae7a3950ff..9d7abf7580c 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java @@ -44,6 +44,14 @@ */ public class PrismContainerValueImpl extends PrismValueImpl implements PrismContainerValue { + + + public static final RuntimeException DIFFERENT_ITEMS_EXCEPTION = new ItemDifferentException(); + + static { + DIFFERENT_ITEMS_EXCEPTION.fillInStackTrace(); + } + // We use linked map because we need to maintain the order internally to provide consistent // output in DOM and other ordering-sensitive representations. // The QNames here should be qualified if at all possible. Unqualified names are kept here nevertheless @@ -105,6 +113,7 @@ public PrismContext getPrismContext() { } // Primarily for testing + @Override public PrismContext getPrismContextLocal() { return prismContext; } @@ -119,6 +128,7 @@ public PrismContext getPrismContextLocal() { * * @return set of items that the property container contains. */ + @Override @NotNull public Collection> getItems() { if (isImmutable()) { @@ -140,6 +150,7 @@ public PrismContext getPrismContextLocal() { return rv; } + @Override public int size() { return items.size(); } @@ -154,6 +165,7 @@ public int size() { * * @return set of properties that the property container contains. */ + @Override @NotNull public Set> getProperties() { Set> properties = new HashSet<>(); @@ -165,15 +177,18 @@ public Set> getProperties() { return properties; } + @Override public Long getId() { return id; } + @Override public void setId(Long id) { checkMutable(); this.id = id; } + @Override @SuppressWarnings("unchecked") public PrismContainerable getParent() { Itemable parent = super.getParent(); @@ -187,6 +202,7 @@ public PrismContainerable getParent() { return (PrismContainerable) parent; } + @Override @SuppressWarnings("unchecked") public PrismContainer getContainer() { Itemable parent = super.getParent(); @@ -200,6 +216,7 @@ public PrismContainer getContainer() { return (PrismContainer) super.getParent(); } + @Override @NotNull public ItemPath getPath() { Itemable parent = getParent(); @@ -220,10 +237,12 @@ protected Object getPathComponent() { } // For compatibility with other PrismValue types + @Override public C getValue() { return asContainerable(); } + @Override @NotNull public C asContainerable() { if (containerable != null) { @@ -235,6 +254,7 @@ public C asContainerable() { return asContainerableInternal(resolveClass(null)); } + @Override public Class getCompileTimeClass() { if (containerable != null) { return (Class) containerable.getClass(); @@ -243,6 +263,7 @@ public Class getCompileTimeClass() { } } + @Override public boolean canRepresent(Class clazz) { Class compileTimeClass = getCompileTimeClass(); if (compileTimeClass == null) { @@ -252,6 +273,7 @@ public boolean canRepresent(Class clazz) { } // returned class must be of type 'requiredClass' (or any of its subtypes) + @Override public C asContainerable(Class requiredClass) { if (containerable != null) { return containerable; @@ -310,11 +332,13 @@ private C asContainerableInternal(Class clazz) { } } + @Override @NotNull public Collection getItemNames() { return new ArrayList<>(items.keySet()); } + @Override public void add(Item item) throws SchemaException { add(item, true); } @@ -325,6 +349,7 @@ public void add(Item * @param item item to add. * @throws IllegalArgumentException an attempt to add value that already exists */ + @Override public void add(Item item, boolean checkUniqueness) throws SchemaException { checkMutable(); ItemName itemName = item.getElementName(); @@ -357,6 +382,7 @@ private void simpleAdd(Item boolean merge(Item item) throws SchemaException { checkMutable(); Item existingItem = findItem(item.getElementName(), Item.class); @@ -382,6 +408,7 @@ public boolean merge(Item boolean subtract(Item item) throws SchemaException { checkMutable(); Item existingItem = findItem(item.getElementName(), Item.class); @@ -403,6 +430,7 @@ public boolean subtract(Item< * * @param item item to add. */ + @Override public void addReplaceExisting(Item item) throws SchemaException { checkMutable(); if (item == null) { @@ -412,6 +440,7 @@ public void addReplaceExistin add(item); } + @Override public void remove(@NotNull Item item) { checkMutable(); @@ -424,6 +453,7 @@ public void remove(@NotNull I } } + @Override public void removeAll() { checkMutable(); Iterator> iterator = items.values().iterator(); @@ -441,6 +471,7 @@ public void removeAll() { * @param itemsToAdd items to add * @throws IllegalArgumentException an attempt to add value that already exists */ + @Override public void addAll(Collection> itemsToAdd) throws SchemaException { for (Item item : itemsToAdd) { add(item); @@ -452,6 +483,7 @@ public void addAll(Collection> itemsToAdd) throws SchemaExc * * @param itemsToAdd items to add */ + @Override public void addAllReplaceExisting(Collection> itemsToAdd) throws SchemaException { checkMutable(); for (Item item : itemsToAdd) { @@ -459,21 +491,25 @@ public void addAllReplaceExisting(Collection> itemsToAdd) t } } + @Override public void replace(Item oldItem, Item newItem) throws SchemaException { remove(oldItem); add(newItem); } + @Override public void clear() { checkMutable(); items.clear(); unqualifiedItemNames.clear(); } + @Override public boolean contains(Item item) { return items.values().contains(item); } + @Override public boolean contains(ItemName itemName) { return findItem(itemName) != null; } @@ -525,8 +561,9 @@ public PartiallyResolvedItem< return subItem.findPartial(rest); } + @Override public PrismProperty findProperty(ItemPath propertyPath) { - return (PrismProperty) findItem(propertyPath, PrismProperty.class); + return findItem(propertyPath, PrismProperty.class); } /** @@ -537,6 +574,7 @@ public PrismProperty findProperty(ItemPath propertyPath) { * @param propertyDefinition property definition to find. * @return found property or null */ + @Override public PrismProperty findProperty(PrismPropertyDefinition propertyDefinition) { if (propertyDefinition == null) { throw new IllegalArgumentException("No property definition"); @@ -544,15 +582,18 @@ public PrismProperty findProperty(PrismPropertyDefinition propertyDefi return findProperty(propertyDefinition.getItemName()); } + @Override public PrismContainer findContainer(QName containerName) { return findItem(ItemName.fromQName(containerName), PrismContainer.class); } + @Override public PrismReference findReference(QName elementName) { return findItem(ItemName.fromQName(elementName), PrismReference.class); } // todo optimize this some day + @Override public PrismReference findReferenceByCompositeObjectElementName(QName elementName) { for (Item item : items.values()) { if (item instanceof PrismReference) { @@ -568,6 +609,7 @@ public PrismReference findReferenceByCompositeObjectElementName(QName elementNam return null; } + @Override public > I findItem(ItemPath itemPath, Class type) { try { return findCreateItem(itemPath, type, null, false); @@ -599,6 +641,7 @@ > I fin } } + @Override public > I findItem(ItemDefinition itemDefinition, Class type) { if (itemDefinition == null) { throw new IllegalArgumentException("No item definition"); @@ -606,6 +649,7 @@ public return findItem(itemDefinition.getItemName(), type); } + @Override public boolean containsItem(ItemPath path, boolean acceptEmptyItem) throws SchemaException { if (!path.startsWithName()) { throw new IllegalArgumentException("Attempt to lookup item using a non-name path " + path + " in " + this); @@ -642,7 +686,7 @@ > I fin if (item != null) { if (rest.isEmpty()) { if (type.isAssignableFrom(item.getClass())) { - return (I) item; + return item; } else { if (create) { throw new SchemaException("The " + type.getSimpleName() + " cannot be created because " @@ -711,6 +755,7 @@ private Item findItem return matching; } + @Override public > I createDetachedSubItem(QName name, Class type, ID itemDefinition, boolean immutable) throws SchemaException { I newItem = createDetachedNewItemInternal(name, type, itemDefinition); @@ -762,30 +807,37 @@ private PrismContainer findOrCreateContainer(QName containerName) throws SchemaException { return findCreateItem(containerName, PrismContainer.class, null, true); } + @Override public PrismReference findOrCreateReference(QName referenceName) throws SchemaException { return findCreateItem(referenceName, PrismReference.class, null, true); } + @Override public Item findOrCreateItem(QName containerName) throws SchemaException { return findCreateItem(containerName, Item.class, null, true); } + @Override public > I findOrCreateItem(QName containerName, Class type) throws SchemaException { return findCreateItem(containerName, type, null, true); } + @Override public > I findOrCreateItem(ItemPath path, Class type, ID definition) throws SchemaException { return findCreateItem(path, type, definition, true); } + @Override public PrismProperty findOrCreateProperty(ItemPath propertyPath) throws SchemaException { return findOrCreateItem(propertyPath, PrismProperty.class, null); } + @Override public PrismProperty findOrCreateProperty(PrismPropertyDefinition propertyDef) throws SchemaException { PrismProperty property = findItem(propertyDef.getItemName(), PrismProperty.class); if (property != null) { @@ -794,6 +846,7 @@ public PrismProperty findOrCreateProperty(PrismPropertyDefinition propert return createProperty(propertyDef); } + @Override public PrismProperty createProperty(QName propertyName) throws SchemaException { checkMutable(); PrismPropertyDefinition propertyDefinition = determineItemDefinition(propertyName, getComplexTypeDefinition()); @@ -814,20 +867,24 @@ public PrismProperty createProperty(QName propertyName) throws SchemaExce return property; } + @Override public PrismProperty createProperty(PrismPropertyDefinition propertyDefinition) throws SchemaException { PrismProperty property = propertyDefinition.instantiate(); add(property); return property; } + @Override public void removeProperty(ItemPath path) { removeItem(path, PrismProperty.class); } + @Override public void removeContainer(ItemPath path) { removeItem(path, PrismContainer.class); } + @Override public void removeReference(ItemPath path) { removeItem(path, PrismReference.class); } @@ -876,6 +933,7 @@ private void removeUnqualifiedItemName(ItemName itemName) { unqualifiedItemNames.remove(itemName.getLocalPart()); } + @Override public void setPropertyRealValue(QName propertyName, T realValue, PrismContext prismContext) throws SchemaException { checkMutable(); PrismProperty property = findOrCreateProperty(ItemName.fromQName(propertyName)); @@ -885,6 +943,7 @@ public void setPropertyRealValue(QName propertyName, T realValue, PrismConte } } + @Override public T getPropertyRealValue(QName propertyName, Class type) { PrismProperty property = findProperty(ItemName.fromQName(propertyName)); if (property == null) { // when using sql repo, even non-existing properties do not have 'null' here @@ -939,6 +998,7 @@ public void acceptParentVisitor(Visitor visitor) { } } + @Override public boolean hasCompleteDefinition() { for (Item item : getItems()) { if (!item.hasCompleteDefinition()) { @@ -977,10 +1037,10 @@ private boolean representsSameValue(PrismContainerValue other, boolean lax) { } @Override - public void diffMatchingRepresentation(PrismValue otherValue, - Collection deltas, ParameterizedEquivalenceStrategy strategy) { + public boolean diffMatchingRepresentation(PrismValue otherValue, + Collection deltas, ParameterizedEquivalenceStrategy strategy, boolean exitOnDiff) { if (otherValue instanceof PrismContainerValue) { - diffRepresentation((PrismContainerValue) otherValue, deltas, strategy); + return diffItems(this, (PrismContainerValue) otherValue, deltas, strategy, exitOnDiff); } else { throw new IllegalStateException("Comparing incompatible values " + this + " - " + otherValue); } @@ -988,7 +1048,7 @@ public void diffMatchingRepresentation(PrismValue otherValue, private void diffRepresentation(PrismContainerValue otherValue, Collection deltas, ParameterizedEquivalenceStrategy strategy) { - diffItems(this, otherValue, deltas, strategy); + diffItems(this, otherValue, deltas, strategy, false); } @Override @@ -996,6 +1056,7 @@ public boolean isRaw() { return false; } + @Override public boolean addRawElement(Object element) throws SchemaException { checkMutable(); PrismContainerDefinition definition = getDefinition(); @@ -1008,6 +1069,7 @@ public boolean addRawElement(Object element) throws SchemaException { } } + @Override public boolean deleteRawElement(Object element) throws SchemaException { checkMutable(); PrismContainerDefinition definition = getDefinition(); @@ -1020,6 +1082,7 @@ public boolean deleteRawElement(Object element) throws SchemaException { } } + @Override public boolean removeRawElement(Object element) { checkMutable(); throw new UnsupportedOperationException("Definition-less containers are not supported any more."); @@ -1030,8 +1093,8 @@ private Item parseRaw return jaxbDomHack.parseRawElement(element, definition); } - private void diffItems(PrismContainerValue thisValue, PrismContainerValue other, - Collection deltas, ParameterizedEquivalenceStrategy strategy) { + private boolean diffItems(PrismContainerValue thisValue, PrismContainerValue other, + Collection deltas, ParameterizedEquivalenceStrategy strategy, boolean exitOnDiff) { for (Item thisItem : thisValue.getItems()) { Item otherItem = other.findItem(thisItem.getElementName()); @@ -1046,7 +1109,10 @@ private void diffItems(PrismContainerValue thisValue, PrismContainerValue } } // The "delete" delta will also result from the following diff - ((ItemImpl) thisItem).diffInternal(otherItem, deltas, false, strategy); + boolean different = ((ItemImpl) thisItem).diffInternal(otherItem, deltas, false, strategy, exitOnDiff); + if(different && exitOnDiff) { + return true; + } } for (Item otherItem : other.getItems()) { @@ -1066,12 +1132,16 @@ private void diffItems(PrismContainerValue thisValue, PrismContainerValue } // Other has an item that we don't have, this must be an add + if (exitOnDiff) { + return true; + } ItemDelta itemDelta = otherItem.createDelta(); itemDelta.addValuesToAdd(otherItem.getClonedValues()); if (!itemDelta.isEmpty()) { ((Collection) deltas).add(itemDelta); } } + return false; } private boolean isOperationalOnly(Item item, ItemDefinition itemDef) { @@ -1119,6 +1189,7 @@ public void applyDefinition(ItemDefinition definition, boolean force) throws Sch applyDefinition((PrismContainerDefinition) definition, force); } + @Override public void applyDefinition(@NotNull PrismContainerDefinition containerDef, boolean force) throws SchemaException { checkMutable(); ComplexTypeDefinition definitionToUse = containerDef.getComplexTypeDefinition(); @@ -1240,6 +1311,7 @@ public boolean hasNoItems() { return items.isEmpty(); } + @Override public boolean isIdOnly() { return id != null && hasNoItems(); } @@ -1275,10 +1347,12 @@ public void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitio } } + @Override public void assertDefinitions(String sourceDescription) throws SchemaException { assertDefinitions(false, sourceDescription); } + @Override public void assertDefinitions(boolean tolerateRaw, String sourceDescription) throws SchemaException { for (Item item : getItems()) { item.assertDefinitions(tolerateRaw, "value(" + getId() + ") in " + sourceDescription); @@ -1353,7 +1427,7 @@ public boolean equals(PrismValue other, @NotNull ParameterizedEquivalenceStrateg return other instanceof PrismContainerValue && equals((PrismContainerValue) other, strategy); } - private boolean equals(PrismContainerValue other, ParameterizedEquivalenceStrategy strategy) { + private boolean equals(@NotNull PrismContainerValue other, ParameterizedEquivalenceStrategy strategy) { if (!super.equals(other, strategy)) { return false; } @@ -1370,11 +1444,20 @@ private boolean equals(PrismContainerValue other, ParameterizedEquivalenceStr } private boolean equalsItems(PrismContainerValue other, ParameterizedEquivalenceStrategy strategy) { - Collection> deltas = new ArrayList<>(); - diffItems(this, other, deltas, strategy); + Collection> deltas = FailOnAddList.INSTANCE; + try { + boolean different = diffItems(this, other, deltas, strategy, true); + if (different) { + return false; + } + } catch (ItemDifferentException e) { + // Exception is only for fast escape + return false; + } return deltas.isEmpty(); } + @Override public boolean equivalent(PrismContainerValue other) { return equals(other, EquivalenceStrategy.REAL_VALUE); } @@ -1472,6 +1555,7 @@ public QName getTypeName() { return getComplexTypeDefinition() != null ? getComplexTypeDefinition().getTypeName() : null; } + @Override @Nullable public ComplexTypeDefinition getComplexTypeDefinition() { if (complexTypeDefinition == null) { @@ -1553,6 +1637,7 @@ public T getRealValue() { * * @param itemName Item name for newly-created container. */ + @Override public PrismContainer asSingleValuedContainer(@NotNull QName itemName) throws SchemaException { PrismContext prismContext = getPrismContext(); Validate.notNull(prismContext, "Prism context is null"); @@ -1568,6 +1653,7 @@ public PrismContainer asSingleValuedContainer(@NotNull QName itemName) throws // EXPERIMENTAL. TODO write some tests // BEWARE, it expects that definitions for items are present. Otherwise definition-less single valued items will get overwritten. + @Override @Experimental @SuppressWarnings("unchecked") public void mergeContent(@NotNull PrismContainerValue other, @NotNull List overwrite) throws SchemaException { @@ -1603,6 +1689,7 @@ public static Collection asContainerables(Collectio /** * Set origin type to all values and subvalues */ + @Override public void setOriginTypeRecursive(final OriginType originType) { accept((visitable) -> { if (visitable instanceof PrismValue) { @@ -1612,6 +1699,7 @@ public void setOriginTypeRecursive(final OriginType originType) { } // TODO optimize a bit + test thoroughly + @Override public void keepPaths(List keep) throws SchemaException { Collection itemNames = getItemNames(); for (QName itemName : itemNames) { @@ -1634,6 +1722,7 @@ public void keepPaths(List keep) throws SchemaException { } // TODO optimize a bit + test thoroughly + @Override public void removePaths(List remove) throws SchemaException { Collection itemNames = getItemNames(); for (QName itemName : itemNames) { @@ -1705,6 +1794,7 @@ public Object getIdentifier() { } // BEWARE!! Assumes the container has no parent! Otherwise item.getPath() provides wrong values. + @Override public void removeItems(List itemsToRemove) { for (ItemPath itemToRemove : itemsToRemove) { Item item = findItem(itemToRemove); // reduce to "removeItem" after fixing that method implementation @@ -1714,6 +1804,7 @@ public void removeItems(List itemsToRemove) { } } + @Override public void removeOperationalItems() { accept(visitable -> { if (visitable instanceof Item) { @@ -1727,4 +1818,31 @@ public void removeOperationalItems() { } }); } + + private static class FailOnAddList extends AbstractList> { + + public static final FailOnAddList INSTANCE = new FailOnAddList(); + + @Override + public ItemDelta get(int index) { + return null; + } + + @Override + public int size() { + return 0; + } + + @Override + public boolean add(ItemDelta e) { + throw new IllegalStateException("Should not happen"); + //throw DIFFERENT_ITEMS_EXCEPTION; + } + } + + private static class ItemDifferentException extends RuntimeException { + + private static final long serialVersionUID = 1L; + + } } diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismValueImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismValueImpl.java index e24d6665f32..234e1527488 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismValueImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismValueImpl.java @@ -70,14 +70,17 @@ public abstract class PrismValueImpl extends AbstractFreezable implements PrismV this.parent = parent; } + @Override public void setPrismContext(PrismContext prismContext) { this.prismContext = prismContext; } + @Override public void setOriginObject(Objectable source) { this.originObject = source; } + @Override public void setOriginType(OriginType type) { this.originType = type; } @@ -92,6 +95,7 @@ public Objectable getOriginObject() { return originObject; } + @Override public Map getUserData() { return userData; } @@ -175,6 +179,7 @@ public void applyDefinition(ItemDefinition definition, boolean force) throws Sch // Do nothing by default } + @Override public void revive(PrismContext prismContext) throws SchemaException { if (this.prismContext == null) { this.prismContext = prismContext; @@ -193,6 +198,7 @@ public void recompute() { recompute(getPrismContext()); } + @Override public abstract void recompute(PrismContext prismContext); @Override @@ -211,8 +217,10 @@ public void accept(Visitor visitor, ItemPath path, boolean recursive) { } } + @Override public abstract void checkConsistenceInternal(Itemable rootItem, boolean requireDefinitions, boolean prohibitRaw, ConsistencyCheckScope scope); + @Override public boolean representsSameValue(PrismValue other, boolean lax) { return false; } @@ -225,6 +233,7 @@ public void normalize() { /** * Literal clone. */ + @Override public abstract PrismValue clone(); @Override @@ -238,6 +247,7 @@ public PrismValue createImmutableClone() { * Complex clone with different cloning strategies. * @see CloneStrategy */ + @Override public abstract PrismValue cloneComplex(CloneStrategy strategy); protected void copyValues(CloneStrategy strategy, PrismValueImpl clone) { @@ -282,6 +292,7 @@ public boolean equals(PrismValue otherValue, @NotNull EquivalenceStrategy equiva } } + @Override @SuppressWarnings("SimplifiableIfStatement") public boolean equals(PrismValue other, @NotNull ParameterizedEquivalenceStrategy strategy) { // parent is not considered at all. it is not relevant. @@ -295,6 +306,7 @@ public boolean equals(PrismValue other, @NotNull ParameterizedEquivalenceStrateg } // original equals was "isLiteral = false"! + @Override public boolean equals(Object other) { return this == other || (other == null || other instanceof PrismValue) && @@ -312,9 +324,15 @@ public Collection diff(PrismValue otherValue, Parameterized return itemDeltas; } - public void diffMatchingRepresentation(PrismValue otherValue, + public final void diffMatchingRepresentation(PrismValue otherValue, Collection deltas, ParameterizedEquivalenceStrategy strategy) { + diffMatchingRepresentation(otherValue, deltas, strategy, false); + } + + public boolean diffMatchingRepresentation(PrismValue otherValue, + Collection deltas, ParameterizedEquivalenceStrategy strategy, boolean exitOnDiff) { // Nothing to do by default + return false; } protected void appendOriginDump(StringBuilder builder) { @@ -328,11 +346,14 @@ protected void appendOriginDump(StringBuilder builder) { } } + @Override public abstract String toHumanReadableString(); + @Override @Nullable abstract public Class getRealClass(); + @Override @Nullable abstract public T getRealValue(); @@ -341,6 +362,7 @@ protected void appendOriginDump(StringBuilder builder) { // of AccessCertificationCampaignType. // // Generally, this method returns either "this" (PrismValue) or a PrismContainerValue. + @Override public PrismValue getRootValue() { PrismValue current = this; for (;;) { @@ -352,10 +374,12 @@ public PrismValue getRootValue() { } } + @Override public PrismContainerValue getParentContainerValue() { return PrismValueUtil.getParentContainerValue(this); } + @Override public QName getTypeName() { ItemDefinition definition = getDefinition(); return definition != null ? definition.getTypeName() : null; @@ -363,6 +387,7 @@ public QName getTypeName() { // Path may contain ambiguous segments (e.g. assignment/targetRef when there are more assignments) // Note that the path can contain name segments only (at least for now) + @Override @NotNull public Collection getAllValues(ItemPath path) { if (path.isEmpty()) { diff --git a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java index 32b6bf7d97d..cb6bde8f6ac 100644 --- a/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java +++ b/infra/schema/src/test/java/com/evolveum/midpoint/schema/performance/PerfTestPrismObjectSize.java @@ -42,7 +42,7 @@ public class PerfTestPrismObjectSize extends AbstractSchemaPerformanceTest { private static final int[] CONTAINER_COUNTS = new int[] { 10, 20, 50, 100, 200, 500, 1000}; - private static final int[] DELTA_PERCENTS = new int[] {10,20,30}; + private static final int[] DELTA_OP_COUNT = new int[] {1,2,5,10,20,50,100,200}; @Test(dataProvider = "combinations") @@ -104,11 +104,11 @@ private void measureDelta(ContainerTestParams config, String name, CheckedFunct @Test(dataProvider = "combinations") public void replaceDelta(ContainerTestParams config) throws SchemaException { - for (int percent : DELTA_PERCENTS) { - int opCount = config.count * percent / 100; + for (int maxOps : DELTA_OP_COUNT) { + int opCount = Math.min(config.count, maxOps); measureDelta(config, monitorName("replace",config.monitorId(), Integer.toString(opCount)), assignments -> { DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); - for (int i = assignments.size() - 1 - opCount; i < assignments.size(); i++) { + for (int i = assignments.size() - opCount; i < assignments.size(); i++) { AssignmentType assignment = assignments.get(i).clone(); assignment.description("Modified"); delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).replace(assignment.asPrismContainerValue()); @@ -120,12 +120,12 @@ public void replaceDelta(ContainerTestParams config) throws SchemaException { @Test(dataProvider = "combinations") public void addDelta(ContainerTestParams config) throws SchemaException { - for (int percent : DELTA_PERCENTS) { - int opCount = config.count * percent / 100; + for (int maxOps : DELTA_OP_COUNT) { + int opCount = maxOps; measureDelta(config, monitorName("add",config.monitorId(), Integer.toString(opCount)), assignments -> { DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); for (int i = 0; i < opCount; i++) { - AssignmentType assignment = assignments.get(i).clone(); + AssignmentType assignment = assignments.get(0).clone(); assignment.getConstruction().resourceRef(newUuid(), assignment.getConstruction().getResourceRef().getType()); assignment.setId(null); delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).add(assignment.asPrismContainerValue()); @@ -137,11 +137,11 @@ public void addDelta(ContainerTestParams config) throws SchemaException { @Test(dataProvider = "combinations") public void deleteDelta(ContainerTestParams config) throws SchemaException { - for (int percent : DELTA_PERCENTS) { - int opCount = config.count * percent / 100; + for (int maxOps : DELTA_OP_COUNT) { + int opCount = Math.min(config.count, maxOps); measureDelta(config, monitorName("delete",config.monitorId(), Integer.toString(opCount)), assignments -> { DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); - for (int i = assignments.size() - 1 - opCount; i < assignments.size(); i++) { + for (int i = assignments.size() - opCount; i < assignments.size(); i++) { AssignmentType assignment = assignments.get(i).clone(); delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).delete(assignment.asPrismContainerValue()); } @@ -153,11 +153,11 @@ public void deleteDelta(ContainerTestParams config) throws SchemaException { @Test(dataProvider = "combinations") public void deleteNoIdDelta(ContainerTestParams config) throws SchemaException { - for (int percent : DELTA_PERCENTS) { - int opCount = config.count * percent / 100; + for (int maxOps : DELTA_OP_COUNT) { + int opCount = Math.min(config.count, maxOps); measureDelta(config, monitorName("delete.no.id",config.monitorId(), Integer.toString(opCount)), assignments -> { DeltaBuilder delta = new DeltaBuilder<>(UserType.class, getPrismContext()); - for (int i = assignments.size() - 1 - opCount; i < assignments.size(); i++) { + for (int i = assignments.size() - opCount; i < assignments.size(); i++) { AssignmentType assignment = assignments.get(i).clone(); delta = (DeltaBuilder) delta.item(UserType.F_ASSIGNMENT).delete(assignment.asPrismContainerValue()); } From 317a3fc5c6080d50a1bf39be1a780d0089b61ef9 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Wed, 12 May 2021 11:11:32 +0200 Subject: [PATCH 4/5] PCV Equals: Disable IllegalStateException if slow path is used Signed-off-by: Tony Tkacik --- .../evolveum/midpoint/prism/impl/PrismContainerValueImpl.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java index 9d7abf7580c..a796b952d09 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java @@ -1835,8 +1835,7 @@ public int size() { @Override public boolean add(ItemDelta e) { - throw new IllegalStateException("Should not happen"); - //throw DIFFERENT_ITEMS_EXCEPTION; + throw DIFFERENT_ITEMS_EXCEPTION; } } From 05ab5b0c3b7081f697015fd7c6b80afb3b4cb019 Mon Sep 17 00:00:00 2001 From: Tony Tkacik Date: Wed, 12 May 2021 12:37:54 +0200 Subject: [PATCH 5/5] Fixed early exit on special case [] == null Signed-off-by: Tony Tkacik --- .../com/evolveum/midpoint/prism/impl/ItemImpl.java | 2 +- .../midpoint/prism/impl/PrismContainerValueImpl.java | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) 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 927ed55b9d3..e8a6b7ec1d6 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 @@ -594,7 +594,7 @@ boolean diffInternal(Item other, Collection deltas, b ItemDelta delta = createDelta(); if (other == null) { // Early exit for equals use case - if (exitOnDiff) { + if (exitOnDiff && hasAnyValue()) { return true; } diff --git a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java index a796b952d09..59fce528ba3 100644 --- a/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java +++ b/infra/prism-impl/src/main/java/com/evolveum/midpoint/prism/impl/PrismContainerValueImpl.java @@ -47,6 +47,7 @@ public class PrismContainerValueImpl extends PrismValue public static final RuntimeException DIFFERENT_ITEMS_EXCEPTION = new ItemDifferentException(); + private static final boolean EARLY_EXIT = true; static { DIFFERENT_ITEMS_EXCEPTION.fillInStackTrace(); @@ -1131,11 +1132,16 @@ private boolean diffItems(PrismContainerValue thisValue, PrismContainerValue< } } - // Other has an item that we don't have, this must be an add + // Other item has no values + if(otherItem.hasNoValues()) { + continue; + } if (exitOnDiff) { return true; } + ItemDelta itemDelta = otherItem.createDelta(); + itemDelta.addValuesToAdd(otherItem.getClonedValues()); if (!itemDelta.isEmpty()) { ((Collection) deltas).add(itemDelta); @@ -1446,7 +1452,7 @@ private boolean equals(@NotNull PrismContainerValue other, ParameterizedEquiv private boolean equalsItems(PrismContainerValue other, ParameterizedEquivalenceStrategy strategy) { Collection> deltas = FailOnAddList.INSTANCE; try { - boolean different = diffItems(this, other, deltas, strategy, true); + boolean different = diffItems(this, other, deltas, strategy, EARLY_EXIT); if (different) { return false; }