Partial fix for manual connector race conditions (MID-4049)

infra/schema/src/main/java/com/evolveum/midpoint/schema/SearchResultList.java

@@ -1,5 +1,5 @@
 /**
- * Copyright (c) 2014-2016 Evolveum
+ * Copyright (c) 2014-2017 Evolveum
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -63,11 +63,14 @@ public void setMetadata(SearchResultMetadata metadata) {
 	}
 
 	public int size() {
+		if (list == null) {
+			return 0;
+		}
 		return list.size();
 	}
 
 	public boolean isEmpty() {
-		return list.isEmpty();
+		return list == null || list.isEmpty();
 	}
 
 	public boolean contains(Object o) {
@@ -87,7 +90,7 @@ public <T> T[] toArray(T[] a) {
 	}
 
 	public boolean add(T e) {
-		return list.add(e);
+		return getInitializedList().add(e);
 	}
 
 	public boolean remove(Object o) {
@@ -213,4 +216,12 @@ public SearchResultList<T> clone() {
 		}
 		return clone;
 	}
+
+	private List<T> getInitializedList() {
+		if (list == null) {
+			list = new ArrayList<>();
+		}
+		return list;
+	}
+
 }

infra/schema/src/main/java/com/evolveum/midpoint/schema/result/OperationConstants.java

@@ -51,4 +51,6 @@ public class OperationConstants {
 	public static final String AUDIT_REINDEX = PREFIX + ".auditReindex";
 	public static final String SHADOW_REFRESH = PREFIX + ".shadowRefresh";
 
+	public static final String OPERATION_SEARCH_RESULT = "com.evolveum.midpoint.schema.result.searchResult";
+
 }

infra/schema/src/main/java/com/evolveum/midpoint/schema/util/ResourceTypeUtil.java

@@ -636,4 +636,16 @@ public static ObjectSynchronizationType findObjectSynchronization(@Nullable Reso
 	public static QName fillDefaultFocusType(QName focusType) {
 		return focusType != null ? focusType : UserType.COMPLEX_TYPE;
 	}
+
+	public static ShadowCheckType getShadowConstaintsCheck(ResourceType resource) {
+		ResourceConsistencyType consistency = resource.getConsistency();
+		if (consistency == null) {
+			return ShadowCheckType.NONE;
+		}
+		ShadowCheckType shadowCheckType = consistency.getShadowConstaintsCheck();
+		if (shadowCheckType == null) {
+			return ShadowCheckType.NONE;
+		}
+		return shadowCheckType;
+	}
 }

infra/schema/src/main/resources/xml/ns/public/common/common-core-3.xsd

@@ -4420,6 +4420,18 @@
         			</xsd:documentation>
         		</xsd:annotation>
         	</xsd:element>
+        	<xsd:element name="shadowConstaintsCheck" type="tns:ShadowCheckType" minOccurs="0" default="none">
+        		<xsd:annotation>
+        			<xsd:documentation>
+        				Shadow constraint uniqueness setting. It mostly applies to shadow uniqueness.
+        				Uniqueness is checked by the resource under normal circumstances. This option
+        				can be used to turn on additional checks. Those checks may be needed to diagnose
+        				configuration issues or bugs (e.g. "duplicate shadow" problems). Or it may be
+        				used in case that the resource cannot check uniqueness by itself (e.g. in case of
+        				manual or asynchronous resources).
+        			</xsd:documentation>
+        		</xsd:annotation>
+        	</xsd:element>
             <xsd:element name="caseIgnoreAttributeNames" type="xsd:boolean" minOccurs="0" default="false">
                 <xsd:annotation>
                     <xsd:documentation>
@@ -4469,6 +4481,47 @@
         <xsd:attribute name="id" type="xsd:long" use="optional"/>
     </xsd:complexType>
 
+    <xsd:simpleType name="ShadowCheckType">
+        <xsd:annotation>
+            <xsd:documentation>
+                Shadow constraint uniqueness setting. It mostly applies to shadow uniqueness.
+   				Uniqueness is checked by the resource under normal circumstances. This option
+   				can be used to turn on additional checks. Those checks may be needed to diagnose
+   				configuration issues or bugs (e.g. "duplicate shadow" problems). Or it may be
+   				used in case that the resource cannot check uniqueness by itself (e.g. in case of
+   				manual or asynchronous resources).
+            </xsd:documentation>
+            <xsd:appinfo>
+                <jaxb:typesafeEnumClass/>
+            </xsd:appinfo>
+        </xsd:annotation>
+        <xsd:restriction base="xsd:string">
+            <xsd:enumeration value="none">
+                <xsd:annotation>
+                	<xsd:documentation>
+                		No additional checks.
+                	</xsd:documentation>
+                    <xsd:appinfo>
+                        <jaxb:typesafeEnumMember name="NONE"/>
+                    </xsd:appinfo>
+                </xsd:annotation>
+            </xsd:enumeration>
+            <xsd:enumeration value="light">
+                <xsd:annotation>
+                	<xsd:documentation>
+                		Light checks only.
+                		E.g. the shadow uniqueness will be checked only once
+                		when new shadow is created.
+                	</xsd:documentation>
+                    <xsd:appinfo>
+                        <jaxb:typesafeEnumMember name="LIGHT"/>
+                    </xsd:appinfo>
+                </xsd:annotation>
+                <!-- TODO: thorough: always check uniqueness -->
+            </xsd:enumeration>
+        </xsd:restriction>
+    </xsd:simpleType>
+
     <xsd:complexType name="ErrorSelectorType">
         <xsd:annotation>
             <xsd:documentation>

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/controller/ModelController.java

@@ -1297,6 +1297,7 @@ public PrismObject<? extends FocusType> searchShadowOwner(String shadowOid, Coll
 		return focus;
 	}
 
+	@Deprecated
 	@Override
 	public List<PrismObject<? extends ShadowType>> listResourceObjects(String resourceOid,
 			QName objectClass, ObjectPaging paging, Task task, OperationResult parentResult) throws SchemaException,

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/lens/projector/ShadowConstraintsChecker.java

@@ -124,11 +124,9 @@ public void check(Task task, OperationResult result) throws SchemaException, Obj
 			return;
 		}
 
-		ConstraintViolationConfirmer confirmer = new ConstraintViolationConfirmer() {
-			@Override
-			public boolean confirmViolation(String oid) {
+		ConstraintViolationConfirmer confirmer = (conflictingShadowCandidate) -> {
 				boolean violation = true;
-				LensProjectionContext foundContext = context.findProjectionContextByOid(oid);
+				LensProjectionContext foundContext = context.findProjectionContextByOid(conflictingShadowCandidate.getOid());
 				if (foundContext != null) {
 					if (foundContext.getResourceShadowDiscriminator() != null) {
 						if (foundContext.getResourceShadowDiscriminator().isThombstone()) {
@@ -138,8 +136,7 @@ public boolean confirmViolation(String oid) {
 					}
 				}
 				return violation;
-			}
-		};
+			};
 
 		constraintsCheckingResult = provisioningService.checkConstraints(projOcDef, projectionNew,
 				projectionContext.getResource(), projectionContext.getOid(), projectionContext.getResourceShadowDiscriminator(),

model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/manual/AbstractManualResourceTest.java

@@ -29,6 +29,7 @@
 import java.io.IOException;
 import java.util.Collection;
 import java.util.List;
+import java.util.Random;
 
 import javax.xml.datatype.XMLGregorianCalendar;
 import javax.xml.namespace.QName;
@@ -154,6 +155,8 @@ public abstract class AbstractManualResourceTest extends AbstractConfiguredModel
 	protected static final QName ATTR_DESCRIPTION_QNAME = new QName(MidPointConstants.NS_RI, ATTR_DESCRIPTION);
 
 	protected static final String INTEREST_ONE = "one";
+
+	protected static final Random RND = new Random();
 
 
 	protected PrismObject<ResourceType> resource;
@@ -2563,7 +2566,7 @@ public void test840AddToBackingStoreAndGetAccountWill() throws Exception {
 	}
 
 	// MID-4047
-	@Test(enabled = false)
+	@Test
 	public void test900ConcurrentConstructions() throws Exception {
 		final String TEST_NAME = "test900ConcurrentConstructions";
 		displayTestTile(TEST_NAME);
@@ -2580,10 +2583,12 @@ public void test900ConcurrentConstructions() throws Exception {
 		for (int i = 0; i < THREADS; i++) {
 			threads[i] = new Thread(() -> {
 				try {
+					Thread.sleep(RND.nextInt(1000)); // Random start delay
+					LOGGER.info("{} starting", Thread.currentThread().getName());
 					login(userAdministrator);
 					Task localTask = createTask(TEST_NAME + ".local");
 					assignAccount(USER_BARBOSSA_OID, getResourceOid(), SchemaConstants.INTENT_DEFAULT, localTask, localTask.getResult());
-				} catch (CommonException e) {
+				} catch (CommonException | InterruptedException e) {
 					throw new SystemException("Couldn't assign resource: " + e.getMessage(), e);
 				}
 			});

model/model-intest/src/test/resources/manual/resource-manual.xml

@@ -127,6 +127,7 @@
 	<!-- Capabilities will be generated by the manual connector. -->
 
 	<consistency>
+		<shadowConstaintsCheck>light</shadowConstaintsCheck>
 		<pendingOperationGracePeriod>PT15M</pendingOperationGracePeriod>
 	</consistency>
 

model/model-intest/src/test/resources/manual/resource-semi-manual-disable.xml

@@ -173,6 +173,7 @@
 
 	<consistency>
 		<avoidDuplicateValues>true</avoidDuplicateValues>
+		<shadowConstaintsCheck>light</shadowConstaintsCheck>
 		<pendingOperationGracePeriod>PT15M</pendingOperationGracePeriod>
 	</consistency>
 

model/model-intest/src/test/resources/manual/resource-semi-manual.xml

@@ -143,6 +143,7 @@
 	</capabilities>
 
 	<consistency>
+		<shadowConstaintsCheck>light</shadowConstaintsCheck>
 		<pendingOperationGracePeriod>PT15M</pendingOperationGracePeriod>
 	</consistency>
 

provisioning/provisioning-api/src/main/java/com/evolveum/midpoint/provisioning/api/ConstraintViolationConfirmer.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010-2014 Evolveum
+ * Copyright (c) 2010-2017 Evolveum
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -16,11 +16,18 @@
 
 package com.evolveum.midpoint.provisioning.api;
 
+import com.evolveum.midpoint.prism.PrismObject;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType;
+
 /**
  * @author mederly
  */
 public interface ConstraintViolationConfirmer {
 
-    boolean confirmViolation(String oid);
+	/**
+	 * Returns true if the candidate conflicts with the shadow being checked.
+	 * Returns false if this is not a conflicting shadow.
+	 */
+    boolean confirmViolation(PrismObject<ShadowType> conflictingShadowCandidate);
 
 }

