Yet another attempt at fixing MID-2102 (iterator fails if no shadow e…

…xists).

This time we kill primary delta if it's ADD one and the object already exists.
Also: Accepting null from countObjects in AbstractSearchIterativeTaskHandler.

infra/prism/src/main/java/com/evolveum/midpoint/prism/query/ObjectPaging.java

@@ -103,7 +103,7 @@ public void setMaxSize(Integer maxSize) {
 	 * when the next page of the same search is requested.
 	 * 
 	 * It is OK to initialize a search without any cookie. If the datastore utilizes a re-usable
-	 * context it will return a coockie in a search response.
+	 * context it will return a cookie in a search response.
 	 */
 	public String getCookie() {
 		return cookie;
@@ -118,7 +118,7 @@ public String getCookie() {
 	 * when the next page of the same search is requested.
 	 * 
 	 * It is OK to initialize a search without any cookie. If the datastore utilizes a re-usable
-	 * context it will return a coockie in a search response.
+	 * context it will return a cookie in a search response.
 	 */
 	public void setCookie(String cookie) {
 		this.cookie = cookie;

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

@@ -649,7 +649,7 @@ void executeDelta(ObjectDelta<T> objectDelta, LensElementContext<T> objectContex
 	    	}
     	}
     }
-	
+
 	private <T extends ObjectType, F extends FocusType> void removeExecutedItemDeltas(
 			ObjectDelta<T> objectDelta, LensElementContext<T> objectContext) {
 		if (objectContext == null){
@@ -690,6 +690,20 @@ private <T extends ObjectType, F extends FocusType> boolean alreadyExecuted(
 		return ObjectDeltaOperation.containsDelta(objectContext.getExecutedDeltas(), objectDelta);
 	}
 
+	/**
+	 * Was this object already added? (temporary method, should be removed soon)
+	 */
+	private <T extends ObjectType> boolean wasAdded(List<LensObjectDeltaOperation<T>> executedOperations, String oid) {
+		for (LensObjectDeltaOperation operation : executedOperations) {
+			if (operation.getObjectDelta().isAdd() &&
+					oid.equals(operation.getObjectDelta().getOid()) &&
+					!operation.getExecutionResult().isFatalError()) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	/**
 	 * Computes delta to execute, given a list of already executes deltas. See below.
 	 */
@@ -717,6 +731,9 @@ private <T extends ObjectType> ObjectDelta<T> computeDeltaToExecute(
 	 *
 	 * Unfortunately, this mechanism is not well-defined, and seems to work more "by accident" than "by design".
 	 * It should be replaced with something more serious. Perhaps by re-reading current focus state when repeating a wave?
+	 * Actually, it is a supplement for rewriting ADD->MODIFY deltas in LensElementContext.getFixedPrimaryDelta.
+	 * That method converts primary deltas (and as far as I know, that is the only place where this problem should occur).
+	 * Nevertheless, for historical and safety reasons I keep also the processing in this method.
 	 *
 	 * Anyway, currently it treats only two cases:
 	 * 1) if the objectDelta is present in the list of executed deltas
@@ -730,7 +747,7 @@ private <T extends ObjectType> ObjectDelta<T> computeDiffDelta(List<? extends Ob
 
 		ObjectDeltaOperation<T> lastRelated = findLastRelatedDelta(executedDeltas, objectDelta);		// any delta related to our OID, not ending with fatal error
 		if (LOGGER.isTraceEnabled()) {
-			LOGGER.trace("findLastRelatedDelta returned:\n{}", objectDelta.debugDump());
+			LOGGER.trace("findLastRelatedDelta returned:\n{}", lastRelated!=null ? lastRelated.debugDump() : "(null)");
 		}
 		if (lastRelated == null) {
 			return objectDelta;			// nothing found, let us apply our delta

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

@@ -182,6 +182,37 @@ public void setObjectNew(PrismObject<O> objectNew) {
 	public ObjectDelta<O> getPrimaryDelta() {
 		return primaryDelta;
 	}
+
+	/**
+	 * As getPrimaryDelta() but caters for the possibility that an object already exists.
+	 * So, if the primary delta is ADD and object already exists, it should be changed somehow,
+	 * e.g. to MODIFY delta or to null.
+	 *
+	 * Actually, the question is what to do with the attribute values if changed to MODIFY.
+	 * (a) Should they become REPLACE item deltas? (b) ADD ones?
+	 * (c) Or should we compute a difference from objectCurrent to objectToAdd, hoping that
+	 * secondary deltas will re-add everything that might be unknowingly removed by this step?
+	 * (d) Or should we simply ignore ADD delta altogether, hoping that it was executed
+	 * so it need not be repeated?
+	 *
+	 * And, should not we report AlreadyExistingException instead?
+	 *
+	 * It seems that (c) i.e. reverting back to objectToAdd is not a good idea at all. For example, this
+	 * may erase linkRefs for good.
+	 *
+	 * For the time being let us proceed with (d), i.e. ignoring such a delta.
+	 *
+	 * TODO is this OK???? [med]
+	 *
+	 * @return
+	 */
+	public ObjectDelta<O> getFixedPrimaryDelta() {
+		if (primaryDelta == null || !primaryDelta.isAdd() || objectCurrent == null) {
+			return primaryDelta;		// nothing to do
+		}
+		// Object does exist. Let's ignore the delta - see description above.
+		return null;
+	}
 
 	public void setPrimaryDelta(ObjectDelta<O> primaryDelta) {
 		this.primaryDelta = primaryDelta;
@@ -294,8 +325,12 @@ public void addToExecutedDeltas(LensObjectDeltaOperation<O> executedDelta) {
     public ObjectDelta<O> getDelta() throws SchemaException {
         return ObjectDelta.union(primaryDelta, getSecondaryDelta());
     }
-
-    public ObjectDeltaObject<O> getObjectDeltaObject() throws SchemaException {
+
+	public ObjectDelta<O> getFixedDelta() throws SchemaException {
+		return ObjectDelta.union(getFixedPrimaryDelta(), getSecondaryDelta());
+	}
+
+	public ObjectDeltaObject<O> getObjectDeltaObject() throws SchemaException {
 		return new ObjectDeltaObject<O>(objectOld, getDelta(), objectNew);
 	}
 

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

@@ -97,9 +97,9 @@ public void setOid(String oid) {
 
 	public ObjectDelta<O> getProjectionWavePrimaryDelta() throws SchemaException {
     	if (getProjectionWave() == 0) {
-    		return getPrimaryDelta();
+    		return getFixedPrimaryDelta();
     	} else {
-    		return secondaryDeltas.getMergedDeltas(getPrimaryDelta(), getProjectionWave());
+    		return secondaryDeltas.getMergedDeltas(getFixedPrimaryDelta(), getProjectionWave());
     	}
     }
 
@@ -212,9 +212,9 @@ public ObjectDelta<O> getWaveDelta(int wave) throws SchemaException {
     	if (wave == 0) {
     		// Primary delta is executed only in the first wave (wave 0)
     		if (LensUtil.isSyncChannel(getLensContext().getChannel())){
-    			return ObjectDelta.union(getPrimaryDelta(), getWaveSecondaryDelta(wave));
+    			return ObjectDelta.union(getFixedPrimaryDelta(), getWaveSecondaryDelta(wave));
     		} else {
-    			return ObjectDelta.union(getPrimaryDelta(), getWaveSecondaryDelta(wave), getWaveSecondaryDelta(1));
+    			return ObjectDelta.union(getFixedPrimaryDelta(), getWaveSecondaryDelta(wave), getWaveSecondaryDelta(1));
     		}
     	} else {
     		return getWaveSecondaryDelta(wave);
@@ -223,8 +223,8 @@ public ObjectDelta<O> getWaveDelta(int wave) throws SchemaException {
 
     public ObjectDelta<O> getWaveExecutableDelta(int wave) throws SchemaException {
     	if (wave == 0){
-    		if (getPrimaryDelta() != null && getPrimaryDelta().isAdd()){
-    			ObjectDelta delta = getPrimaryDelta();
+    		if (getFixedPrimaryDelta() != null && getFixedPrimaryDelta().isAdd()){
+    			ObjectDelta delta = getFixedPrimaryDelta();
     			for (ObjectDelta<O> secondary : getSecondaryDeltas()){
     				if (secondary != null){
     					secondary.applyTo(delta.getObjectToAdd());

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

@@ -688,8 +688,10 @@ private void distributeResourceValues(Collection<PrismReferenceValue> values, Pr
 	 */
 	public ObjectDelta<ShadowType> getExecutableDelta() throws SchemaException {
 		SynchronizationPolicyDecision policyDecision = getSynchronizationPolicyDecision();
-		ObjectDelta<ShadowType> origDelta = getDelta();
+		ObjectDelta<ShadowType> origDelta = getFixedDelta();
 		if (policyDecision == SynchronizationPolicyDecision.ADD) {
+			// let's try to retrieve original (non-fixed) delta. Maybe it's ADD delta so we spare fixing it.
+			origDelta = getDelta();
             if (origDelta == null || origDelta.isModify()) {
             	// We need to convert modify delta to ADD
             	ObjectDelta<ShadowType> addDelta = new ObjectDelta<ShadowType>(getObjectTypeClass(),

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/AbstractSearchIterativeResultHandler.java

@@ -16,6 +16,7 @@
 package com.evolveum.midpoint.model.impl.util;
 
 import com.evolveum.midpoint.prism.PrismProperty;
+import com.evolveum.midpoint.repo.cache.RepositoryCache;
 import com.evolveum.midpoint.schema.constants.SchemaConstants;
 import com.evolveum.midpoint.schema.result.OperationResultStatus;
 import com.evolveum.midpoint.task.api.LightweightTaskHandler;
@@ -247,6 +248,8 @@ private void processRequest(ProcessingRequest request, Task workerTask, Operatio
 
 		try {
 
+			RepositoryCache.enter();
+
 			if (LOGGER.isTraceEnabled()) {
 				LOGGER.trace("{} starting for {} {}",new Object[] {
 						getProcessShortNameCapitalized(), object, getContextDesc()});
@@ -272,6 +275,8 @@ private void processRequest(ProcessingRequest request, Task workerTask, Operatio
 		} catch (CommonException|RuntimeException e) {
 			cont = processError(object, e, result);
 		} finally {
+			RepositoryCache.exit();
+
 			long duration = System.currentTimeMillis()-startTime;
 			long total = totalTimeProcessing.addAndGet(duration);
 			int progress = objectsProcessed.incrementAndGet();

model/model-impl/src/main/java/com/evolveum/midpoint/model/impl/util/AbstractSearchIterativeTaskHandler.java

@@ -140,8 +140,11 @@ public TaskRunResult run(Task coordinatorTask) {
             // counting objects can be within try-catch block, because the handling is similar to handling errors within searchIterative
             Long expectedTotal = null;
             if (countObjectsOnStart) {
-                expectedTotal = (long) modelObjectResolver.countObjects(type, query, opResult);
+                Integer expectedTotalInt = modelObjectResolver.countObjects(type, query, opResult);
                 LOGGER.trace("{}: expecting {} objects to be processed", taskName, expectedTotal);
+                if (expectedTotalInt != null) {
+                    expectedTotal = (long) expectedTotalInt;        // conversion would fail on null
+                }
             }
 
             runResult.setProgress(0);

model/model-intest/src/test/java/com/evolveum/midpoint/model/intest/TestIteration.java

@@ -141,6 +141,9 @@ public class TestIteration extends AbstractInitializedModelIntegrationTest {
 	private static final String USER_JUPITER_NAME = "jupiter";
 	private static final String ACCOUNT_JUPITER_DUMMY_FUCHSIA_USERNAME = "Jupiter Jones";
 
+	private static final File USER_ALFRED_FILE = new File(TEST_DIR, "user-alfred.xml");
+	private static final String USER_ALFRED_NAME = "alfred";
+
 	private static final String USER_BOB_NAME = "bob";
 
 	protected String jupiterUserOid;
@@ -474,6 +477,56 @@ public void test220DeWattAssignAccountDummyPinkCaseIgnore() throws Exception {
         dummyAuditService.assertExecutionSuccess();
 	}
 
+	@Test
+	public void test230ScroogeAddAccountDummyConflictingNoShadow() throws Exception {
+		final String TEST_NAME = "test230ScroogeAddAccountDummyConflictingNoShadow";
+		TestUtil.displayTestTile(this, TEST_NAME);
+
+		// GIVEN
+		Task task = taskManager.createTaskInstance(TestIteration.class.getName() + "." + TEST_NAME);
+		OperationResult result = task.getResult();
+		dummyAuditService.clear();
+
+		// Make sure there is a conflicting account and NO shadow for it
+		DummyAccount account = new DummyAccount("scrooge");
+		account.setEnabled(true);
+		account.addAttributeValues(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_FULLNAME_NAME, "Scrooge Pinky");
+		dummyResourcePink.addAccount(account);
+
+		PrismObject<UserType> userScrooge = createUser("scrooge", "Scrooge McDuck", true);
+		ShadowType newPinkyShadow = createShadow(resourceDummyPinkType.asPrismObject(), null, null).asObjectable();
+		userScrooge.asObjectable().getLink().add(newPinkyShadow);
+
+		Collection<ObjectDelta<? extends ObjectType>> deltas = new ArrayList<ObjectDelta<? extends ObjectType>>();
+		deltas.add(ObjectDelta.createAddDelta(userScrooge));
+
+		// WHEN
+		TestUtil.displayWhen(TEST_NAME);
+		modelService.executeChanges(deltas, null, task, result);
+
+		// THEN
+		TestUtil.displayThen(TEST_NAME);
+		result.computeStatus();
+		TestUtil.assertSuccess(result);
+
+		PrismObject<UserType> userScroogeAfter = findUserByUsername("scrooge");
+		display("User after change execution", userScroogeAfter);
+		assertUser(userScroogeAfter, null, "scrooge", "Scrooge McDuck", null, null, null);
+		String accountOid = getSingleLinkOid(userScroogeAfter);
+
+		// Check shadow
+		PrismObject<ShadowType> accountShadow = repositoryService.getObject(ShadowType.class, accountOid, null, result);
+		assertAccountShadowRepo(accountShadow, accountOid, "scrooge1", resourceDummyPinkType);
+
+		// Check account
+		PrismObject<ShadowType> accountModel = modelService.getObject(ShadowType.class, accountOid, null, task, result);
+		assertAccountShadowModel(accountModel, accountOid, "scrooge1", resourceDummyPinkType);
+
+		// Check account in dummy resource
+		assertDummyAccount(RESOURCE_DUMMY_PINK_NAME, "scrooge1", "Scrooge McDuck", true);
+	}
+
+
 	@Test
 	public void test240LargoAssignAccountDummyConflictingNoShadow() throws Exception {
 		final String TEST_NAME = "test240LargoAssignAccountDummyConflictingNoShadow";
@@ -939,6 +992,55 @@ public void test282RenamePeterNoShadowSync() throws Exception {
 		assertNoDummyAccount(RESOURCE_DUMMY_PINK_NAME, "peter");
 	}
 
+	// as with jupiter, but ADD instead of ASSIGN account
+	@Test
+	public void test290AlfredConflictNoShadowSyncBackAdd() throws Exception {
+		final String TEST_NAME = "test290AlfredConflictNoShadowSyncBackAdd";
+		TestUtil.displayTestTile(this, TEST_NAME);
+
+		// GIVEN
+		Task task = taskManager.createTaskInstance(TestIteration.class.getName() + "." + TEST_NAME);
+		OperationResult result = task.getResult();
+		dummyAuditService.clear();
+
+		// Make sure there is a conflicting account and NO shadow for it
+		DummyAccount account = new DummyAccount("Alfred Hitchcock");
+		account.setEnabled(true);
+		account.addAttributeValues(DummyResourceContoller.DUMMY_ACCOUNT_ATTRIBUTE_AD_SAM_ACCOUNT_NAME_NAME, "alfred");
+		dummyResourceFuchsia.addAccount(account);
+
+		Collection<ObjectDelta<? extends ObjectType>> deltas = new ArrayList<>();
+		PrismObject<UserType> userJupiter = PrismTestUtil.parseObject(USER_ALFRED_FILE);			// there is a linked account
+		deltas.add(ObjectDelta.createAddDelta(userJupiter));
+
+		// WHEN
+		TestUtil.displayWhen(TEST_NAME);
+		modelService.executeChanges(deltas, null, task, result);
+
+		// THEN
+		TestUtil.displayThen(TEST_NAME);
+		result.computeStatus();
+		TestUtil.assertSuccess(result);
+
+		PrismObject<UserType> userAlfredAfter = findUserByUsername(USER_ALFRED_NAME);		// alfred
+		display("User after change execution", userAlfredAfter);
+		assertUser(userAlfredAfter, null, USER_ALFRED_NAME, "Alfred Hitchcock", "Alfred", "Hitchcock", null);
+		String accountOid = getSingleLinkOid(userAlfredAfter);
+
+		// Check shadow
+		PrismObject<ShadowType> accountShadow = repositoryService.getObject(ShadowType.class, accountOid, null, result);
+		display("Account shadow from repo", accountShadow);
+		assertAccountShadowRepo(accountShadow, accountOid, "Alfred Hitchcock", resourceDummyFuchsiaType);
+
+		// Check account
+		PrismObject<ShadowType> accountModel = modelService.getObject(ShadowType.class, accountOid, null, task, result);
+		assertAccountShadowModel(accountModel, accountOid, "Alfred Hitchcock", resourceDummyFuchsiaType);
+
+		// Check account in dummy resource (actually, the fullname attribute does not exist but it's OK)
+		assertDummyAccount(RESOURCE_DUMMY_FUCHSIA_NAME, "Alfred Hitchcock", null, true);
+	}
+
+
 
 	@Test
     public void test300JackAssignAccountDummyVioletConflicting() throws Exception {

model/model-intest/src/test/resources/iteration/user-alfred.xml

@@ -0,0 +1,48 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<!--
+  ~ Copyright (c) 2010-2014 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
+		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'
+        xmlns:ri="http://midpoint.evolveum.com/xml/ns/public/resource/instance-3">
+    <name>alfred</name>
+	<fullName>Alfred Hitchcock</fullName>
+    <givenName>Alfred</givenName>
+    <familyName>Hitchcock</familyName>
+	<activation>
+		<validFrom>1777-05-30T14:15:00Z</validFrom>
+		<validTo>2222-02-22T08:30:42Z</validTo>
+	</activation>
+    <link>
+        <resourceRef oid="10000000-0000-0000-0000-0000000dd204"/>           <!-- fuchsia -->
+        <objectClass>ri:AccountObjectClass</objectClass>
+        <activation>
+            <administrativeStatus>enabled</administrativeStatus>
+        </activation>
+    </link>
+	 <credentials>
+        <password>
+        	<value>
+	            <clearValue>dead123</clearValue>
+            </value>
+        </password>
+    </credentials>
+</user>