Fixes and tests for manual connectors with repeated operations (MID-4…

…587)

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

@@ -39,6 +39,8 @@ public interface BackingStore {
 
 	void addPhantom()  throws IOException;
 
+	void addPhoenix()  throws IOException;
+
 	void deleteAccount(String username) throws IOException;
 
 	void displayContent() throws IOException;

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

@@ -125,11 +125,23 @@ public void addPhantom() throws IOException {
 				// Wrong fullname here ... by purpose. We wonder whether reconciliation fixes this.
 				AbstractManualResourceTest.USER_PHANTOM_FULL_NAME_WRONG,
 				AbstractManualResourceTest.ACCOUNT_PHANTOM_DESCRIPTION_MANUAL, 
-				"", 
+				"",
 				"false",
 				AbstractManualResourceTest.ACCOUNT_PHANTOM_PASSWORD_MANUAL
 			});
 	}
+
+	@Override
+	public void addPhoenix() throws IOException {
+		appendToCsv(new String[]{
+				AbstractManualResourceTest.USER_PHOENIX_USERNAME,
+				AbstractManualResourceTest.USER_PHOENIX_FULL_NAME,
+				AbstractManualResourceTest.ACCOUNT_PHOENIX_DESCRIPTION_MANUAL, 
+				"",
+				"false",
+				AbstractManualResourceTest.ACCOUNT_PHOENIX_PASSWORD_MANUAL
+			});
+	}
 
 
 	@Override

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

@@ -24,20 +24,27 @@
 
 import java.io.File;
 
+import javax.xml.namespace.QName;
+
 import org.springframework.test.annotation.DirtiesContext;
 import org.springframework.test.annotation.DirtiesContext.ClassMode;
 import org.springframework.test.context.ContextConfiguration;
 import org.testng.annotations.Listeners;
+import org.testng.annotations.Test;
 
 import com.evolveum.midpoint.prism.PrismObject;
+import com.evolveum.midpoint.prism.path.ItemPath;
+import com.evolveum.midpoint.schema.constants.MidPointConstants;
 import com.evolveum.midpoint.schema.constants.SchemaConstants;
 import com.evolveum.midpoint.schema.result.OperationResult;
 import com.evolveum.midpoint.task.api.Task;
 import com.evolveum.midpoint.util.exception.ObjectNotFoundException;
 import com.evolveum.midpoint.util.logging.Trace;
 import com.evolveum.midpoint.util.logging.TraceManager;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ActivationStatusType;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.ObjectReferenceType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.OperationResultStatusType;
+import com.evolveum.midpoint.xml.ns._public.common.common_3.PendingOperationExecutionStatusType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.PendingOperationType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.ShadowType;
 import com.evolveum.midpoint.xml.ns._public.common.common_3.UserType;
@@ -163,6 +170,106 @@ protected void cleanupUser(final String TEST_NAME, String userOid, String userna
 		assertLinks(userAfter, 0);
 		assertNoShadow(accountOid);
 	}