provisioning/provisioning-api/src/main/java/com/evolveum/midpoint/provisioning/api/ProvisioningService.java

@@ -429,6 +429,7 @@ List<ConnectorOperationalStatus> getConnectorOperationalStatus(String resourceOi
 	 * @throws SchemaException error handling resource schema
 	 * @throws CommunicationException error communicating with the resource
 	 */
+	@Deprecated
 	List<PrismObject<? extends ShadowType>> listResourceObjects(String resourceOid, QName objectClass, ObjectPaging paging,
 				Task task, OperationResult parentResult) 
 						throws SchemaException, ObjectNotFoundException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException;

provisioning/provisioning-impl/src/main/java/com/evolveum/midpoint/provisioning/impl/ConstraintsChecker.java

@@ -73,9 +73,16 @@ public class ConstraintsChecker {
 	private static ThreadLocal<Cache> cacheThreadLocal = new ThreadLocal<>();
 
 	private PrismContext prismContext;
-	private RepositoryService cacheRepositoryService;
-	private ProvisioningService provisioningService;
+	private RepositoryService repositoryService;
+	private ShadowCache shadowCache;
 	private StringBuilder messageBuilder = new StringBuilder();
+	private RefinedObjectClassDefinition shadowDefinition;
+	private PrismObject<ShadowType> shadowObject;
+	private ResourceType resourceType;
+	private String shadowOid;
+	private ResourceShadowDiscriminator resourceShadowDiscriminator;
+	private ConstraintViolationConfirmer constraintViolationConfirmer;
+	private boolean useCache = true;
 
 	public PrismContext getPrismContext() {
 		return prismContext;
@@ -85,20 +92,17 @@ public void setPrismContext(PrismContext prismContext) {
 		this.prismContext = prismContext;
 	}
 
-	public void setCacheRepositoryService(RepositoryService cacheRepositoryService) {
-		this.cacheRepositoryService = cacheRepositoryService;
+	public void setRepositoryService(RepositoryService repositoryService) {
+		this.repositoryService = repositoryService;
 	}
 
-	public void setProvisioningService(ProvisioningService provisioningService) {
-		this.provisioningService = provisioningService;
+	public ShadowCache getShadowCache() {
+		return shadowCache;
 	}
 
-	private RefinedObjectClassDefinition shadowDefinition;
-	private PrismObject<ShadowType> shadowObject;
-	private ResourceType resourceType;
-	private String shadowOid;
-	private ResourceShadowDiscriminator resourceShadowDiscriminator;
-	private ConstraintViolationConfirmer constraintViolationConfirmer;
+	public void setShadowCache(ShadowCache shadowCache) {
+		this.shadowCache = shadowCache;
+	}
 
 	public void setShadowDefinition(RefinedObjectClassDefinition shadowDefinition) {
 		this.shadowDefinition = shadowDefinition;
@@ -124,6 +128,14 @@ public void setConstraintViolationConfirmer(ConstraintViolationConfirmer constra
 		this.constraintViolationConfirmer = constraintViolationConfirmer;
 	}
 
+	public boolean isUseCache() {
+		return useCache;
+	}
+
+	public void setUseCache(boolean useCache) {
+		this.useCache = useCache;
+	}
+
 	private ConstraintsCheckingResult constraintsCheckingResult;
 
 	public ConstraintsCheckingResult check(Task task, OperationResult result) throws SchemaException, ObjectAlreadyExistsException, ObjectNotFoundException, CommunicationException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
@@ -140,7 +152,7 @@ public ConstraintsCheckingResult check(Task task, OperationResult result) throws
 
 		Collection<? extends ResourceAttributeDefinition> uniqueAttributeDefs = MiscUtil.unionExtends(shadowDefinition.getPrimaryIdentifiers(),
 				shadowDefinition.getSecondaryIdentifiers());
-		LOGGER.trace("Secondary IDs {}", shadowDefinition.getSecondaryIdentifiers());
+		LOGGER.trace("Checking uniquenss of attributes: {}", uniqueAttributeDefs);
 		for (ResourceAttributeDefinition attrDef: uniqueAttributeDefs) {
 			PrismProperty<?> attr = attributesContainer.findProperty(attrDef.getName());
 			LOGGER.trace("Attempt to check uniqueness of {} (def {})", attr, attrDef);
@@ -184,7 +196,7 @@ private boolean checkAttributeUniqueness(PrismProperty identifier, RefinedObject
 
 	private boolean checkUniqueness(String oid, PrismProperty identifier, ObjectQuery query, Task task, OperationResult result) throws SchemaException, ObjectNotFoundException, SecurityViolationException, CommunicationException, ConfigurationException, ExpressionEvaluationException {
 
-		if (Cache.isOk(resourceType.getOid(), oid, shadowDefinition.getTypeName(), identifier.getDefinition().getName(), identifier.getValues())) {
+		if (useCache && Cache.isOk(resourceType.getOid(), oid, shadowDefinition.getTypeName(), identifier.getDefinition().getName(), identifier.getValues())) {
 			return true;
 		}
 
@@ -193,13 +205,15 @@ private boolean checkUniqueness(String oid, PrismProperty identifier, ObjectQuer
 		// because there could be a matching rule; see ShadowManager.processQueryMatchingRuleFilter.
 		// Besides that, now the constraint checking is cached at a higher level, so this is not a big issue any more.
 		Collection<SelectorOptions<GetOperationOptions>> options = SelectorOptions.createCollection(GetOperationOptions.createNoFetch());
-		List<PrismObject<ShadowType>> foundObjects = provisioningService.searchObjects(ShadowType.class, query, options, task, result);
+		List<PrismObject<ShadowType>> foundObjects = shadowCache.searchObjects(query, options, true, task, result);
 		if (LOGGER.isTraceEnabled()) {
 			LOGGER.trace("Uniqueness check of {} resulted in {} results, using query:\n{}",
 					identifier, foundObjects.size(), query.debugDump());
 		}
 		if (foundObjects.isEmpty()) {
-			Cache.setOk(resourceType.getOid(), oid, shadowDefinition.getTypeName(), identifier.getDefinition().getName(), identifier.getValues());
+			if (useCache) {
+				Cache.setOk(resourceType.getOid(), oid, shadowDefinition.getTypeName(), identifier.getDefinition().getName(), identifier.getValues());
+			}
 			return true;
 		}
 		if (foundObjects.size() > 1) {
@@ -220,11 +234,13 @@ private boolean checkUniqueness(String oid, PrismProperty identifier, ObjectQuer
 						+ foundObjects.get(0).debugDump());
 			}
 			message("Found conflicting existing object with attribute " + identifier.toHumanReadableString() + ": " + foundObjects.get(0));
-			match = !constraintViolationConfirmer.confirmViolation(foundObjects.get(0).getOid());
+			match = !constraintViolationConfirmer.confirmViolation(foundObjects.get(0));
 			constraintsCheckingResult.setConflictingShadow(foundObjects.get(0));
 			// we do not cache "OK" here because the violation confirmer could depend on attributes/items that are not under our observations
 		} else {
-			Cache.setOk(resourceType.getOid(), oid, shadowDefinition.getTypeName(), identifier.getDefinition().getName(), identifier.getValues());
+			if (useCache) {
+				Cache.setOk(resourceType.getOid(), oid, shadowDefinition.getTypeName(), identifier.getDefinition().getName(), identifier.getValues());
+			}
 			return true;
 		}
 		return match;