Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(schedule): use croner library to check schedule #32573

Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ module.exports = {
'import/no-named-as-default-member': 0,
'import/no-extraneous-dependencies': [
'error',
{ devDependencies: ['test/**/*', '**/*.spec.ts'] },
{
devDependencies: ['test/**/*', '**/*.spec.ts'],
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
},
],
'import/prefer-default-export': 0, // no benefit

Expand Down
64 changes: 63 additions & 1 deletion lib/workers/repository/update/branch/schedule.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ describe('workers/repository/update/branch/schedule', () => {
it('returns true if schedule uses cron syntax', () => {
expect(schedule.hasValidSchedule(['* 5 * * *'])[0]).toBeTrue();
expect(schedule.hasValidSchedule(['* * * * * 6L'])[0]).toBeTrue();
expect(schedule.hasValidSchedule(['* * */2 6#1'])[0]).toBeTrue();
expect(schedule.hasValidSchedule(['* * * */2 6#1'])[0]).toBeTrue();
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
});

it('massages schedules', () => {
Expand Down Expand Up @@ -267,6 +267,50 @@ describe('workers/repository/update/branch/schedule', () => {
});
});

describe('supports L syntax in cron schedules', () => {
beforeEach(() => {
jest.setSystemTime(new Date('2024-10-31T10:50:00.000'));
viceice marked this conversation as resolved.
Show resolved Hide resolved
});

it('supports last day of month', () => {
config.schedule = ['* * * L *'];
const res = schedule.isScheduledNow(config);
expect(res).toBeTrue();
});

it('supports last day of week', () => {
config.schedule = ['* * * * 4L'];
expect(schedule.isScheduledNow(config)).toBeTrue();

config.schedule = ['* * * * 5L'];
expect(schedule.isScheduledNow(config)).toBeFalse();
});
});

describe('supports # syntax in cron schedules', () => {
it('supports first Monday of month', () => {
jest.setSystemTime(new Date('2024-10-07T10:50:00.000'));
config.schedule = ['* * * * 1#1'];
expect(schedule.isScheduledNow(config)).toBeTrue();
config.schedule = ['* * * * 1#2'];
expect(schedule.isScheduledNow(config)).toBeFalse();
});
});

describe('complex cron schedules', () => {
it.each`
viceice marked this conversation as resolved.
Show resolved Hide resolved
sched | tz | datetime | expected
${'* * 1-7 * 0'} | ${'Asia/Tokyo'} | ${'2024-10-04T10:50:00.000+0900'} | ${true}
${'* * 1-7 * 0'} | ${'Asia/Tokyo'} | ${'2024-10-13T10:50:00.000+0900'} | ${true}
${'* * 1-7 * 0'} | ${'Asia/Tokyo'} | ${'2024-10-16T10:50:00.000+0900'} | ${false}
`('$sched, $tz, $datetime', ({ sched, tz, datetime, expected }) => {
config.schedule = [sched];
config.timezone = tz;
jest.setSystemTime(new Date(datetime));
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
expect(schedule.isScheduledNow(config)).toBe(expected);
});
});

describe('supports timezone', () => {
it.each`
sched | tz | datetime | expected
Expand All @@ -282,6 +326,24 @@ describe('workers/repository/update/branch/schedule', () => {
});
});

it('reject if day mismatch', () => {
config.schedule = ['* 10 21 * *'];
const res = schedule.isScheduledNow(config);
expect(res).toBeFalse();
});

it('reject if month mismatch', () => {
config.schedule = ['* 10 30 1 *'];
const res = schedule.isScheduledNow(config);
expect(res).toBeFalse();
});

it('reject if no schedule available', () => {
config.schedule = ['* * * 1 *'];
const res = schedule.isScheduledNow(config);
expect(res).toBeFalse();
});

it('supports multiple schedules', () => {
config.schedule = ['after 4:00pm', 'before 11:00am'];
const res = schedule.isScheduledNow(config);
Expand Down
58 changes: 25 additions & 33 deletions lib/workers/repository/update/branch/schedule.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,13 @@
import later from '@breejs/later';
import is from '@sindresorhus/is';
import type {
CronExpression,
DayOfTheMonthRange,
DayOfTheWeekRange,
HourRange,
MonthRange,
} from 'cron-parser';
import { parseExpression } from 'cron-parser';
import type { Cron as CronJob } from 'croner';
import { Cron } from 'croner';
import cronstrue from 'cronstrue';
import { DateTime } from 'luxon';
import { fixShortHours } from '../../../../config/migration';
import type { RenovateConfig } from '../../../../config/types';
import { logger } from '../../../../logger';

const minutesChar = '*';

const scheduleMappings: Record<string, string> = {
'every month': 'before 5am on the first day of the month',
monthly: 'before 5am on the first day of the month',
Expand All @@ -24,9 +16,11 @@ const scheduleMappings: Record<string, string> = {
function parseCron(
scheduleText: string,
timezone?: string,
): CronExpression | undefined {
): CronJob | undefined {
try {
return parseExpression(scheduleText, { tz: timezone });
return new Cron(scheduleText, {
...(timezone && { timezone }),
});
} catch {
return undefined;
}
Expand Down Expand Up @@ -54,10 +48,7 @@ export function hasValidSchedule(
const hasFailedSchedules = schedule.some((scheduleText) => {
const parsedCron = parseCron(scheduleText);
if (parsedCron !== undefined) {
if (
parsedCron.fields.minute.length !== 60 ||
scheduleText.indexOf(minutesChar) !== 0
) {
if (scheduleText.split(' ')[0] !== '*') {
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
message = `Invalid schedule: "${scheduleText}" has cron syntax, but doesn't have * as minutes`;
return true;
}
Expand Down Expand Up @@ -99,42 +90,43 @@ export function hasValidSchedule(
return [true];
}

function cronMatches(cron: string, now: DateTime, timezone?: string): boolean {
export function cronMatches(
cron: string,
now: DateTime,
timezone?: string,
): boolean {
const parsedCron = parseCron(cron, timezone);

// it will always parse because it is checked beforehand
// istanbul ignore if
if (!parsedCron) {
return false;
}

if (parsedCron.fields.hour.indexOf(now.hour as HourRange) === -1) {
// Hours mismatch
// return the next date which matches the cron schedule
const nextRun = parsedCron.nextRun();
// istanbul ignore if: should not happen
if (!nextRun) {
logger.warn(`Invalid cron schedule ${cron}. No next run is possible`);
return false;
}

if (
parsedCron.fields.dayOfMonth.indexOf(now.day as DayOfTheMonthRange) === -1
) {
// Days mismatch
let nextDate: DateTime = DateTime.fromISO(nextRun.toISOString());
if (timezone) {
nextDate = nextDate.setZone(timezone);
RahulGautamSingh marked this conversation as resolved.
Show resolved Hide resolved
}

if (nextDate.hour !== now.hour) {
return false;
}

if (
!parsedCron.fields.dayOfWeek.includes(
(now.weekday % 7) as DayOfTheWeekRange,
)
) {
// Weekdays mismatch
if (nextDate.day !== now.day) {
return false;
}

if (parsedCron.fields.month.indexOf(now.month as MonthRange) === -1) {
// Months mismatch
if (nextDate.month !== now.month) {
return false;
}

// Match
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@
"clean-git-ref": "2.0.1",
"commander": "12.1.0",
"conventional-commits-detector": "1.0.3",
"cron-parser": "4.9.0",
"croner": "9.0.0",
"cronstrue": "2.51.0",
"deepmerge": "4.3.1",
"dequal": "2.0.3",
Expand Down
17 changes: 7 additions & 10 deletions pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.