diff --git a/dependencies.gradle b/dependencies.gradle index f67af46..36dfd19 100644 --- a/dependencies.gradle +++ b/dependencies.gradle @@ -10,7 +10,6 @@ ext { jacksonYamlDep = 'com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.+' javaxInjectDep = 'javax.inject:javax.inject:1' javaxValidationDep = 'javax.validation:validation-api:2.0.+' - quartzDep = 'org.quartz-scheduler:quartz:2.3.+' jodaTimeDep = 'joda-time:joda-time:2.10.+' cronutilsDep = 'com.cronutils:cron-utils:9.1.+' zipkinDep = 'io.zipkin.brave:brave:5.+' diff --git a/maestro-common/build.gradle b/maestro-common/build.gradle index cb4bcdf..3e5d626 100644 --- a/maestro-common/build.gradle +++ b/maestro-common/build.gradle @@ -3,7 +3,6 @@ dependencies { implementation javaxInjectDep implementation javaxValidationDep implementation jacksonYamlDep - implementation quartzDep implementation jodaTimeDep implementation(cronutilsDep) { exclude group: 'org.glassfish' diff --git a/maestro-common/gradle.lockfile b/maestro-common/gradle.lockfile index cc3b5c6..9260ed7 100644 --- a/maestro-common/gradle.lockfile +++ b/maestro-common/gradle.lockfile @@ -7,14 +7,10 @@ com.fasterxml.jackson.core:jackson-core:2.15.4=compileClasspath com.fasterxml.jackson.core:jackson-databind:2.15.4=compileClasspath com.fasterxml.jackson.dataformat:jackson-dataformat-yaml:2.15.4=compileClasspath com.fasterxml.jackson:jackson-bom:2.15.4=compileClasspath -com.mchange:c3p0:0.9.5.4=compileClasspath -com.mchange:mchange-commons-java:0.2.15=compileClasspath -com.zaxxer:HikariCP-java7:2.4.13=compileClasspath javax.inject:javax.inject:1=compileClasspath javax.validation:validation-api:2.0.1.Final=compileClasspath joda-time:joda-time:2.10.14=compileClasspath org.projectlombok:lombok:1.18.34=annotationProcessor,compileClasspath -org.quartz-scheduler:quartz:2.3.2=compileClasspath org.slf4j:slf4j-api:1.7.30=compileClasspath org.yaml:snakeyaml:2.1=compileClasspath empty= diff --git a/maestro-common/src/main/java/com/netflix/maestro/utils/TriggerHelper.java b/maestro-common/src/main/java/com/netflix/maestro/utils/TriggerHelper.java index 732ccb3..3f9708c 100644 --- a/maestro-common/src/main/java/com/netflix/maestro/utils/TriggerHelper.java +++ b/maestro-common/src/main/java/com/netflix/maestro/utils/TriggerHelper.java @@ -12,27 +12,24 @@ */ package com.netflix.maestro.utils; -import com.cronutils.mapper.CronMapper; import com.cronutils.model.Cron; import com.cronutils.model.CronType; import com.cronutils.model.definition.CronDefinitionBuilder; +import com.cronutils.model.time.ExecutionTime; import com.cronutils.parser.CronParser; -import com.netflix.maestro.models.Defaults; import com.netflix.maestro.models.trigger.CronTimeTrigger; import com.netflix.maestro.models.trigger.PredefinedTimeTrigger; import com.netflix.maestro.models.trigger.TimeTrigger; import com.netflix.maestro.models.trigger.TimeTriggerWithJitter; -import java.text.ParseException; +import java.time.ZoneId; +import java.time.ZonedDateTime; import java.util.Date; import java.util.Optional; import java.util.Random; -import java.util.TimeZone; import java.util.UUID; import java.util.concurrent.ThreadLocalRandom; import lombok.SneakyThrows; import lombok.extern.slf4j.Slf4j; -import org.joda.time.DateTimeZone; -import org.quartz.CronExpression; /** Cron Helper utility class. * */ @Slf4j @@ -41,49 +38,45 @@ public final class TriggerHelper { private TriggerHelper() {} /** - * Build cron from expression. + * Build cron from cron expression. * * @param cron cron string - * @return cron expression object - * @throws ParseException parse error + * @return the cron expression object */ - public static CronExpression buildCron(String cron) throws ParseException { - return buildCron(cron, Defaults.DEFAULT_TIMEZONE); + public static Cron buildCron(String cron) { + Cron cronExpression = parseUnixCron(cron); + return cronExpression != null ? cronExpression : parseQuartzCron(cron); } - /** - * Build cron from expression and timezone string. - * - * @param cron cron string - * @param timezone timezone - * @return cron expression object - * @throws ParseException parse error - */ - public static CronExpression buildCron(String cron, String timezone) throws ParseException { - return buildCron(cron, DateTimeZone.forID(timezone).toTimeZone()); + private static Cron parseUnixCron(String cron) { + CronParser unixParser = + new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.UNIX)); + try { + return unixParser.parse(cron); + } catch (IllegalArgumentException e) { + LOG.debug("Invalid unix cron expression {}", cron, e); + } + return null; } - /** - * Build cron from cron expression and timezone. - * - * @param cron cron string - * @param timezone timezone - * @return the cron expression object - * @throws ParseException parse error - */ - public static CronExpression buildCron(String cron, TimeZone timezone) throws ParseException { - String cronStr = cron; + private static Cron parseQuartzCron(String cron) { CronParser unixParser = - new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.UNIX)); + new CronParser(CronDefinitionBuilder.instanceDefinitionFor(CronType.QUARTZ)); try { - Cron parsedCron = unixParser.parse(cron); - cronStr = CronMapper.fromUnixToQuartz().map(parsedCron).asString(); + return unixParser.parse(cron); } catch (IllegalArgumentException e) { - LOG.debug("Unix cron parsing not successful for " + cron, e); + LOG.debug("Invalid quartz cron expression {}", cron, e); } - CronExpression cronExpression = new CronExpression(cronStr); - cronExpression.setTimeZone(timezone); - return cronExpression; + return null; + } + + public static ZonedDateTime nextExecutionTime( + Cron cron, ZonedDateTime startDate, String timezone) { + ZoneId zoneId = ZoneId.of(timezone); + ExecutionTime executionTime = ExecutionTime.forCron(cron); + return executionTime + .nextExecution(startDate.withZoneSameInstant(zoneId)) + .orElseThrow(() -> new IllegalArgumentException("Unable to calculate next execution time")); } /** @@ -96,17 +89,19 @@ public static CronExpression buildCron(String cron, TimeZone timezone) throws Pa * @return next execution date if present */ @SneakyThrows - public static Optional nextExecutionDate( + public static Optional nextExecutionDate( TimeTrigger trigger, Date startDate, String uniqueId) { CronTimeTrigger cronTimeTrigger = getCronTimeTrigger(trigger); if (cronTimeTrigger != null) { - CronExpression cronExpression = - TriggerHelper.buildCron(cronTimeTrigger.getCron(), cronTimeTrigger.getTimezone()); - Date nextTime = cronExpression.getNextValidTimeAfter(startDate); + Cron cron = TriggerHelper.buildCron(cronTimeTrigger.getCron()); + ZonedDateTime zonedDateTime = + ZonedDateTime.ofInstant(startDate.toInstant(), ZoneId.of(cronTimeTrigger.getTimezone())); + ZonedDateTime nextTime = + TriggerHelper.nextExecutionTime(cron, zonedDateTime, cronTimeTrigger.getTimezone()); if (nextTime != null) { - nextTime.setTime( - nextTime.getTime() - + getDelayInSeconds(cronTimeTrigger, uniqueId) * TimeTrigger.MS_IN_SECONDS); + nextTime = + nextTime.plusSeconds( + (int) (getDelayInSeconds(cronTimeTrigger, uniqueId) * TimeTrigger.MS_IN_SECONDS)); } return Optional.ofNullable(nextTime); } @@ -119,8 +114,7 @@ private static CronTimeTrigger getCronTimeTrigger(TimeTrigger trigger) { CronTimeTrigger cronTimeTrigger = null; if (trigger instanceof CronTimeTrigger) { cronTimeTrigger = (CronTimeTrigger) trigger; - } else if (trigger instanceof PredefinedTimeTrigger) { - PredefinedTimeTrigger timeTrigger = (PredefinedTimeTrigger) trigger; + } else if (trigger instanceof PredefinedTimeTrigger timeTrigger) { cronTimeTrigger = new CronTimeTrigger(); cronTimeTrigger.setCron(timeTrigger.getExpression().key()); cronTimeTrigger.setTimezone(timeTrigger.getTimezone()); diff --git a/maestro-common/src/main/java/com/netflix/maestro/validations/CronConstraint.java b/maestro-common/src/main/java/com/netflix/maestro/validations/CronConstraint.java index 6f14d48..2acc3b3 100644 --- a/maestro-common/src/main/java/com/netflix/maestro/validations/CronConstraint.java +++ b/maestro-common/src/main/java/com/netflix/maestro/validations/CronConstraint.java @@ -12,13 +12,13 @@ */ package com.netflix.maestro.validations; +import com.cronutils.model.Cron; import com.netflix.maestro.utils.TriggerHelper; import java.lang.annotation.Documented; import java.lang.annotation.ElementType; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.annotation.Target; -import java.text.ParseException; import javax.validation.Constraint; import javax.validation.ConstraintValidator; import javax.validation.ConstraintValidatorContext; @@ -52,14 +52,12 @@ public boolean isValid(String cronExpression, ConstraintValidatorContext context .addConstraintViolation(); return false; } - try { - TriggerHelper.buildCron(cronExpression); - } catch (ParseException e) { + Cron cron = TriggerHelper.buildCron(cronExpression); + if (cron == null) { context .buildConstraintViolationWithTemplate( String.format( - "[cron expression] is not valid - rejected value is [%s] - error: [%s]", - cronExpression, e.getMessage())) + "[cron expression] is not valid - rejected value is [%s]", cronExpression)) .addConstraintViolation(); return false; } diff --git a/maestro-common/src/test/java/com/netflix/maestro/utils/TriggerHelperTest.java b/maestro-common/src/test/java/com/netflix/maestro/utils/TriggerHelperTest.java index 414d5c5..b3b0dbb 100644 --- a/maestro-common/src/test/java/com/netflix/maestro/utils/TriggerHelperTest.java +++ b/maestro-common/src/test/java/com/netflix/maestro/utils/TriggerHelperTest.java @@ -12,12 +12,14 @@ */ package com.netflix.maestro.utils; -import static org.junit.Assert.*; +import static org.junit.Assert.assertEquals; import com.netflix.maestro.AssertHelper; import com.netflix.maestro.MaestroBaseTest; import com.netflix.maestro.models.trigger.TimeTrigger; import java.time.Instant; +import java.time.ZoneId; +import java.time.ZonedDateTime; import java.util.Date; import java.util.Optional; import org.junit.Test; @@ -28,18 +30,22 @@ public class TriggerHelperTest extends MaestroBaseTest { public void testNextExecutionDateForCron() throws Exception { TimeTrigger trigger = loadObject("fixtures/time_triggers/sample-cron-time-trigger.json", TimeTrigger.class); - Optional actual = + Optional actual = TriggerHelper.nextExecutionDate(trigger, Date.from(Instant.EPOCH), "test-id"); - assertEquals(Optional.of(Date.from(Instant.ofEpochSecond(72000))), actual); + ZonedDateTime expected = + ZonedDateTime.ofInstant(Instant.ofEpochSecond(72000), ZoneId.of(trigger.getTimezone())); + assertEquals(Optional.of(expected), actual); } @Test public void testNextExecutionDateForPredefined() throws Exception { TimeTrigger trigger = loadObject("fixtures/time_triggers/sample-predefined-time-trigger.json", TimeTrigger.class); - Optional actual = + Optional actual = TriggerHelper.nextExecutionDate(trigger, Date.from(Instant.EPOCH), "test-id"); - assertEquals(Optional.of(Date.from(Instant.ofEpochSecond(28800))), actual); + ZonedDateTime expected = + ZonedDateTime.ofInstant(Instant.ofEpochSecond(28800), ZoneId.of(trigger.getTimezone())); + assertEquals(Optional.of(expected), actual); } @Test diff --git a/maestro-common/src/test/java/com/netflix/maestro/validations/CronConstraintTest.java b/maestro-common/src/test/java/com/netflix/maestro/validations/CronConstraintTest.java index 83f864b..8009abf 100644 --- a/maestro-common/src/test/java/com/netflix/maestro/validations/CronConstraintTest.java +++ b/maestro-common/src/test/java/com/netflix/maestro/validations/CronConstraintTest.java @@ -44,9 +44,7 @@ public void testInvalid() { assertEquals(1, violations.size()); ConstraintViolation violation = violations.iterator().next(); assertThat(violation.getMessage()) - .contains( - "[cron expression] is not valid " - + "- rejected value is [* * blah blah] - error: [Illegal characters for this position"); + .contains("[cron expression] is not valid " + "- rejected value is [* * blah blah]"); } @Test diff --git a/maestro-engine/src/main/java/com/netflix/maestro/engine/utils/WorkflowEnrichmentHelper.java b/maestro-engine/src/main/java/com/netflix/maestro/engine/utils/WorkflowEnrichmentHelper.java index a39a73c..28f62e1 100644 --- a/maestro-engine/src/main/java/com/netflix/maestro/engine/utils/WorkflowEnrichmentHelper.java +++ b/maestro-engine/src/main/java/com/netflix/maestro/engine/utils/WorkflowEnrichmentHelper.java @@ -21,6 +21,7 @@ import com.netflix.maestro.models.definition.WorkflowDefinitionExtras; import com.netflix.maestro.models.parameter.ParamDefinition; import com.netflix.maestro.utils.TriggerHelper; +import java.time.ZonedDateTime; import java.util.Comparator; import java.util.Date; import java.util.LinkedHashMap; @@ -72,7 +73,7 @@ private void enrichNextRunDate( return; } Date now = new Date(); - Optional earliestDate = + Optional earliestDate = workflowDefinition.getWorkflow().getTimeTriggers().stream() .map( trigger -> @@ -81,7 +82,8 @@ private void enrichNextRunDate( .filter(Optional::isPresent) .map(Optional::get) .min(Comparator.naturalOrder()); - earliestDate.ifPresent(date -> enrichedWorkflowExtras.setNextExecutionTime(date.getTime())); + earliestDate.ifPresent( + date -> enrichedWorkflowExtras.setNextExecutionTime(date.toEpochSecond())); enrichedWorkflowExtras.setNextExecutionTimes( workflowDefinition.getWorkflow().getTimeTriggers().stream() @@ -89,7 +91,7 @@ private void enrichNextRunDate( trigger -> TriggerHelper.nextExecutionDate( trigger, now, workflowDefinition.getWorkflow().getId())) - .map(date -> date.map(Date::getTime).orElse(null)) + .map(date -> date.map(ZonedDateTime::toEpochSecond).orElse(null)) .collect(Collectors.toList())); } } diff --git a/maestro-server/gradle.lockfile b/maestro-server/gradle.lockfile index 90c4f0b..a1a01e3 100644 --- a/maestro-server/gradle.lockfile +++ b/maestro-server/gradle.lockfile @@ -28,15 +28,12 @@ com.google.inject.extensions:guice-multibindings:4.1.0=compileClasspath,runtimeC com.google.inject:guice:4.1.0=compileClasspath,runtimeClasspath com.google.protobuf:protobuf-java:3.5.1=compileClasspath,runtimeClasspath com.jayway.jsonpath:json-path:2.2.0=compileClasspath,runtimeClasspath -com.mchange:c3p0:0.9.5.4=runtimeClasspath -com.mchange:mchange-commons-java:0.2.15=runtimeClasspath com.netflix.conductor:conductor-common:2.31.5=compileClasspath,runtimeClasspath com.netflix.conductor:conductor-core:2.31.5=compileClasspath,runtimeClasspath com.netflix.servo:servo-core:0.12.17=compileClasspath,runtimeClasspath com.netflix.spectator:spectator-api:0.68.0=compileClasspath com.netflix.spectator:spectator-api:1.7.17=runtimeClasspath com.spotify:completable-futures:0.3.1=compileClasspath,runtimeClasspath -com.zaxxer:HikariCP-java7:2.4.13=runtimeClasspath com.zaxxer:HikariCP:4.0.3=compileClasspath,runtimeClasspath commons-codec:commons-codec:1.9=compileClasspath,runtimeClasspath commons-lang:commons-lang:2.6=compileClasspath,runtimeClasspath @@ -92,7 +89,6 @@ org.jetbrains:annotations:17.0.0=runtimeClasspath org.ow2.asm:asm:5.0.3=compileClasspath,runtimeClasspath org.postgresql:postgresql:42.7.3=compileClasspath,runtimeClasspath org.projectlombok:lombok:1.18.34=annotationProcessor,compileClasspath -org.quartz-scheduler:quartz:2.3.2=runtimeClasspath org.rnorth.duct-tape:duct-tape:1.0.8=runtimeClasspath org.slf4j:jul-to-slf4j:1.7.36=compileClasspath,runtimeClasspath org.slf4j:slf4j-api:1.7.36=compileClasspath,runtimeClasspath