mirror of
https://github.com/nocobase/nocobase.git
synced 2026-08-28 17:43:07 +08:00
fix(workflow): avoid extra trigger at startsOn for cron based schedule (#10410)
When a static schedule workflow repeats on a cron expression, getNextTime() returned `startTime` as soon as `startsOn` was in the future, without ever consulting `repeat`. The workflow therefore fired once at `startsOn` even when that moment did not match the expression, and only followed the cron afterwards. That shortcut is correct for a numeric `repeat`, where the period is `startsOn + n * repeat` and `startsOn` is occurrence #0, so it is now limited to the non-cron cases. For a cron string the next match is computed from `startsOn` instead of "now", stepping back one second so that a `startsOn` which does match the expression still triggers. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+54
@@ -190,6 +190,60 @@ describe('workflow > triggers > schedule > static mode', () => {
|
||||
expect(date.getTime()).toBe(now.getTime());
|
||||
});
|
||||
|
||||
it('start in future should not trigger on a time not matching cron', async () => {
|
||||
await sleepToEvenSecond();
|
||||
|
||||
const base = new Date();
|
||||
base.setMilliseconds(0);
|
||||
// starts 2 seconds later, while the cron only matches 4 seconds later
|
||||
const start = new Date(base.getTime() + 2000);
|
||||
const cronTime = new Date(base.getTime() + 4000);
|
||||
|
||||
const workflow = await WorkflowRepo.create({
|
||||
values: {
|
||||
enabled: true,
|
||||
type: 'schedule',
|
||||
config: {
|
||||
mode: 0,
|
||||
startsOn: start.toISOString(),
|
||||
repeat: `${cronTime.getSeconds()} * * * * *`,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await sleep(6000);
|
||||
|
||||
const executions = await workflow.getExecutions();
|
||||
expect(executions.length).toBe(1);
|
||||
expect(new Date(executions[0].context.date).getTime()).toBe(cronTime.getTime());
|
||||
});
|
||||
|
||||
it('start in future should trigger when it matches cron', async () => {
|
||||
await sleepToEvenSecond();
|
||||
|
||||
const base = new Date();
|
||||
base.setMilliseconds(0);
|
||||
const start = new Date(base.getTime() + 2000);
|
||||
|
||||
const workflow = await WorkflowRepo.create({
|
||||
values: {
|
||||
enabled: true,
|
||||
type: 'schedule',
|
||||
config: {
|
||||
mode: 0,
|
||||
startsOn: start.toISOString(),
|
||||
repeat: `${start.getSeconds()} * * * * *`,
|
||||
},
|
||||
},
|
||||
});
|
||||
|
||||
await sleep(3000);
|
||||
|
||||
const executions = await workflow.getExecutions();
|
||||
expect(executions.length).toBe(1);
|
||||
expect(new Date(executions[0].context.date).getTime()).toBe(start.getTime());
|
||||
});
|
||||
|
||||
it('no repeat triggered then update to repeat', async () => {
|
||||
const start = await sleepToEvenSecond();
|
||||
|
||||
|
||||
+9
-2
@@ -63,7 +63,11 @@ export default class StaticScheduleTrigger {
|
||||
currentDate.setMilliseconds(nextSecond ? 1000 : 0);
|
||||
const timestamp = currentDate.getTime();
|
||||
const startTime = parseDateWithoutMs(config.startsOn);
|
||||
if (startTime > timestamp) {
|
||||
// NOTE: a cron expression fully defines its own trigger moments, so `startsOn` only acts as a lower bound for them.
|
||||
// Returning `startTime` here would fire once at `startsOn` even when it does not match the expression. When
|
||||
// `repeat` is a number the period is `startsOn + n * repeat`, which makes `startsOn` the very first occurrence, so
|
||||
// that case keeps returning it.
|
||||
if (startTime > timestamp && typeof config.repeat !== 'string') {
|
||||
return startTime;
|
||||
}
|
||||
if (config.repeat) {
|
||||
@@ -72,7 +76,10 @@ export default class StaticScheduleTrigger {
|
||||
return null;
|
||||
}
|
||||
if (typeof config.repeat === 'string') {
|
||||
const interval = parser.parseExpression(config.repeat, { currentDate });
|
||||
// NOTE: `next()` is exclusive, so step back a second to keep `startsOn` itself eligible when it happens to
|
||||
// match the expression.
|
||||
const base = startTime > timestamp ? new Date(startTime - 1000) : currentDate;
|
||||
const interval = parser.parseExpression(config.repeat, { currentDate: base });
|
||||
const next = interval.next();
|
||||
return next.getTime();
|
||||
} else if (typeof config.repeat === 'number') {
|
||||
|
||||
Reference in New Issue
Block a user