fix: align every()+startOnTime()/between() to next period boundary instead of firing immediately - #687
Merged
Conversation
…stead of firing immediately
A scheduled task built with every(period).startOnTime("HH:mm") (or
.between(startTime, endTime)) fired immediately the moment the scheduler
registered it (e.g. on every server/container restart) instead of waiting
for the next period boundary counted from the daily start time.
Root cause: every() only set variables.period/timeUnit, never
variables.delay (defaults to 0). startTime/endTime were only consulted by
isConstrained() as a runtime gate on later ticks, not fed into the initial
delay calculation, so start() handed delay=0 straight to
scheduleAtFixedRate().
Fix mirrors the existing "smart" everyDayAt()/setInitialDelayPeriodAndTimeUnit()
pattern: start() now computes an initial delay aligned to the next period
boundary from startTime when period > 0, delay == 0, and startTime is set.
Tasks with an explicit delay() or one-off schedule() are unaffected.
This is a behavior change for anyone relying on the old "fire immediately
at registration" semantics for every()+startOnTime()/between().
Mirrors the identical fix already merged in ortus-boxlang/BoxLang PR #606
(Jira BL-2633).
CI's Checks Source Code Formatting job flagged two formatting violations introduced by the previous commit: a misaligned consecutive assignment in calculateStartTimeAlignedDelay(), and method chains in the new tests that needed to wrap per the project's method_call.chain.multiline rule. Verified locally with the project's own cfformat (./.cfformat.json) - only a pre-existing, unrelated violation in system/web/routing/Router.cfc remains, which also fails on development and is out of scope here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
A scheduled task built with
every(period).startOnTime("HH:mm")(or.between(startTime, endTime)) fired immediately the moment the scheduler registered it (e.g. on every server/container restart) instead of waiting for the next period boundary counted from the daily start time.Example:
every(1800, "seconds").startOnTime("00:00")should wait until the next 30-minute boundary from midnight (00:00, 00:30, 01:00, ...) — instead it fired right away at registration time, every time.Root cause:
every()only setsvariables.period/variables.timeUnit, nevervariables.delay(defaults to0).startTime/endTimeare only consulted byisConstrained()as a runtime gate on later ticks — not fed into the initial delay calculation — sostart()handeddelay=0straight toscheduleAtFixedRate(). The "smart"everyDayAt()/everyHourAt()/etc. methods already compute a real future first-run time viasetInitialDelayPeriodAndTimeUnit(), but that pattern was never wired up forevery()+startOnTime()/between().Fix:
system/async/tasks/ScheduledTask.cfc—start()now computes an initial delay aligned to the next period boundary fromstartTimewhenperiod > 0,delay == 0, andstartTimeis set, via a new privatecalculateStartTimeAlignedDelay()method that mirrors the existingeveryDayAt()/setInitialDelayPeriodAndTimeUnit()pattern. Tasks with an explicitdelay()already set, or one-offschedule()tasks, are unaffected.This mirrors the identical fix already merged in
ortus-boxlang/BoxLangPR #606 (Jira BL-2633).Note: this is a behavior change for anyone currently relying on the old "fire immediately at registration" semantics for
every()+startOnTime()/between().Jira Issues
Type of change
Checklist
box run-script format:checkwas not run — CommandBox wasn't available in the environment this PR was prepared in)tests/specs/async/tasks/ScheduledTaskSpec.cfcvia TestBox before merge)