Skip to content

Commit

Permalink
DMP-4216: Allow inactive tasks to be run manually (#2238)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ben-Edwards-cgi authored Nov 12, 2024
1 parent a7c3a26 commit 5c0f772
Show file tree
Hide file tree
Showing 10 changed files with 49 additions and 69 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void findsAutomatedTasksById() {
void findsAllAutomatedTasks() {
var persistedTasks = dartsDatabase.getAllAutomatedTasks();

var automatedTasks = adminAutomatedTaskService.getAllAutomatedTasks();
var automatedTasks = adminAutomatedTaskService.getAllAutomatedTasksSummaries();

assertThat(automatedTasks).extracting("id").isEqualTo(taskIdsOf(persistedTasks));
assertThat(automatedTasks).extracting("name").isEqualTo(taskNamesOf(persistedTasks));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class AutomatedTasksController implements TasksApi {
@Authorisation(contextId = ANY_ENTITY_ID, globalAccessSecurityRoles = {SUPER_ADMIN})
@Override
public ResponseEntity<List<AutomatedTaskSummary>> getAutomatedTasks() {
return new ResponseEntity<>(adminAutomatedTaskService.getAllAutomatedTasks(), HttpStatus.OK);
return new ResponseEntity<>(adminAutomatedTaskService.getAllAutomatedTasksSummaries(), HttpStatus.OK);
}

@SecurityRequirement(name = SECURITY_SCHEMES_BEARER_AUTH)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,12 @@ public interface AutomatedTask extends Runnable {

String getTaskName();

void run(boolean isManualRun);

default void run() {
run(false);
}

AutomatedTaskStatus getAutomatedTaskStatus();

String getLastCronExpression();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ public abstract class AbstractLockableAutomatedTask implements AutomatedTask, Au

private ThreadLocal<UUID> executionId;

private boolean isManualTask = false;

public void setManualTask() {
this.isManualTask = true;
}

protected AbstractLockableAutomatedTask(AutomatedTaskRepository automatedTaskRepository,
AutomatedTaskConfigurationProperties automatedTaskConfigurationProperties,
LogApi logApi, LockService lockService) {
Expand All @@ -69,17 +63,15 @@ protected AbstractLockableAutomatedTask(AutomatedTaskRepository automatedTaskRep
}

private void setupUserAuthentication() {

Jwt jwt = Jwt.withTokenValue("automated-task")
.header("alg", "RS256")
.claim("emails", List.of(automatedTaskConfigurationProperties.getSystemUserEmail()))
.build();
SecurityContextHolder.getContext().setAuthentication(new JwtAuthenticationToken(jwt));

}

@Override
public void run() {
public void run(boolean isManualRun) {
executionId = ThreadLocal.withInitial(UUID::randomUUID);
preRunTask();
try {
Expand All @@ -89,8 +81,11 @@ public void run() {
AutomatedTaskEntity automatedTask = automatedTaskEntity.get();
String dbCronExpression = automatedTask.getCronExpression();
// Check the cron expression hasn't been changed in the database by another instance, if so skip this run
if (isManualTask || getLastCronExpression().equals(dbCronExpression)) {
if (TRUE.equals(automatedTask.getTaskEnabled())) {
if (isManualRun || getLastCronExpression().equals(dbCronExpression)) {
if (isManualRun || TRUE.equals(automatedTask.getTaskEnabled())) {
if (!TRUE.equals(automatedTask.getTaskEnabled())) {
log.info("Task: {} is inactive but has been run manually", getTaskName());
}
logApi.taskStarted(executionId.get(), this.getTaskName());
lockService.getLockingTaskExecutor().executeWithLock(new LockedTask(), getLockConfiguration());
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

public interface AdminAutomatedTaskService {

List<AutomatedTaskSummary> getAllAutomatedTasks();
List<AutomatedTaskSummary> getAllAutomatedTasksSummaries();

DetailedAutomatedTask getAutomatedTaskById(Integer taskId);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import uk.gov.hmcts.darts.common.repository.AutomatedTaskRepository;
import uk.gov.hmcts.darts.task.api.AutomatedTaskName;
import uk.gov.hmcts.darts.task.exception.AutomatedTaskApiError;
import uk.gov.hmcts.darts.task.runner.AutomatedTask;
import uk.gov.hmcts.darts.task.service.AdminAutomatedTaskService;
import uk.gov.hmcts.darts.task.service.LockService;
import uk.gov.hmcts.darts.tasks.model.AutomatedTaskPatch;
Expand All @@ -34,22 +35,29 @@ public class AdminAutomatedTasksServiceImpl implements AdminAutomatedTaskService

private final AutomatedTaskRepository automatedTaskRepository;
private final AutomatedTasksMapper mapper;
private final ManualTaskService manualTaskService;
private final AutomatedTaskRunner automatedTaskRunner;
private final AuditApi auditApi;
private final LockService lockService;

private final ConfigurableBeanFactory configurableBeanFactory;
private final List<AutomatedTask> automatedTasks;

@Override
public List<AutomatedTaskSummary> getAllAutomatedTasks() {
var automatedTask = automatedTaskRepository.findAll()
public List<AutomatedTaskSummary> getAllAutomatedTasksSummaries() {
return mapper.mapEntitiesToModel(getAllAutomatedTasksEntities());
}

List<AutomatedTaskEntity> getAllAutomatedTasksEntities() {
return automatedTaskRepository.findAll()
.stream()
.filter(this::shouldIncludeAutomatedTask)
.toList();
return mapper.mapEntitiesToModel(automatedTask);
}

List<AutomatedTask> getAllAutomatedTasks() {
return automatedTasks;
}


@Override
public DetailedAutomatedTask getAutomatedTaskById(Integer taskId) {
return mapper.mapEntityToDetailedAutomatedTask(getAutomatedTaskEntityById(taskId));
Expand All @@ -65,7 +73,8 @@ public void runAutomatedTask(Integer taskId) {
throw new DartsApiException(AUTOMATED_TASK_ALREADY_RUNNING);
}

var automatedTask = manualTaskService.getAutomatedTasks().stream()
var automatedTask = getAllAutomatedTasks()
.stream()
.filter(task -> task.getTaskName().equals(automatedTaskEntity.getTaskName()))
.findFirst();

Expand All @@ -74,7 +83,7 @@ public void runAutomatedTask(Integer taskId) {
throw new DartsApiException(AutomatedTaskApiError.AUTOMATED_TASK_NOT_CONFIGURED_CORRECTLY);
}

automatedTaskRunner.run(automatedTask.get());
automatedTaskRunner.run(automatedTask.get(), true);

auditApi.record(RUN_JOB_MANUALLY, automatedTaskEntity.getTaskName());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,14 @@
import lombok.extern.slf4j.Slf4j;
import org.springframework.scheduling.annotation.Async;
import org.springframework.stereotype.Component;
import uk.gov.hmcts.darts.task.runner.impl.AbstractLockableAutomatedTask;
import uk.gov.hmcts.darts.task.runner.AutomatedTask;

@Component
@Slf4j
public class AutomatedTaskRunner {

@Async
public void run(AbstractLockableAutomatedTask task) {
log.info("Attempting manual run of {}", task.getTaskName());
task.run();
public void run(AutomatedTask task, boolean isManualRun) {
log.info("Attempting {} run of {}", isManualRun ? " manual " : " automated", task.getTaskName());
task.run(isManualRun);
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import uk.gov.hmcts.darts.common.exception.DartsApiException;
import uk.gov.hmcts.darts.common.repository.AutomatedTaskRepository;
import uk.gov.hmcts.darts.task.exception.AutomatedTaskApiError;
import uk.gov.hmcts.darts.task.runner.AutomatedTask;
import uk.gov.hmcts.darts.task.runner.impl.AbstractLockableAutomatedTask;
import uk.gov.hmcts.darts.task.service.LockService;
import uk.gov.hmcts.darts.tasks.model.AutomatedTaskPatch;
Expand All @@ -27,8 +28,10 @@
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.lenient;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import static org.mockito.Mockito.verifyNoInteractions;
Expand All @@ -43,8 +46,6 @@ class AdminAutomatedTasksServiceImplTest {
@Mock
private AutomatedTasksMapper mapper;
@Mock
private ManualTaskService manualTaskService;
@Mock
private AutomatedTaskRunner automatedTaskRunner;
@Mock
private LockService lockService;
Expand All @@ -62,17 +63,19 @@ class AdminAutomatedTasksServiceImplTest {

@Test
void invokesTaskWhenTaskIsNotLocked() {
adminAutomatedTaskService = spy(adminAutomatedTaskService);
var automatedTask = anAutomatedTaskEntityWithName("some-task-name", null);
when(someAutomatedTask.getTaskName()).thenReturn("some-task-name");
when(lockService.isLocked(automatedTask)).thenReturn(false);

when(automatedTaskRepository.findById(1)).thenReturn(Optional.of(automatedTask));
when(manualTaskService.getAutomatedTasks()).thenReturn(List.of(someAutomatedTask));
doReturn(List.of((AutomatedTask) someAutomatedTask)).when(adminAutomatedTaskService).getAllAutomatedTasks();

adminAutomatedTaskService.runAutomatedTask(1);

verify(automatedTaskRunner, times(1)).run(someAutomatedTask);
verify(auditApi, times(1)).record(AuditActivity.RUN_JOB_MANUALLY,"some-task-name");
verify(automatedTaskRunner, times(1)).run(someAutomatedTask, true);
verify(auditApi, times(1)).record(AuditActivity.RUN_JOB_MANUALLY, "some-task-name");
verify(adminAutomatedTaskService, times(1)).getAllAutomatedTasks();
}

@Test
Expand All @@ -95,7 +98,7 @@ void updateAutomatedTask() {
assertFalse(automatedTaskEntity.getTaskEnabled());
assertEquals(100, automatedTaskEntity.getBatchSize());
assertEquals(expectedReturnTask, task);
verify(auditApi).record(AuditActivity.ENABLE_DISABLE_JOB,"some-task-name disabled");
verify(auditApi).record(AuditActivity.ENABLE_DISABLE_JOB, "some-task-name disabled");
verifyNoMoreInteractions(auditApi);
}

Expand All @@ -116,7 +119,7 @@ void updateAutomatedTaskDoesNotUpdateFieldsWithNullValueInPatchRequest() {


@Test
void positiveGetAllAutomatedTasks() {
void positiveGetAllAutomatedTasksSummaries() {
AutomatedTaskEntity automatedTaskEntity1 =
createAutomatedTaskEntity("task1", true);
AutomatedTaskEntity automatedTaskEntity2 =
Expand All @@ -126,7 +129,7 @@ void positiveGetAllAutomatedTasks() {

when(automatedTaskRepository.findAll()).thenReturn(List.of(automatedTaskEntity1, automatedTaskEntity2, automatedTaskEntity3));

adminAutomatedTaskService.getAllAutomatedTasks();
adminAutomatedTaskService.getAllAutomatedTasksSummaries();

verify(automatedTaskRepository, times(1))
.findAll();
Expand All @@ -136,7 +139,7 @@ void positiveGetAllAutomatedTasks() {


@Test
void positiveGetAllAutomatedTasksDisabledExcludeFlag() {
void positiveGetAllAutomatedTasksSummariesDisabledExcludeFlag() {
AutomatedTaskEntity automatedTaskEntity1 =
createAutomatedTaskEntity("task1", true);
AutomatedTaskEntity automatedTaskEntity2 =
Expand All @@ -146,7 +149,7 @@ void positiveGetAllAutomatedTasksDisabledExcludeFlag() {

when(automatedTaskRepository.findAll()).thenReturn(List.of(automatedTaskEntity1, automatedTaskEntity2, automatedTaskEntity3));

adminAutomatedTaskService.getAllAutomatedTasks();
adminAutomatedTaskService.getAllAutomatedTasksSummaries();

verify(automatedTaskRepository, times(1))
.findAll();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,7 @@ public void setLastCronExpression(String cronExpression) {
}

@Override
public void run() {
public void run(boolean isManualRun) {
log.debug("Running test automated task");
}

Expand Down

0 comments on commit 5c0f772

Please sign in to comment.