+
+	/**
+	 * MID-4587
+	 */
+	@Test
+	@Override
+	public void test416PhoenixAccountUnassignCloseCase() throws Exception {
+		final String TEST_NAME = "test416PhoenixAccountUnassignCloseCase";
+		displayTestTitle(TEST_NAME);
+		// GIVEN
+		Task task = createTask(TEST_NAME);
+		OperationResult result = task.getResult();
+
+		closeCase(phoenixLastCaseOid);
+
+		// WHEN
+		displayWhen(TEST_NAME);
+		reconcileUser(USER_PHOENIX_OID, task, result);
+
+		// THEN
+		displayThen(TEST_NAME);
+		assertSuccess(result);
+
+		// Make sure the operation will be picked up by propagation task
+		clockForward("PT3M");
+
+		// WHEN
+		displayWhen(TEST_NAME);
+		runPropagation();
+
+		// THEN
+		displayThen(TEST_NAME);
+
+		PrismObject<UserType> userAfter = getUser(USER_PHOENIX_OID);
+		display("User after", userAfter);
+		String shadowOid = getSingleLinkOid(userAfter);
+		PrismObject<ShadowType> shadowModel = getShadowModel(shadowOid);
+		display("Shadow after", shadowModel);
+
+		// Shadow NOT dead. We are disabling instead of deleting
+		assertShadowNotDead(shadowModel);
+		assertAttribute(shadowModel, ATTR_USERNAME_QNAME, USER_PHOENIX_USERNAME);
+		// Semi-manual ... we still see old activationStatus value
+		assertActivationAdministrativeStatus(shadowModel, ActivationStatusType.ENABLED);
+
+		assertSinglePendingOperation(shadowModel, PendingOperationExecutionStatusType.COMPLETED, OperationResultStatusType.SUCCESS);
+
+		assertSteadyResources();
+	}
+
+	@Test
+	@Override
+	public void test418AssignPhoenixAccountAgain() throws Exception {
+		final String TEST_NAME = "test418AssignPhoenixAccountAgain";
+		displayTestTitle(TEST_NAME);
+		// GIVEN
+		Task task = createTask(TEST_NAME);
+		OperationResult result = task.getResult();
+
+		// WHEN
+		displayWhen(TEST_NAME);
+		assignAccount(USER_PHOENIX_OID, getResourceOid(), null, task, result);
+
+		// THEN
+		displayThen(TEST_NAME);
+		phoenixLastCaseOid = assertInProgress(result);
+
+		// Make sure the operation will be picked up by propagation task
+		clockForward("PT3M");
+
+		// WHEN
+		displayWhen(TEST_NAME);
+		runPropagation();
+
+		// THEN
+		displayThen(TEST_NAME);
+
+		PrismObject<UserType> userAfter = getUser(USER_PHOENIX_OID);
+		display("User after", userAfter);
+
+		String shadowOid = getSingleLinkOid(userAfter);
+		PrismObject<ShadowType> shadowModel = getShadowModel(shadowOid);
+		display("Shadow after", shadowModel);
+
+		assertShadowNotDead(shadowModel);
+		assertAttribute(shadowModel, ATTR_USERNAME_QNAME, USER_PHOENIX_USERNAME);
+		assertAttribute(shadowModel, ATTR_FULLNAME_QNAME, USER_PHOENIX_FULL_NAME);
+
+		assertPendingOperationDeltas(shadowModel, 2);
+		PendingOperationType disableOperation = findPendingOperation(shadowModel, OperationResultStatusType.SUCCESS, ChangeTypeType.MODIFY);
+		assertPendingOperation(shadowModel, disableOperation,
+						PendingOperationExecutionStatusType.COMPLETED, OperationResultStatusType.SUCCESS);
+		assertNotNull("Null completion timestamp", disableOperation.getCompletionTimestamp());
+		PendingOperationType enableOperation = findPendingOperation(shadowModel, PendingOperationExecutionStatusType.EXECUTING, null, 
+				ChangeTypeType.MODIFY, SchemaConstants.PATH_ACTIVATION_ADMINISTRATIVE_STATUS);
+		assertPendingOperation(shadowModel, enableOperation,
+				PendingOperationExecutionStatusType.EXECUTING, OperationResultStatusType.IN_PROGRESS);
+
+		assertSteadyResources();
+	}
 
 	@Override
 	protected void assertTest526Deltas(PrismObject<ShadowType> shadowRepo, OperationResult result) {

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

@@ -171,7 +171,7 @@ public void test400PhantomAccount() throws Exception {
 		PendingOperationType pendingOperation = assertSinglePendingOperation(shadowMid1NoFetch, 
 				PendingOperationExecutionStatusType.EXECUTION_PENDING, null);
 
-		clockForward("PT20M");
+		clockForward("PT3M");
 
 		// WHEN (mid2)
 		displayWhen(TEST_NAME, "mid2");
@@ -197,10 +197,11 @@ public void test400PhantomAccount() throws Exception {
 
 		assertAttribute(shadowMid2NoFetch, ATTR_USERNAME_QNAME, USER_PHANTOM_USERNAME);
 
-		assertPendingOperationDeltas(shadowMid2NoFetch, 1);
-//		PendingOperationType fizzledAddOperation = findPendingOperation(shadowMid2NoFetch, OperationResultStatusType.HANDLED_ERROR, ChangeTypeType.ADD);
-//		assertPendingOperation(shadowMid2NoFetch, fizzledAddOperation,
-//						PendingOperationExecutionStatusType.COMPLETED, OperationResultStatusType.HANDLED_ERROR);
+		assertPendingOperationDeltas(shadowMid2NoFetch, 2);
+		PendingOperationType fizzledAddOperation = findPendingOperation(shadowMid2NoFetch, OperationResultStatusType.HANDLED_ERROR, ChangeTypeType.ADD);
+		assertPendingOperation(shadowMid2NoFetch, fizzledAddOperation,
+						PendingOperationExecutionStatusType.COMPLETED, OperationResultStatusType.HANDLED_ERROR);
+		assertNotNull("Null completion timestamp", fizzledAddOperation.getCompletionTimestamp());
 		PendingOperationType reconOperation = findPendingOperation(shadowMid2NoFetch, null, ChangeTypeType.MODIFY);
 		assertPendingOperation(shadowMid2NoFetch, reconOperation,
 				PendingOperationExecutionStatusType.EXECUTION_PENDING, null);

model/model-intest/src/test/resources/logback-test.xml

@@ -80,11 +80,11 @@
  	<logger name="com.evolveum.midpoint.model.impl.util" level="DEBUG" />
 	<logger name="com.evolveum.midpoint.model.impl.sync" level="DEBUG" />
  	<logger name="com.evolveum.midpoint.model.impl.sync.CorrelationConfirmationEvaluator" level="DEBUG" />
-    <logger name="com.evolveum.midpoint.provisioning" level="DEBUG" />
+    <logger name="com.evolveum.midpoint.provisioning" level="TRACE" />
     <logger name="com.evolveum.midpoint.provisioning.impl.ResourceManager" level="DEBUG" />
-    <logger name="com.evolveum.midpoint.provisioning.impl.ShadowCache" level="DEBUG" />
-    <logger name="com.evolveum.midpoint.provisioning.impl.ShadowManager" level="DEBUG" />
-    <logger name="com.evolveum.midpoint.provisioning.impl.task" level="DEBUG" />
+    <logger name="com.evolveum.midpoint.provisioning.impl.ShadowCache" level="TRACE" />
+    <logger name="com.evolveum.midpoint.provisioning.impl.ShadowManager" level="TRACE" />
+    <logger name="com.evolveum.midpoint.provisioning.impl.task" level="TRACE" />
     <logger name="com.evolveum.midpoint.provisioning.consistency" level="DEBUG" />
 	<logger name="com.evolveum.midpoint.expression" level="DEBUG" />
 	<logger name="com.evolveum.midpoint.model.common.expression" level="DEBUG" />

model/model-intest/src/test/resources/manual/user-phoenix.xml

@@ -0,0 +1,28 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ Copyright (c) 2018 Evolveum
+  ~
+  ~ Licensed under the Apache License, Version 2.0 (the "License");
+  ~ you may not use this file except in compliance with the License.
+  ~ You may obtain a copy of the License at
+  ~
+  ~     http://www.apache.org/licenses/LICENSE-2.0
+  ~
+  ~ Unless required by applicable law or agreed to in writing, software
+  ~ distributed under the License is distributed on an "AS IS" BASIS,
+  ~ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  ~ See the License for the specific language governing permissions and
+  ~ limitations under the License.
+  -->
+<user oid="ed2ca15a-5ccb-11e8-b62d-4b94763188e4" 
+		xmlns='http://midpoint.evolveum.com/xml/ns/public/common/common-3'
+		xmlns:c='http://midpoint.evolveum.com/xml/ns/public/common/common-3'
+		xmlns:t='http://prism.evolveum.com/xml/ns/public/types-3'
+		xmlns:xsi='http://www.w3.org/2001/XMLSchema-instance'
+		xmlns:xsd='http://www.w3.org/2001/XMLSchema'
+		xmlns:piracy='http://midpoint.evolveum.com/xml/ns/samples/piracy'>
+    <name>phoenix</name>
+    <fullName>Phoebe Phoenix</fullName>
+    <givenName>Phoebe</givenName>
+    <familyName>Phoenix</familyName>
+</user>

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

@@ -109,6 +109,7 @@ public class ResourceObjectConverter {
 	@Autowired private EntitlementConverter entitlementConverter;
 	@Autowired private MatchingRuleRegistry matchingRuleRegistry;	
 	@Autowired private ResourceObjectReferenceResolver resourceObjectReferenceResolver;
+	@Autowired private ShadowCaretaker shadowCaretaker;
 	@Autowired private Clock clock;
 	@Autowired private PrismContext prismContext;
 
@@ -232,7 +233,7 @@ private boolean hasAllIdentifiers(Collection<? extends ResourceAttribute<?>> att
 
 
 	public AsynchronousOperationReturnValue<PrismObject<ShadowType>> addResourceObject(ProvisioningContext ctx, 
-			PrismObject<ShadowType> shadow, OperationProvisioningScriptsType scripts, OperationResult parentResult)
+			PrismObject<ShadowType> shadow, OperationProvisioningScriptsType scripts, boolean skipExplicitUniquenessCheck, OperationResult parentResult)
 			throws ObjectNotFoundException, SchemaException, CommunicationException,
 			ObjectAlreadyExistsException, ConfigurationException, SecurityViolationException, ExpressionEvaluationException {
 
@@ -256,7 +257,9 @@ public AsynchronousOperationReturnValue<PrismObject<ShadowType>> addResourceObje
 			throw e;
 		}
 
-		checkForAddConflicts(ctx, shadow, result);
+		if (!skipExplicitUniquenessCheck) {
+			checkForAddConflicts(ctx, shadow, result);
+		}
 
 		Collection<Operation> additionalOperations = new ArrayList<>();
 		addExecuteScriptOperation(additionalOperations, ProvisioningOperationTypeType.ADD, scripts, resource,
@@ -448,10 +451,14 @@ public AsynchronousOperationResult deleteResourceObject(ProvisioningContext ctx,
 	}
 
 	public AsynchronousOperationReturnValue<Collection<PropertyDelta<PrismPropertyValue>>> modifyResourceObject(
-			ProvisioningContext ctx, PrismObject<ShadowType> repoShadow, OperationProvisioningScriptsType scripts,
-			Collection<? extends ItemDelta> itemDeltas, OperationResult parentResult)
-			throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException,
-			SecurityViolationException, ObjectAlreadyExistsException, ExpressionEvaluationException {
+			ProvisioningContext ctx,
+			PrismObject<ShadowType> repoShadow,
+			OperationProvisioningScriptsType scripts,
+			Collection<? extends ItemDelta> itemDeltas,
+			XMLGregorianCalendar now,
+			OperationResult parentResult)
+					throws ObjectNotFoundException, SchemaException, CommunicationException, ConfigurationException,
+						SecurityViolationException, ObjectAlreadyExistsException, ExpressionEvaluationException {
 
 		OperationResult result = parentResult.createSubresult(OPERATION_MODIFY_RESOURCE_OBJECT);
 
@@ -544,7 +551,20 @@ public AsynchronousOperationReturnValue<Collection<PropertyDelta<PrismPropertyVa
 				LOGGER.trace("Pre-reading resource shadow");
 				preReadShadow = preReadShadow(ctx, identifiers, operations, true, repoShadow, result);  // yes, we need associations here
 				if (LOGGER.isTraceEnabled()) {
-					LOGGER.trace("Pre-read object:\n{}", preReadShadow==null?null:preReadShadow.debugDump());
+					LOGGER.trace("Pre-read object (straight from the resource):\n{}", preReadShadow==null?null:preReadShadow.debugDump(1));
+				}
+				// If there are pending changes in the shadow then we have to apply to pre-read object.
+				// The pre-read object may be out of date (e.g. in case of semi-manual connectors).
+				// In that case we may falsely remove some of the modifications. E.g. in case that
+				// account is enabled, then disable and then enabled again. If backing store still
+				// has the account as enabled, then the last enable operation would be ignored.
+				// No case is created to re-enable the account. And the account stays disabled at the end.
+				List<PendingOperationType> pendingOperations = repoShadow.asObjectable().getPendingOperation();
+				if (!pendingOperations.isEmpty()) {
+					preReadShadow = shadowCaretaker.applyPendingOperations(ctx, preReadShadow, pendingOperations, true, now);
+					if (LOGGER.isTraceEnabled()) {
+						LOGGER.trace("Pre-read object (applied pending operations):\n{}", preReadShadow==null?null:preReadShadow.debugDump(1));
+					}
 				}
 			}