diff --git a/changelog.md b/changelog.md index 999659d50..d518355f4 100644 --- a/changelog.md +++ b/changelog.md @@ -15,6 +15,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 cache, which reads a bare number as seconds, so every timeout expired sixty times too soon. A region moved from `CacheBoxProvider` to `BoxLangProvider` kept a 10 minute object for 10 seconds. `LuceeProvider` and `CFProvider` already convert. +- [COLDBOX-1434](https://ortussolutions.atlassian.net/browse/COLDBOX-1434): `ScheduledTask` combining + `withNoOverlaps()` with a daily start time (`between()`/`startOnTime()`) snapshotted `spacedDelay` + from `period` before the start-time alignment converted `period`/`timeUnit` to seconds, so the + original unit's value (e.g. `1` for "1 minute") was scheduled using the converted `timeUnit` + ("seconds") instead. A `1` minute task with `withNoOverlaps()` and an aligned start time re-fired + every second instead of every 60 seconds. ## [8.1.0] - 2026-04-14 diff --git a/system/async/tasks/ScheduledTask.cfc b/system/async/tasks/ScheduledTask.cfc index 582a44193..005354250 100644 --- a/system/async/tasks/ScheduledTask.cfc +++ b/system/async/tasks/ScheduledTask.cfc @@ -805,11 +805,6 @@ component accessors="true" { * @return A ScheduledFuture from where you can monitor the task, an empty ScheduledFuture if the task was not registered */ ScheduledFuture function start(){ - // If we have overlaps and the spaced delay is 0 then grab it from the period - if ( variables.noOverlaps && variables.spacedDelay == 0 ) { - variables.spacedDelay = variables.period; - } - // If we have a delay and a delayTimeUnit, then we need to compare to our // current timeUnit and convert to support the delay // ( only if our time unit is seconds , if not we disable the delay ) @@ -841,6 +836,14 @@ component accessors="true" { calculateStartTimeAlignedDelay(); } + // If we have noOverlaps and the spaced delay is 0 then grab it from the period. + // This must happen AFTER any start-time alignment above, since that can convert + // the period/timeUnit (e.g. minutes -> seconds); otherwise spacedDelay would be + // snapshotted in the original unit while timeUnit has already changed. + if ( variables.noOverlaps && variables.spacedDelay == 0 ) { + variables.spacedDelay = variables.period; + } + debugLog( "start", { diff --git a/tests/specs/async/tasks/ScheduledTaskSpec.cfc b/tests/specs/async/tasks/ScheduledTaskSpec.cfc index 656b5eba6..f5faef3d5 100644 --- a/tests/specs/async/tasks/ScheduledTaskSpec.cfc +++ b/tests/specs/async/tasks/ScheduledTaskSpec.cfc @@ -204,6 +204,46 @@ component extends="tests.specs.async.BaseAsyncSpec" { t.start(); expect( t.getDelay() ).toBe( 5 ); } ); + + it( "keeps spacedDelay consistent with the converted period/timeUnit when withNoOverlaps() is combined with startOnTime()/between() (COLDBOX-1434)", function(){ + var t = scheduler + .task( "test" ) + .every( 1, "minutes" ) + .between( "00:00", "23:59" ) + .withNoOverlaps(); + t.start(); + + // startOnTime()/between() align the period to seconds : 1 minute -> 60 seconds + expect( t.getTimeUnit() ).toBe( "seconds" ); + expect( t.getPeriod() ).toBe( 60 ); + // spacedDelay must be derived from the ALREADY converted period, not the + // original "1" ( minute ) value, or it gets scheduled as 1 second instead of 60 + expect( t.getSpacedDelay() ).toBe( t.getPeriod() ); + } ); + + it( "keeps spacedDelay consistent for withNoOverlaps() without a start time (unchanged behavior)", function(){ + var t = scheduler + .task( "test" ) + .every( 1, "minutes" ) + .withNoOverlaps(); + t.start(); + + expect( t.getTimeUnit() ).toBe( "minutes" ); + expect( t.getPeriod() ).toBe( 1 ); + expect( t.getSpacedDelay() ).toBe( t.getPeriod() ); + } ); + + it( "does not override an explicitly set spacedDelay() when combined with startOnTime()", function(){ + var t = scheduler + .task( "test" ) + .every( 1, "minutes" ) + .startOnTime( "00:00" ) + .spacedDelay( 3600, "seconds" ) + .withNoOverlaps(); + t.start(); + + expect( t.getSpacedDelay() ).toBe( 3600 ); + } ); } ); describe( "can register frequencies with constraints", function(){