Skip to content

Commit

Permalink
Generate task identifier if missing
Browse files Browse the repository at this point in the history
The task identifier was required for years; tasks without it
were not started. Now we simply generate it if it's not there.

This resolves MID-9423.
  • Loading branch information
mederly committed Jan 18, 2024
1 parent 01c0af2 commit 6f38ce3
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@

package com.evolveum.midpoint.task.quartzimpl;

import com.evolveum.midpoint.task.api.LightweightIdentifierGenerator;

import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.stereotype.Component;
Expand Down Expand Up @@ -66,6 +68,7 @@ public class TaskBeans {
@Autowired public NodeRegistrar nodeRegistrar;
@Autowired public UpAndDown upAndDown;
@Autowired public LightweightTaskManager lightweightTaskManager;
@Autowired public LightweightIdentifierGenerator lightweightIdentifierGenerator;
@Autowired public TaskSynchronizer taskSynchronizer;
@Autowired public ClusterStatusInformationRetriever clusterStatusInformationRetriever;
//endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static com.evolveum.midpoint.task.quartzimpl.run.GroupLimitsChecker.*;
import static com.evolveum.midpoint.task.quartzimpl.run.StopJobException.Severity.*;
import static com.evolveum.midpoint.util.MiscUtil.stateCheck;
import static com.evolveum.midpoint.util.MiscUtil.stateNonNull;

/**
* Executes a Quartz job i.e. midPoint task.
Expand Down Expand Up @@ -92,7 +93,6 @@ private void executeInternal(OperationResult result)

fetchTheTask(oid, result);

checkTaskSanity(result);
checkTaskReady();
fixTaskExecutionInformation(result);
checkLocalSchedulerRunning(result);
Expand Down Expand Up @@ -152,15 +152,6 @@ private void executeInternal(OperationResult result)
}
}

private void checkTaskSanity(OperationResult result) throws StopJobException {
if (task.getTaskIdentifier() == null) {
result.recordFatalError("Task without identifier cannot be executed");
suspendFlawedTaskRecordingResult(result);
throw new StopJobException(ERROR, "Task without identifier cannot be executed: %s", null, task);
}
// Consider checking e.g. ownerRef.oid here
}

private void executeHandler(TaskHandler handler, OperationResult result) throws StopJobException {
new TaskCycleExecutor(task, handler, this, beans)
.execute(result);
Expand Down Expand Up @@ -301,6 +292,11 @@ private void checkTaskReady() throws StopJobException {
private void fetchTheTask(String oid, OperationResult result) throws StopJobException {
try {
TaskQuartzImpl taskWithResult = beans.taskRetriever.getTaskWithResult(oid, result);

if (taskWithResult.getTaskIdentifier() == null) {
taskWithResult = generateTaskIdentifier(taskWithResult, result);
}

ParentAndRoot parentAndRoot = taskWithResult.getParentAndRoot(result);
task = beans.taskInstantiator.toRunningTaskInstance(taskWithResult, parentAndRoot.root, parentAndRoot.parent);
} catch (ObjectNotFoundException e) {
Expand All @@ -314,6 +310,27 @@ private void fetchTheTask(String oid, OperationResult result) throws StopJobExce
}
}

private static TaskQuartzImpl generateTaskIdentifier(TaskQuartzImpl task, OperationResult result)
throws ObjectNotFoundException, SchemaException, ObjectAlreadyExistsException {
String taskOid = task.getOid();
String newIdentifier = beans.lightweightIdentifierGenerator.generate().toString();

beans.repositoryService.modifyObject(
TaskType.class,
taskOid,
beans.prismContext.deltaFor(TaskType.class)
.item(TaskType.F_TASK_IDENTIFIER)
.replace(newIdentifier)
.asItemDeltas(),
result);

LOGGER.info("Generated identifier {} for task {}", newIdentifier, task);

var reloadedTask = beans.taskRetriever.getTaskWithResult(taskOid, result);
stateNonNull(reloadedTask.getTaskIdentifier(), "Still no identifier in %s", reloadedTask);
return reloadedTask;
}

/**
* This is a sanity check against concurrent executions of a task in a cluster.
* Quartz should take care of it but it looks like it sometimes fails to do that
Expand Down Expand Up @@ -377,20 +394,15 @@ private TaskExecutionStateType getNewExecutionState() {
LOGGER.error("No scheduling state in {}. Setting execution state to SUSPENDED.", task);
return TaskExecutionStateType.SUSPENDED;
}
switch (task.getSchedulingState()) {
case SUSPENDED:
return TaskExecutionStateType.SUSPENDED;
case CLOSED:
return TaskExecutionStateType.CLOSED;
case WAITING:
// The current execution state should be OK. It is because the switch to WAITING was done internally
// by the task handler, and was accompanied by the change in the execution state.
return null;
case READY: // Not much probable, but can occur in theory.
return TaskExecutionStateType.RUNNABLE;
default:
throw new AssertionError(task.getSchedulingState());
}
return switch (task.getSchedulingState()) {
case SUSPENDED -> TaskExecutionStateType.SUSPENDED;
case CLOSED -> TaskExecutionStateType.CLOSED;
case READY -> TaskExecutionStateType.RUNNABLE; // Not much probable, but can occur in theory.

// The current execution state should be OK. It is because the switch to WAITING was done internally
// by the task handler, and was accompanied by the change in the execution state.
case WAITING -> null;
};
}

private void checkGroupLimits(OperationResult result) throws StopJobException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,14 +195,11 @@ private String serialize(TaskExecutionLimitationsType parsed) throws SchemaExcep
return getPrismContext().xmlSerializer().serializeRealValue(parsed, new QName(SchemaConstants.NS_C, "value"));
}

/** MID-9058. */
/** MID-9423. */
@Test
public void test220TaskWithoutIdentifier() throws Exception {
var result = createOperationResult();

given("scheduler is down");
taskManager.stopLocalScheduler(result);

when("task without identifier is added");
TaskType task = new TaskType()
.name(getTestNameShort())
Expand All @@ -211,19 +208,15 @@ public void test220TaskWithoutIdentifier() throws Exception {
.handlerUri(MOCK_TASK_HANDLER_URI);
repositoryService.addObject(task.asPrismObject(), null, result);

and("scheduler is started");
taskManager.startLocalScheduler(result);

and("tasks are synchronized");
taskManager.synchronizeTasks(result);

then("task is started but suspended");
then("task is started and correctly finishes");
waitForTaskCloseOrSuspend(task.getOid(), 10000);

assertTask(task.getOid(), "after")
.display()
.assertSuspended()
.assertFatalError()
.assertResultMessageContains("Task without identifier cannot be executed");
.assertClosed()
.assertSuccess();
}
}

0 comments on commit 6f38ce3

Please sign in to comment.