Fix for exception w/ month+ periods. (mesos#800)

If a job has a period which exceeds 1 month and is specified using the month or year notation, an exception will be thrown because months and years do not have fixed durations.
gokulchandrap · Feb 10, 2017 · 29231f6 · 29231f6
1 parent d7da535
commit 29231f6
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 14 deletions.
diff --git a/src/main/scala/org/apache/mesos/chronos/scheduler/jobs/JobUtils.scala b/src/main/scala/org/apache/mesos/chronos/scheduler/jobs/JobUtils.scala
@@ -9,7 +9,7 @@ import org.apache.commons.math3.stat.descriptive.DescriptiveStatistics
 import org.apache.mesos.chronos.scheduler.state.PersistenceStore
 import org.apache.mesos.chronos.utils.{JobDeserializer, JobSerializer}
 import org.joda.time.format.DateTimeFormat
-import org.joda.time.{DateTime, Period, Seconds}
+import org.joda.time.{DateTime, Duration, Period, Seconds}
 
 import scala.collection.mutable
 import scala.collection.mutable.ListBuffer
@@ -126,24 +126,19 @@ object JobUtils {
   protected def calculateSkips(dateTime: DateTime,
                                jobStart: DateTime,
                                period: Period): Int = {
-    // If the period is at least a month, we have to actually add the period to the date
-    // until it's in the future because a month-long period might have different seconds
-    if (period.getMonths >= 1) {
-      var skips = 0
-      var newDate = new DateTime(jobStart)
-      while (newDate.isBefore(dateTime)) {
-        newDate = newDate.plus(period)
-        skips += 1
-      }
-      skips
-    } else {
-      if (jobStart.isBefore(dateTime) && period.toStandardSeconds.getSeconds > 0) {
+    if (jobStart.isBefore(dateTime) && !period.equals(Period.ZERO)) {
+      if (period.getMonths == 0 && period.getYears == 0) {
         Seconds
           .secondsBetween(jobStart, dateTime)
           .getSeconds / period.toStandardSeconds.getSeconds + 1
       } else {
-        0
+        val duration = period.toDurationFrom(dateTime)
+        Seconds
+          .secondsBetween(jobStart, dateTime)
+          .getSeconds / duration.toStandardSeconds.getSeconds + 1
       }
+    } else {
+      0
     }
   }
 

diff --git a/src/test/scala/org/apache/mesos/chronos/scheduler/jobs/JobUtilsSpec.scala b/src/test/scala/org/apache/mesos/chronos/scheduler/jobs/JobUtilsSpec.scala
@@ -186,4 +186,36 @@ class JobUtilsSpec extends SpecificationWithJUnit with Mockito {
 
     JobUtils.isValidJobName(jobName)
   }
+
+  "Skip forward to current date and stop for P1D" in {
+    val now = new DateTime()
+    val schedule = s"R/${now.minusDays(1).toDateTimeISO.toString}/P1D"
+    val job = ScheduleBasedJob(schedule, "sample-name", "sample-command")
+
+    // Get the schedule stream, which should have been skipped forward
+    val stream = JobUtils.skipForward(job, now)
+    val (repeat, scheduledTime, period) =
+      Iso8601Expressions.parse(stream.get.schedule, job.scheduleTimeZone).get
+
+    repeat must_== -1
+    period.getDays must_== 1
+    period.getYears must_== 0
+    scheduledTime.toLocalDate must_== now.plusDays(1).toLocalDate
+  }
+
+  "Skip forward to current date and stop for P1Y" in {
+    val now = new DateTime()
+    val schedule = s"R/${now.minusDays(1).toDateTimeISO.toString}/P1Y"
+    val job = ScheduleBasedJob(schedule, "sample-name", "sample-command")
+
+    // Get the schedule stream, which should have been skipped forward
+    val stream = JobUtils.skipForward(job, now)
+    val (repeat, scheduledTime, period) =
+      Iso8601Expressions.parse(stream.get.schedule, job.scheduleTimeZone).get
+
+    repeat must_== -1
+    period.getDays must_== 0
+    period.getYears must_== 1
+    scheduledTime.toLocalDate must_== now.minusDays(1).plusYears(1).toLocalDate
+  }
 }