From f586522edbeebb8fc4c6ea829bea9aa0f2015e40 Mon Sep 17 00:00:00 2001 From: Pavol Mederly Date: Mon, 3 May 2021 23:52:11 +0200 Subject: [PATCH] Make dummy objects thread safe This is an attempt to enable DummyGroup objects to participate in multi-threaded tests. It also incorporates approximate fixing of null values handling in DummyObject: As the thread safety was ensured using ConcurrentHashMap (and corresponding Set implementation), the null values can no longer be stored in attributes. This is not a problem, as they do not belong there anyway. So we filter out such values. Unfortunately, we do not do that 100% consistently, so there are places (like sync delta creation) where such values leak. (cherry picked from commit 729d699ec466564b14a3f8eb407f5d25ed069398) (cherry picked from commit b47e62935714670c0a02d8e577d5bcb29bd4806a) --- .../icf/dummy/resource/DummyObject.java | 163 +++++++++--------- 1 file changed, 84 insertions(+), 79 deletions(-) diff --git a/icf-connectors/dummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyObject.java b/icf-connectors/dummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyObject.java index 30bd7b0704c..1f5f2d60ef8 100644 --- a/icf-connectors/dummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyObject.java +++ b/icf-connectors/dummy-resource/src/main/java/com/evolveum/icf/dummy/resource/DummyObject.java @@ -8,16 +8,8 @@ import java.io.FileNotFoundException; import java.net.ConnectException; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Date; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; +import java.util.*; +import java.util.concurrent.ConcurrentHashMap; import org.apache.commons.lang.StringUtils; @@ -25,23 +17,26 @@ import com.evolveum.midpoint.util.DebugUtil; import com.evolveum.midpoint.util.PrettyPrinter; +import static java.util.Collections.*; + /** - * @author Radovan Semancik + * TODO Treat null attribute values more consistently. * + * @author Radovan Semancik */ public abstract class DummyObject implements DebugDumpable { private String id; // private int internalId = -1; private String name; - private Map> attributes = new HashMap<>(); + private final Map> attributes = new ConcurrentHashMap<>(); private Boolean enabled = true; private Date validFrom = null; private Date validTo = null; private String lastModifier; protected DummyResource resource; - private final Set auxiliaryObjectClassNames = new HashSet<>(); + private final Set auxiliaryObjectClassNames = ConcurrentHashMap.newKeySet(); private BreakMode modifyBreakMode = null; @@ -80,33 +75,36 @@ public Boolean isEnabled() { return enabled; } - public void setEnabled(Boolean enabled) throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + public void setEnabled(Boolean enabled) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); this.enabled = enabled; - recordModify("_ENABLED", null, null, Arrays.asList(enabled)); + recordModify("_ENABLED", null, null, singletonList(enabled)); } public Date getValidFrom() { return validFrom; } - public void setValidFrom(Date validFrom) throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + public void setValidFrom(Date validFrom) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); this.validFrom = validFrom; - recordModify("_VALID_FROM", null, null, Arrays.asList(validFrom)); + recordModify("_VALID_FROM", null, null, singletonList(validFrom)); } public Date getValidTo() { return validTo; } - public void setValidTo(Date validTo) throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + public void setValidTo(Date validTo) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); this.validTo = validTo; - recordModify("_VALID_TO", null, null, Arrays.asList(validTo)); + recordModify("_VALID_TO", null, null, singletonList(validTo)); } public String getLastModifier() { @@ -130,8 +128,8 @@ public Set getAttributeNames() { } public Set getAttributeValues(String attrName, Class type) { - Set values = attributes.get(attrName); - return (Set)values; + //noinspection unchecked + return (Set) attributes.get(attrName); } public T getAttributeValue(String attrName, Class type) { @@ -139,8 +137,8 @@ public T getAttributeValue(String attrName, Class type) { if (values == null || values.isEmpty()) { return null; } - if (values.size()>1) { - throw new IllegalArgumentException("Attempt to fetch single value from a multi-valued attribute "+attrName); + if (values.size() > 1) { + throw new IllegalArgumentException("Attempt to fetch single value from a multi-valued attribute " + attrName); } return values.iterator().next(); } @@ -149,83 +147,95 @@ public String getAttributeValue(String attrName) { return getAttributeValue(attrName,String.class); } - public void replaceAttributeValue(String name, Object value) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { - Collection values = new ArrayList<>(1); - values.add(value); - replaceAttributeValues(name, values); + public void replaceAttributeValue(String name, Object value) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + replaceAttributeValues(name, createCollection(value)); } - public void replaceAttributeValues(String name, Collection values) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + private Set createCollection(Object value) { + return value != null ? singleton(value) : emptySet(); + } + + public void replaceAttributeValues(String name, Collection values) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); - Set currentValues = attributes.get(name); - if (currentValues == null) { - currentValues = new HashSet<>(); - attributes.put(name, currentValues); - } else { - currentValues.clear(); - } - currentValues.addAll(values); + + Set currentValues = getOrCreateAttributeValueSet(name); + currentValues.clear(); + + addNonNullValues(currentValues, values); checkSchema(name, values, "replace"); recordModify(name, null, null, values); } - public void replaceAttributeValues(String name, Object... values) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + private void addNonNullValues(Set currentValues, Collection values) { + for (Object value : values) { + if (value != null) { + currentValues.add(value); + } + } + } + + private Set getOrCreateAttributeValueSet(String name) { + return attributes.computeIfAbsent(name, k -> ConcurrentHashMap.newKeySet()); + } + + public void replaceAttributeValues(String name, Object... values) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); - Set currentValues = attributes.get(name); - if (currentValues == null) { - currentValues = new HashSet<>(); - attributes.put(name, currentValues); - } else { - currentValues.clear(); - } + + Set currentValues = getOrCreateAttributeValueSet(name); + currentValues.clear(); + List valuesList = Arrays.asList(values); - currentValues.addAll(valuesList); + addNonNullValues(currentValues, valuesList); checkSchema(name, valuesList, "replace"); + + // Why isn't the same code in the "collection" version of this method (above)? + // Maybe to check null-attribute handling of UCF layer, see TestDummy.test129NullAttributeValue. if (valuesList.isEmpty()) { attributes.remove(name); } - recordModify(name, null, null, Arrays.asList(values)); + recordModify(name, null, null, valuesList); } - public void addAttributeValue(String name, Object value) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { - Collection values = new ArrayList<>(1); - values.add(value); - addAttributeValues(name, values); + public void addAttributeValue(String name, Object value) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + addAttributeValues(name, createCollection(value)); } - public void addAttributeValues(String name, Collection valuesToAdd) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + public void addAttributeValues(String name, Collection valuesToAdd) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); - Set currentValues = attributes.get(name); - if (currentValues == null) { - currentValues = new HashSet<>(); - attributes.put(name, currentValues); - } - for(T valueToAdd: valuesToAdd) { + Set currentValues = getOrCreateAttributeValueSet(name); + for (T valueToAdd: valuesToAdd) { addAttributeValue(name, currentValues, valueToAdd); } recordModify(name, valuesToAdd, null, null); } - public void addAttributeValues(String name, String... valuesToAdd) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + public void addAttributeValues(String name, String... valuesToAdd) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); - Set currentValues = attributes.get(name); - if (currentValues == null) { - currentValues = new HashSet<>(); - attributes.put(name, currentValues); - } + Set currentValues = getOrCreateAttributeValueSet(name); for (Object valueToAdd: valuesToAdd) { addAttributeValue(name, currentValues, valueToAdd); } recordModify(name, Arrays.asList(valuesToAdd), null, null); } - private void addAttributeValue(String attrName, Set currentValues, Object valueToAdd) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + private void addAttributeValue(String attrName, Set currentValues, Object valueToAdd) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); + + if (valueToAdd == null) { + return; // Concurrent hash map does not allow null values. + } if (resource != null && !resource.isTolerateDuplicateValues()) { for (Object currentValue: currentValues) { if (currentValue.equals(valueToAdd)) { @@ -257,34 +267,29 @@ private void addAttributeValue(String attrName, Set currentValues, Objec currentValues.add(valueToAdd); } - public void removeAttributeValue(String name, Object value) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { - Collection values = new ArrayList<>(); - values.add(value); - removeAttributeValues(name, values); + public void removeAttributeValue(String name, Object value) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + removeAttributeValues(name, createCollection(value)); } - public void removeAttributeValues(String name, Collection values) throws SchemaViolationException, ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { + public void removeAttributeValues(String name, Collection values) + throws ConnectException, FileNotFoundException, SchemaViolationException, ConflictException, InterruptedException { checkModifyBreak(); delayOperation(); - Set currentValues = attributes.get(name); - if (currentValues == null) { - currentValues = new HashSet<>(); - attributes.put(name, currentValues); - } + Set currentValues = getOrCreateAttributeValueSet(name); - Set valuesToCheck = new HashSet<>(); - valuesToCheck.addAll(currentValues); + Set valuesToCheck = new HashSet<>(currentValues); valuesToCheck.removeAll(values); checkSchema(name, valuesToCheck, "remove"); Iterator iterator = currentValues.iterator(); boolean foundMember = false; - if (name.equals(DummyGroup.ATTR_MEMBERS_NAME) && !resource.isTolerateDuplicateValues()){ + if (name.equals(DummyGroup.ATTR_MEMBERS_NAME) && !resource.isTolerateDuplicateValues()) { checkIfExist(values, currentValues); } - while(iterator.hasNext()) { + while (iterator.hasNext()) { Object currentValue = iterator.next(); boolean found = false; for (Object value: values) { @@ -351,7 +356,7 @@ private void checkIfExist(Collection valuesToDelete, Set currentV } } - if (!found){ + if (!found) { throw new SchemaViolationException("no such member: " + valueToDelete + " in " + currentValues); } }