From 78167c4edb9b1e1aa8b6b33cb10379294edbc143 Mon Sep 17 00:00:00 2001 From: Matt Date: Mon, 4 Nov 2024 17:04:17 +0000 Subject: [PATCH] Added Integration Tests, Unique key on UUID column --- .../box/l10n/mojito/entity/ScheduledJob.java | 4 + .../scheduledjob/ScheduledJobManager.java | 40 ++- .../db/migration/V71__Add_scheduled_jobs.sql | 3 + .../scheduledjob/NoOpScheduledJobTest.java | 27 +- .../scheduledjob/ScheduledJobManagerTest.java | 231 +++++++++++++----- 5 files changed, 232 insertions(+), 73 deletions(-) diff --git a/webapp/src/main/java/com/box/l10n/mojito/entity/ScheduledJob.java b/webapp/src/main/java/com/box/l10n/mojito/entity/ScheduledJob.java index b67c819bc9..e06765bcdd 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/entity/ScheduledJob.java +++ b/webapp/src/main/java/com/box/l10n/mojito/entity/ScheduledJob.java @@ -2,6 +2,7 @@ import com.box.l10n.mojito.json.ObjectMapper; import com.box.l10n.mojito.service.scheduledjob.ScheduledJobProperties; +import jakarta.persistence.Basic; import jakarta.persistence.Column; import jakarta.persistence.Entity; import jakarta.persistence.ForeignKey; @@ -24,6 +25,7 @@ public class ScheduledJob { @GeneratedValue(strategy = GenerationType.IDENTITY) private Long id; + @Basic(optional = false) @Column(name = "uuid") private String uuid; @@ -42,6 +44,7 @@ public class ScheduledJob { @Transient private ScheduledJobProperties properties; + @Basic(optional = false) @Column(name = "properties") private String propertiesString; @@ -55,6 +58,7 @@ public class ScheduledJob { @Column(name = "end_date") private ZonedDateTime endDate; + @Basic(optional = false) @Column(name = "enabled") private Boolean enabled = true; diff --git a/webapp/src/main/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManager.java b/webapp/src/main/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManager.java index b203278103..c85901e4b7 100644 --- a/webapp/src/main/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManager.java +++ b/webapp/src/main/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManager.java @@ -15,6 +15,7 @@ import java.util.HashSet; import java.util.List; import java.util.Optional; +import java.util.stream.Collectors; import org.quartz.CronScheduleBuilder; import org.quartz.JobBuilder; import org.quartz.JobDetail; @@ -124,7 +125,11 @@ public void scheduleAllJobs() throws ClassNotFoundException, SchedulerException scheduledJobStatusRepository.findByEnum(ScheduledJobStatus.SCHEDULED)); scheduledJob.setStartDate(null); scheduledJob.setEndDate(null); - scheduledJobRepository.save(scheduledJob); + deadlockRetryTemplate.execute( + c -> { + scheduledJobRepository.save(scheduledJob); + return null; + }); } } @@ -178,21 +183,19 @@ public void pushJobsToDB() { public void cleanQuartzJobs() throws SchedulerException { // Clean Quartz jobs that don't exist in the UUID pool logger.info("Performing Quartz scheduled jobs clean up"); - List jobTypes = Arrays.stream(ScheduledJobType.values()).map(Enum::toString).toList(); - for (JobKey jobKey : getScheduler().getJobKeys(GroupMatcher.anyGroup())) { - - // The job type may not be a Scheduled job (this is only the case if the configuration is set - // to use the default scheduler) so continue without deleting. - if (!jobTypes.contains(jobKey.getGroup())) continue; - + for (JobKey jobKey : getAllJobKeys()) { if (!uuidPool.contains(jobKey.getName())) { if (getScheduler().checkExists(jobKey)) { getScheduler().deleteJob(jobKey); } - scheduledJobRepository - .findByUuid(jobKey.getName()) - .ifPresent(scheduledJobRepository::delete); + deadlockRetryTemplate.execute( + c -> { + scheduledJobRepository + .findByUuid(jobKey.getName()) + .ifPresent(scheduledJobRepository::delete); + return null; + }); logger.info( "Removed job with id: '{}' as it is no longer in the data source.", jobKey.getName()); @@ -245,6 +248,21 @@ public Scheduler getScheduler() { return schedulerManager.getScheduler(schedulerName); } + /** + * Retrieve all jobs where the group is in the ScheduledJobType enums. Cannot assume all jobs + * under the scheduler is a scheduled job as the scheduler could be the default scheduler. + */ + public List getAllJobKeys() throws SchedulerException { + List jobTypes = Arrays.stream(ScheduledJobType.values()).map(Enum::toString).toList(); + return getScheduler().getJobKeys(GroupMatcher.anyGroup()).stream() + .filter(jobKey -> jobTypes.contains(jobKey.getGroup())) + .collect(Collectors.toList()); + } + + public List getAllJobNames() throws SchedulerException { + return getAllJobKeys().stream().map(JobKey::getName).collect(Collectors.toList()); + } + public Class loadJobClass(String className) throws ClassNotFoundException { Class clazz = Class.forName(className); diff --git a/webapp/src/main/resources/db/migration/V71__Add_scheduled_jobs.sql b/webapp/src/main/resources/db/migration/V71__Add_scheduled_jobs.sql index 206f428593..f3e8a4cb04 100644 --- a/webapp/src/main/resources/db/migration/V71__Add_scheduled_jobs.sql +++ b/webapp/src/main/resources/db/migration/V71__Add_scheduled_jobs.sql @@ -21,6 +21,9 @@ CREATE TABLE scheduled_job_status_type ( name VARCHAR(255) NOT NULL ); +ALTER TABLE scheduled_job + ADD UNIQUE KEY UK__JOB_UUID(uuid); + ALTER TABLE scheduled_job ADD CONSTRAINT FK__SCHEDULED_JOB__IMPORT_REPOSITORY__ID FOREIGN KEY (repository_id) REFERENCES repository (id); diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/NoOpScheduledJobTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/NoOpScheduledJobTest.java index e0f8425015..df2a51cb55 100644 --- a/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/NoOpScheduledJobTest.java +++ b/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/NoOpScheduledJobTest.java @@ -2,14 +2,35 @@ import org.quartz.JobExecutionContext; import org.quartz.JobExecutionException; +import org.springframework.stereotype.Component; +@Component public class NoOpScheduledJobTest implements IScheduledJob { + public static boolean throwException = false; + public static boolean successEvent = false; + public static boolean failureEvent = false; + @Override - public void onSuccess(JobExecutionContext context) {} + public void onSuccess(JobExecutionContext context) { + successEvent = true; + } @Override - public void onFailure(JobExecutionContext context, JobExecutionException jobException) {} + public void onFailure(JobExecutionContext context, JobExecutionException jobException) { + failureEvent = true; + } @Override - public void execute(JobExecutionContext jobExecutionContext) throws JobExecutionException {} + public void execute(JobExecutionContext jobExecutionContext) throws JobExecutionException { + + if (throwException) { + throw new JobExecutionException(); + } + + try { + Thread.sleep(1000); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } } diff --git a/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManagerTest.java b/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManagerTest.java index 928141ca74..394ae32212 100644 --- a/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManagerTest.java +++ b/webapp/src/test/java/com/box/l10n/mojito/service/scheduledjob/ScheduledJobManagerTest.java @@ -1,7 +1,11 @@ package com.box.l10n.mojito.service.scheduledjob; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.mockito.ArgumentMatchers.any; import com.box.l10n.mojito.entity.ScheduledJob; import com.box.l10n.mojito.quartz.QuartzSchedulerManager; @@ -14,14 +18,26 @@ import com.box.l10n.mojito.service.thirdparty.ThirdPartySyncJobConfig; import com.box.l10n.mojito.service.thirdparty.ThirdPartySyncJobsConfig; import com.google.common.collect.ImmutableMap; +import java.time.ZonedDateTime; +import java.util.HashSet; import java.util.List; +import java.util.function.Supplier; import org.junit.Before; +import org.junit.FixMethodOrder; import org.junit.Test; +import org.junit.runners.MethodSorters; import org.mockito.Mockito; import org.quartz.SchedulerException; -import org.quartz.impl.matchers.GroupMatcher; import org.springframework.beans.factory.annotation.Autowired; +/** + * Tests for the ScheduledJobManager. SpyBean is not used in these tests to ensure a new context is + * not created for the test. If a new context is created another test will begin to hang as it tries + * to use this context for its own tests. As an alternative to spy, static variables are set on the + * No-op scheduled job to determine if the methods success and failure methods are executed by the + * listener. + */ +@FixMethodOrder(MethodSorters.NAME_ASCENDING) public class ScheduledJobManagerTest extends ServiceTestBase { private static boolean initializedDB = false; @@ -34,6 +50,12 @@ public class ScheduledJobManagerTest extends ServiceTestBase { @Autowired RepositoryRepository repositoryRepository; @Autowired DeadLockLoserExceptionRetryTemplate deadlockRetryTemplate; + private static final int MAX_RETRIES = 10; + private static final int RETRY_DELAY_MS = 100; + private static final int JOB_WAIT_TIME_MS = 5000; + + private final String TEST_UUID = "e4c72563-d8f0-4465-9eec-aaa96087665e"; + @Before public void initialize() throws RepositoryNameAlreadyUsedException, @@ -42,7 +64,6 @@ public void initialize() SchedulerException { if (!initializedDB) { - // Flyway not used in HSQL, do the manually insertions flyway would do for (int i = 1; i <= 4; i++) { if (repositoryService .findRepositoriesIsNotDeletedOrderByName("scheduled-job-" + i) @@ -54,35 +75,32 @@ public void initialize() initializedDB = true; } - ThirdPartySyncJobConfig scheduledJobOne = new ThirdPartySyncJobConfig(); - scheduledJobOne.setUuid("e4c72563-d8f0-4465-9eec-9ed96087665e"); - scheduledJobOne.setCron("0/5 * * * * ?"); - scheduledJobOne.setRepository("scheduled-job-1"); - scheduledJobOne.setThirdPartyProjectId("123456"); - scheduledJobOne.setActions(List.of()); - scheduledJobOne.setPluralSeparator("_"); - scheduledJobOne.setLocaleMapping(""); - scheduledJobOne.setSkipTextUnitsWithPattern(""); - scheduledJobOne.setSkipAssetsWithPathPattern(""); - scheduledJobOne.setIncludeTextUnitsWithPattern(""); - scheduledJobOne.setOptions(List.of()); - - ThirdPartySyncJobConfig scheduledJobTwo = new ThirdPartySyncJobConfig(); - scheduledJobTwo.setUuid("e4c72563-d8f0-4465-9eec-aaa96087665e"); - scheduledJobTwo.setCron("0/5 * * * * ?"); - scheduledJobTwo.setRepository("scheduled-job-2"); - scheduledJobTwo.setThirdPartyProjectId("123456"); - scheduledJobTwo.setActions(List.of()); - scheduledJobTwo.setPluralSeparator("_"); - scheduledJobTwo.setLocaleMapping(""); - scheduledJobTwo.setSkipTextUnitsWithPattern(""); - scheduledJobTwo.setSkipAssetsWithPathPattern(""); - scheduledJobTwo.setIncludeTextUnitsWithPattern(""); - scheduledJobTwo.setOptions(List.of()); + ThirdPartySyncJobConfig scheduledJobOne = + createThirdPartySyncJobConfig(TEST_UUID, "scheduled-job-1"); + + ThirdPartySyncJobConfig scheduledJobTwo = + createThirdPartySyncJobConfig("7d39ee64-415e-42bb-8492-33f1909484c9", "scheduled-job-2"); + + // No cron schedule, shouldn't schedule + ThirdPartySyncJobConfig scheduledJobThree = + createThirdPartySyncJobConfig("f1d1a12d-b23e-4ef8-9391-f647be6f9db4", "scheduled-job-3"); + scheduledJobThree.setCron(null); + + // No uuid, shouldn't schedule + ThirdPartySyncJobConfig scheduledJobFour = + createThirdPartySyncJobConfig(null, "scheduled-job-4"); ThirdPartySyncJobsConfig thirdPartySyncJobsConfig = new ThirdPartySyncJobsConfig(); thirdPartySyncJobsConfig.setThirdPartySyncJobs( - ImmutableMap.of("scheduled-job-1", scheduledJobOne, "scheduled-job-2", scheduledJobTwo)); + ImmutableMap.of( + "scheduled-job-1", + scheduledJobOne, + "scheduled-job-2", + scheduledJobTwo, + "scheduled-job-3", + scheduledJobThree, + "scheduled-job-4", + scheduledJobFour)); // Return a Spy with the Job always being a no-op job scheduledJobManager = @@ -96,15 +114,13 @@ public void initialize() repositoryRepository, deadlockRetryTemplate)); - Mockito.doReturn(NoOpScheduledJobTest.class) - .when(scheduledJobManager) - .loadJobClass(ScheduledJobType.THIRD_PARTY_SYNC.getJobClassName()); + Mockito.doReturn(NoOpScheduledJobTest.class).when(scheduledJobManager).loadJobClass(any()); scheduledJobManager.init(); } @Test - public void testListenersExists() throws Exception { + public void testAListenersExists() throws Exception { assertEquals( 1, scheduledJobManager.getScheduler().getListenerManager().getJobListeners().size()); @@ -113,32 +129,129 @@ public void testListenersExists() throws Exception { } @Test - public void testQuartzScheduledJob() throws Exception { - assertEquals( - 2, - scheduledJobManager - .getScheduler() - .getJobKeys(GroupMatcher.groupEquals(ScheduledJobType.THIRD_PARTY_SYNC.toString())) - .size()); - - Thread.sleep(10000); - - ScheduledJob sj = - scheduledJobRepository.findByUuid("e4c72563-d8f0-4465-9eec-9ed96087665e").get(); - assertEquals(sj.getJobStatus().getEnum(), ScheduledJobStatus.SUCCEEDED); - assertNotNull(sj.getEndDate()); + public void testBQuartzScheduledJob() throws Exception { + assertEquals(2, scheduledJobManager.getAllJobKeys().size()); + assertEquals(2, scheduledJobRepository.findAll().size()); + } + + @Test + public void testCJobSucceeds() throws Exception { + ScheduledJob scheduledJob = getTestJob(); + scheduledJob.setEndDate(null); + scheduledJobRepository.save(scheduledJob); + + NoOpScheduledJobTest.successEvent = false; + + // Wait for job to be in progress + waitForCondition( + () -> getTestJob().getJobStatus().getEnum() == ScheduledJobStatus.IN_PROGRESS, + "Max retries met waiting for scheduled job to go in progress."); + + assertNotNull(getTestJob().getStartDate()); + + // Wait for job to succeed + waitForCondition( + () -> getTestJob().getJobStatus().getEnum() == ScheduledJobStatus.SUCCEEDED, + "Max retries met waiting for scheduled job to succeed."); + + assertNotNull(getTestJob().getEndDate()); + // Make sure onSuccess event was called by listener + assertTrue(NoOpScheduledJobTest.successEvent); + } + + @Test + public void testDEnableDisableJob() throws Exception { + ScheduledJob scheduledJob = getTestJob(); + scheduledJob.setEnabled(false); + scheduledJob.setEndDate(null); + scheduledJobRepository.save(scheduledJob); + ZonedDateTime start = scheduledJob.getStartDate(); + // Let the job run + Thread.sleep(JOB_WAIT_TIME_MS); + assertEquals(start, getTestJob().getStartDate()); + + // Enable the job and ensure the end date is present + scheduledJob.setEnabled(true); + scheduledJobRepository.save(scheduledJob); + Thread.sleep(JOB_WAIT_TIME_MS); + assertNotEquals(start, getTestJob().getEndDate()); + } + + @Test + public void testEJobFails() throws Exception { + // Use static variable to manipulate the job, Quartz is in charge of creating the instance of + // the job so a spy won't work without using SpyBean which causes a new context to be spun up. + NoOpScheduledJobTest.throwException = true; + NoOpScheduledJobTest.failureEvent = false; + Thread.sleep(JOB_WAIT_TIME_MS); + waitForCondition( + () -> getTestJob().getJobStatus().getEnum() == ScheduledJobStatus.FAILED, + "Max retries met waiting for scheduled job to fail."); + + // Check the start and end dates + assertNotNull(getTestJob().getEndDate()); + assertTrue(NoOpScheduledJobTest.failureEvent); + NoOpScheduledJobTest.throwException = false; } - // @Test - // public void testQuartzCleanup() throws Exception { - // scheduledJobManager.uuidPool = List.of("e4c72563-d8f0-4465-9eec-9ed96087665e"); - // scheduledJobManager.cleanQuartzJobs(); - // assertEquals( - // 1, - // scheduledJobManager - // .getScheduler() - // - // .getJobKeys(GroupMatcher.groupEquals(ScheduledJobType.THIRD_PARTY_SYNC.toString())) - // .size()); - // } + @Test + public void testFJobCleanup() throws Exception { + scheduledJobManager.uuidPool = new HashSet<>(); + scheduledJobManager.cleanQuartzJobs(); + assertEquals(0, scheduledJobManager.getAllJobKeys().size()); + + scheduledJobManager + .getScheduler() + .getListenerManager() + .removeJobListener( + scheduledJobManager + .getScheduler() + .getListenerManager() + .getJobListeners() + .get(0) + .getName()); + + scheduledJobManager + .getScheduler() + .getListenerManager() + .removeTriggerListener( + scheduledJobManager + .getScheduler() + .getListenerManager() + .getTriggerListeners() + .get(0) + .getName()); + } + + private ScheduledJob getTestJob() { + return scheduledJobRepository.findByUuid(TEST_UUID).get(); + } + + private void waitForCondition(Supplier condition, String failMessage) + throws InterruptedException { + int retryCount = 0; + while (!condition.get()) { + if (retryCount++ >= MAX_RETRIES) { + fail(failMessage); + } + Thread.sleep(RETRY_DELAY_MS); + } + } + + private ThirdPartySyncJobConfig createThirdPartySyncJobConfig( + String uuid, String repositoryName) { + ThirdPartySyncJobConfig thirdPartySyncJobConfig = new ThirdPartySyncJobConfig(); + thirdPartySyncJobConfig.setUuid(uuid); + thirdPartySyncJobConfig.setCron("0/2 * * * * ?"); + thirdPartySyncJobConfig.setRepository(repositoryName); + thirdPartySyncJobConfig.setThirdPartyProjectId("123456"); + thirdPartySyncJobConfig.setActions(List.of()); + thirdPartySyncJobConfig.setPluralSeparator("_"); + thirdPartySyncJobConfig.setLocaleMapping(""); + thirdPartySyncJobConfig.setSkipTextUnitsWithPattern(""); + thirdPartySyncJobConfig.setSkipAssetsWithPathPattern(""); + thirdPartySyncJobConfig.setIncludeTextUnitsWithPattern(""); + thirdPartySyncJobConfig.setOptions(List.of()); + return thirdPartySyncJobConfig; + } }