diff --git a/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java b/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java index 741063ea616..bb21f73f031 100644 --- a/model/model-test/src/main/java/com/evolveum/midpoint/model/test/AbstractModelIntegrationTest.java +++ b/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; } diff --git a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/activiti/ActivitiInterface.java b/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/activiti/ActivitiInterface.java index c606ff872f7..6a6aea9630f 100644 --- a/model/workflow-impl/src/main/java/com/evolveum/midpoint/wf/impl/activiti/ActivitiInterface.java +++ b/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); } diff --git a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java b/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java index 156f6cbef88..d0a1980c8a0 100644 --- a/repo/repo-sql-impl-test/src/test/java/com/evolveum/midpoint/repo/sql/AddGetObjectTest.java +++ b/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> 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(); diff --git a/repo/repo-sql-impl-test/src/test/resources/basic/task.xml b/repo/repo-sql-impl-test/src/test/resources/basic/task.xml index afecee13506..1f0486d3fe1 100644 --- a/repo/repo-sql-impl-test/src/test/resources/basic/task.xml +++ b/repo/repo-sql-impl-test/src/test/resources/basic/task.xml @@ -37,4 +37,5 @@ in_progress 1000000000000000895 + in_progress \ No newline at end of file diff --git a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java index 06864b5bb00..0162c537240 100644 --- a/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java +++ b/repo/repo-sql-impl/src/main/java/com/evolveum/midpoint/repo/sql/helpers/ObjectRetriever.java @@ -578,15 +578,14 @@ private PrismObject updateLoadedObject(GetObjectResult prismObject.setPropertyRealValue(TaskType.F_RESULT_STATUS, resultType.getStatus()); } - - - } else { - Query 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 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) { diff --git a/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/IntegrationTestTools.java b/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/IntegrationTestTools.java index a25ac62d7fb..ba26140a8e5 100644 --- a/repo/repo-test-util/src/main/java/com/evolveum/midpoint/test/IntegrationTestTools.java +++ b/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 { diff --git a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerQuartzImpl.java b/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerQuartzImpl.java index 79eda7e75c7..8a0cf507509 100644 --- a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskManagerQuartzImpl.java +++ b/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) diff --git a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java b/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java index 327abbe6fed..6c48d29d8ae 100644 --- a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/TaskQuartzImpl.java +++ b/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,7 +207,7 @@ private TaskQuartzImpl(TaskManagerQuartzImpl taskManager) { setBindingTransient(DEFAULT_BINDING_TYPE); setProgressTransient(0); setObjectTransient(null); - createOrUpdateTaskResult(operationName); + createOrUpdateTaskResult(operationName, true); setDefaults(); } @@ -209,6 +215,8 @@ private TaskQuartzImpl(TaskManagerQuartzImpl taskManager) { /** * 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 taskPrism, RepositoryService repositoryService, @@ -216,22 +224,11 @@ private TaskQuartzImpl(TaskManagerQuartzImpl taskManager) { 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 taskPrism) { - this.taskPrism = taskPrism; - createOrUpdateTaskResult(null); - setDefaults(); - } - private PrismObject 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 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"); + } } diff --git a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/execution/JobExecutor.java b/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/execution/JobExecutor.java index 60de986dfd6..42957848eff 100644 --- a/repo/task-quartz-impl/src/main/java/com/evolveum/midpoint/task/quartzimpl/execution/JobExecutor.java +++ b/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) { diff --git a/repo/task-quartz-impl/src/test/java/com/evolveum/midpoint/task/quartzimpl/AbstractTaskManagerTest.java b/repo/task-quartz-impl/src/test/java/com/evolveum/midpoint/task/quartzimpl/AbstractTaskManagerTest.java index 33560fdbf99..a03fbc5af7f 100644 --- a/repo/task-quartz-impl/src/test/java/com/evolveum/midpoint/task/quartzimpl/AbstractTaskManagerTest.java +++ b/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 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; } diff --git a/testing/sanity/src/test/java/com/evolveum/midpoint/testing/sanity/TestSanity.java b/testing/sanity/src/test/java/com/evolveum/midpoint/testing/sanity/TestSanity.java index 18e96151396..3b250495558 100644 --- a/testing/sanity/src/test/java/com/evolveum/midpoint/testing/sanity/TestSanity.java +++ b/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);