Merge branch 'master' of github.com:Evolveum/midpoint

model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java

@@ -3057,7 +3057,7 @@ private boolean isInProgress(OperationResult result, boolean checkSubresult) {
 
 	private OperationResult getSubresult(OperationResult result, boolean checkSubresult) {
 		if (checkSubresult) {
-			return result.getLastSubresult();
+			return result != null ? result.getLastSubresult() : null;
 		}
 		return result;
 	}

model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/activiti/ActivitiInterface.java

@@ -167,7 +167,7 @@ private void notifyMidpointAboutProcessEvent(ProcessEvent event) {
 		String taskOid = ActivitiUtil.getTaskOid(event.getVariables());
 		Task task;
 		try {
-			task = taskManager.getTask(taskOid, result);
+			task = taskManager.getTaskWithResult(taskOid, result);
 		} catch (ObjectNotFoundException|SchemaException|RuntimeException e) {
 			throw new SystemException("Couldn't get task " + taskOid + " from repository: " + e.getMessage(), e);
 		}

repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java

@@ -21,6 +21,7 @@
 import com.evolveum.midpoint.prism.delta.ItemDelta;
 import com.evolveum.midpoint.prism.delta.ObjectDelta;
 import com.evolveum.midpoint.prism.delta.ReferenceDelta;
+import com.evolveum.midpoint.prism.delta.builder.DeltaBuilder;
 import com.evolveum.midpoint.prism.path.ItemPath;
 import com.evolveum.midpoint.prism.polystring.PolyString;
 import com.evolveum.midpoint.prism.util.PrismTestUtil;
@@ -855,9 +856,11 @@ public void test400AddModifyTask() throws Exception {
         OperationResultType res = new OperationResultType();
         res.setOperation("asdf");
         res.setStatus(OperationResultStatusType.FATAL_ERROR);
-        ObjectDelta delta = ObjectDelta.createModificationReplaceProperty(TaskType.class, oid, TaskType.F_RESULT, prismContext, res);
-        repositoryService.modifyObject(TaskType.class, oid, delta.getModifications(), result);
-
+        List<ItemDelta<?, ?>> itemDeltas = DeltaBuilder.deltaFor(TaskType.class, prismContext)
+                .item(TaskType.F_RESULT).replace(res)
+                .item(TaskType.F_RESULT_STATUS).replace(res.getStatus())
+                .asItemDeltas();
+        repositoryService.modifyObject(TaskType.class, oid, itemDeltas, result);
 
         task = repositoryService.getObject(TaskType.class, oid, null, result);
         taskType = task.asObjectable();

repo/repo-sql-impl-test/src/test/resources/basic/task.xml

@@ -37,4 +37,5 @@
         <status>in_progress</status>
         <token>1000000000000000895</token>
     </result>
+    <resultStatus>in_progress</resultStatus>
 </task>

repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java

@@ -578,15 +578,14 @@ private <T extends ObjectType> PrismObject<T> updateLoadedObject(GetObjectResult
 
                     prismObject.setPropertyRealValue(TaskType.F_RESULT_STATUS, resultType.getStatus());
                 }
-
-
-            } else {
-                Query<ROperationResultStatus> query = session.getNamedQuery("get.taskStatus");
-                query.setParameter("oid", prismObject.getOid());
-
-                ROperationResultStatus status = query.uniqueResult();
-                prismObject.setPropertyRealValue(TaskType.F_RESULT_STATUS, (status != null ? status.getSchemaValue() : null));
             }
+//            else {
+//                Query<ROperationResultStatus> query = session.getNamedQuery("get.taskStatus");
+//                query.setParameter("oid", prismObject.getOid());
+//
+//                ROperationResultStatus status = query.uniqueResult();
+//                prismObject.setPropertyRealValue(TaskType.F_RESULT_STATUS, (status != null ? status.getSchemaValue() : null));
+//            }
         }
 
         if (partialValueHolder != null) {

repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/IntegrationTestTools.java

@@ -406,7 +406,7 @@ public static void waitFor(String message, Checker checker, long timeoutInterval
 	}
 
 	public static void displayJaxb(String title, Object o, QName defaultElementName) throws SchemaException {
-		String serialized = PrismTestUtil.serializeAnyData(o, defaultElementName);
+		String serialized = o != null ? PrismTestUtil.serializeAnyData(o, defaultElementName) : "(null)";
 		System.out.println(OBJECT_TITLE_OUT_PREFIX + title);
 		System.out.println(serialized);
 		LOGGER.debug(OBJECT_TITLE_LOG_PREFIX + title + "\n" + serialized);
@@ -472,9 +472,10 @@ public static void display(OperationResult result) {
 
 	public static void display(String title, OperationResult result) {
 		System.out.println(OBJECT_TITLE_OUT_PREFIX + title);
-		System.out.println(result.debugDump());
+		String debugDump = result != null ? result.debugDump() : "(null)";
+		System.out.println(debugDump);
 		LOGGER.debug(OBJECT_TITLE_LOG_PREFIX + title  + "\n"
-				+ result.debugDump());
+				+ debugDump);
 	}
 
 	public static void display(String title, OperationResultType result) throws SchemaException {

repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerQuartzImpl.java

@@ -1877,7 +1877,7 @@ public void unscheduleTask(Task task, OperationResult parentResult) {
     // use with care (e.g. w.r.t. dependent tasks)
     public void closeTask(Task task, OperationResult parentResult) throws ObjectNotFoundException, SchemaException {
         try {
-			OperationResult taskResult = updateTaskResult(task);
+			OperationResult taskResult = updateTaskResult(task, parentResult);
 			task.close(taskResult, true, parentResult);
         } finally {
             if (task.isPersistent()) {
@@ -1893,8 +1893,8 @@ private boolean shouldPurgeResult(Task task) {
 
 	// do not forget to kick dependent tasks when closing this one (currently only done in finishHandler)
     public void closeTaskWithoutSavingState(Task task, OperationResult parentResult) {
-		OperationResult taskResult = updateTaskResult(task);
 		try {
+			OperationResult taskResult = updateTaskResult(task, parentResult);
 			task.close(taskResult, false, parentResult);
 		} catch (ObjectNotFoundException | SchemaException e) {
 			throw new SystemException(e);       // shouldn't occur
@@ -1904,11 +1904,20 @@ public void closeTaskWithoutSavingState(Task task, OperationResult parentResult)
 
     // returns null if no change is needed in the task
 	@Nullable
-	private OperationResult updateTaskResult(Task task) {
+	private OperationResult updateTaskResult(Task task, OperationResult parentResult) throws SchemaException {
 		OperationResult taskResult = task.getResult();
 		if (taskResult == null) {
-			// should not occur
-			return null;
+			try {
+				task.refresh(parentResult);     // expecting to get the result
+			} catch (ObjectNotFoundException e) {
+				LOGGER.warn("Task result cannot be updated because the task is gone: {}", task, e);
+				return null;
+			}
+			taskResult = task.getResult();
+			if (taskResult == null) {
+				LOGGER.warn("Null task result in {}", task);
+				return null;
+			}
 		}
 		boolean resultChanged = false;
 		// this is a bit of magic to ensure closed tasks will not stay with IN_PROGRESS result (and, if possible, also not with UNKNOWN)

repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java

@@ -136,13 +136,19 @@ public class TaskQuartzImpl implements Task {
      *
      * This one is the live value of this task's result. All operations working with this task
      * should work with this value. This value is explicitly updated from the value in prism
-     * when fetching task from repo (or creating anew), see initializeFromRepo().
+     * when fetching task from repo (or creating anew).
      *
      * The value in taskPrism is updated when necessary, e.g. when getting taskPrism
      * (for example, used when persisting task to repo), etc, see the code.
      *
      * Note that this means that we SHOULD NOT get operation result from the prism - we should
      * use task.getResult() instead!
+     *
+     * This result can be null if the task was created from taskPrism retrieved from repo without fetching the result.
+     * Such tasks should NOT be used to execute handlers, as the result information would be lost.
+     *
+     * Basically, the result should be initialized only when a new transient task is created. It should be then persisted
+     * into the repository. Tasks that are to execute handlers should be fetched from the repository with their results.
      */
 	private OperationResult taskResult;
 
@@ -201,37 +207,28 @@ private TaskQuartzImpl(TaskManagerQuartzImpl taskManager) {
 		setBindingTransient(DEFAULT_BINDING_TYPE);
 		setProgressTransient(0);
 		setObjectTransient(null);
-		createOrUpdateTaskResult(operationName);
+		createOrUpdateTaskResult(operationName, true);
 
 		setDefaults();
 	}
 
 	/**
 	 * Assumes that the task is persistent
 	 *
+	 * NOTE: if the result in prism is null, task result will be kept null as well (meaning it was not fetched from the repository).
+	 *
 	 * @param operationName if null, default op. name will be used
 	 */
 	TaskQuartzImpl(TaskManagerQuartzImpl taskManager, PrismObject<TaskType> taskPrism, RepositoryService repositoryService,
 			String operationName) {
 		this(taskManager);
 		this.repositoryService = repositoryService;
 		this.taskPrism = taskPrism;
-		createOrUpdateTaskResult(operationName);
+		createOrUpdateTaskResult(operationName, false);
 
 		setDefaults();
 	}
 
-	/**
-	 * Analogous to the previous constructor.
-	 *
-	 * @param taskPrism
-	 */
-	private void replaceTaskPrism(PrismObject<TaskType> taskPrism) {
-		this.taskPrism = taskPrism;
-		createOrUpdateTaskResult(null);
-		setDefaults();
-	}
-
 	private PrismObject<TaskType> createPrism() {
 		try {
 			return getPrismContext().createObject(TaskType.class);
@@ -246,20 +243,22 @@ private void setDefaults() {
 		}
 	}
 
-	private void createOrUpdateTaskResult(String operationName) {
+	private void createOrUpdateTaskResult(String operationName, boolean create) {
 		OperationResultType resultInPrism = taskPrism.asObjectable().getResult();
-		if (resultInPrism == null) {
+		if (resultInPrism == null && create) {
 			if (operationName == null) {
-				resultInPrism = new OperationResult(DOT_INTERFACE + "run").createOperationResultType();
+				resultInPrism = createUnnamedTaskResult().createOperationResultType();
 			} else {
 				resultInPrism = new OperationResult(operationName).createOperationResultType();
 			}
 			taskPrism.asObjectable().setResult(resultInPrism);
 		}
-		try {
-			taskResult = OperationResult.createOperationResult(resultInPrism);
-		} catch (SchemaException e) {
-			throw new SystemException(e.getMessage(), e);
+		if (resultInPrism != null) {
+			try {
+				taskResult = OperationResult.createOperationResult(resultInPrism);
+			} catch (SchemaException e) {
+				throw new SystemException(e.getMessage(), e);
+			}
 		}
 	}
 	//endregion
@@ -2497,7 +2496,9 @@ private void updateTaskInstance(PrismObject<TaskType> taskPrism, OperationResult
 		result.addArbitraryObjectAsParam("task", this);
 		result.addParam("taskPrism", taskPrism);
 
-		replaceTaskPrism(taskPrism);
+		this.taskPrism = taskPrism;
+		createOrUpdateTaskResult(null, false);
+		setDefaults();
 		resolveOwnerRef(result);
 		result.recordSuccessIfUnknown();
 	}
@@ -3236,4 +3237,9 @@ public String getExecutionGroup() {
 		TaskExecutionConstraintsType executionConstraints = getExecutionConstraints();
 		return executionConstraints != null ? executionConstraints.getGroup() : null;
 	}
+
+	@NotNull
+	public OperationResult createUnnamedTaskResult() {
+		return new OperationResult(DOT_INTERFACE + "run");
+	}
 }

repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/execution/JobExecutor.java

@@ -103,7 +103,7 @@ public void execute(JobExecutionContext context) throws JobExecutionException {
             taskManagerImpl.getExecutionManager().removeTaskFromQuartz(oid, executionResult);
             return;
         } catch (SchemaException e) {
-            LoggingUtils.logUnexpectedException(LOGGER, "Task with OID {} cannot be retrieved because of schema exception. Please correct the problem or resynchronize midPoint repository with Quartz job store using 'xxxxxxx' function. Now exiting the execution routine.", e, oid);
+            LoggingUtils.logUnexpectedException(LOGGER, "Task with OID {} cannot be retrieved because of schema exception. Please correct the problem or resynchronize midPoint repository with Quartz job store. Now exiting the execution routine.", e, oid);
             return;
         } catch (RuntimeException e) {
 			LoggingUtils.logUnexpectedException(LOGGER, "Task with OID {} could not be retrieved, exiting the execution routine.", e, oid);
@@ -648,6 +648,10 @@ private void executeRecurrentTask(TaskHandler handler) {
     private TaskRunResult executeHandler(TaskHandler handler, OperationResult executionResult) {
 
 		task.startCollectingOperationStats(handler.getStatisticsCollectionStrategy());
+	    if (task.getResult() == null) {
+		    LOGGER.warn("Task without operation result found, please check the task creation/retrieval/update code: {}", task);
+		    task.setResultTransient(task.createUnnamedTaskResult());
+	    }
 
 	    TaskRunResult runResult;
 		if (handler instanceof WorkBucketAwareTaskHandler) {

repo/task-quartz-impl/src/test/java/com/evolveum/midpoint/task/quartzimpl/AbstractTaskManagerTest.java

@@ -213,7 +213,8 @@ protected void waitForTaskCloseCheckingSubtasks(String taskOid, OperationResult
 	        }
 		    List<Task> subtasks = task.listSubtasksDeeply(result);
 		    for (Task subtask : subtasks) {
-			    if (subtask.getResult().isError()) {
+			    if (subtask.getResultStatus() == OperationResultStatusType.FATAL_ERROR
+					    || subtask.getResultStatus() == OperationResultStatusType.PARTIAL_ERROR) {
 			    	display("Error detected in subtask, finishing waiting: " + subtask);
 			    	return true;
 			    }

testing/sanity/src/test/java/com/evolveum/midpoint/testing/sanity/TestSanity.java

@@ -3554,10 +3554,12 @@ public void timeout() {
         // [pm] commented out, as progress in recon task is now determined not only using # of changes
         //AssertJUnit.assertEquals(0, task.getProgress());
 
-        // Test for presence of a result. It should be there and it should
-        // indicate success
+        // Test for presence of a result. It was not fetched - so it should NOT be there
         OperationResult taskResult = task.getResult();
-        AssertJUnit.assertNotNull(taskResult);
+        AssertJUnit.assertNull(taskResult);
+
+        // However, the task should indicate success
+        AssertJUnit.assertEquals(OperationResultStatusType.SUCCESS, task.getResultStatus());
 
         // STOP the task. We don't need it any more and we don't want to give it a chance to run more than once
         taskManager.deleteTask(TASK_OPENDJ_RECON_OID, result);