Skip to content

Commit 8951701

Browse files
committed
Use duration field for cronjobs
1 parent 22eaf55 commit 8951701

File tree

6 files changed

+161
-63
lines changed

6 files changed

+161
-63
lines changed

packages/snaps-controllers/src/cronjob/CronjobController.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,11 @@ import { castDraft } from 'immer';
1717
import { DateTime } from 'luxon';
1818
import { nanoid } from 'nanoid';
1919

20-
import { getCurrentDate, getExecutionDate } from './utils';
20+
import {
21+
getCronjobSpecificationSchedule,
22+
getCurrentDate,
23+
getExecutionDate,
24+
} from './utils';
2125
import type {
2226
GetAllSnaps,
2327
HandleSnapRequest,
@@ -441,7 +445,7 @@ export class CronjobController extends BaseController<
441445
snapId,
442446
id: `cronjob-${snapId}-${idx}`,
443447
request: definition.request,
444-
schedule: definition.expression,
448+
schedule: getCronjobSpecificationSchedule(definition),
445449
recurring: true,
446450
};
447451
});

packages/snaps-controllers/src/cronjob/utils.test.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,32 @@
1-
import { getCurrentDate, getExecutionDate } from './utils';
1+
import {
2+
getCronjobSpecificationSchedule,
3+
getCurrentDate,
4+
getExecutionDate,
5+
} from './utils';
26

37
jest.useFakeTimers();
48
jest.setSystemTime(1747994147000);
59

10+
describe('getCronjobSpecificationSchedule', () => {
11+
it('returns the duration if specified', () => {
12+
const specification = {
13+
duration: 'PT1H',
14+
request: { method: 'foo' },
15+
};
16+
17+
expect(getCronjobSpecificationSchedule(specification)).toBe('PT1H');
18+
});
19+
20+
it('returns the expression if no duration is specified', () => {
21+
const specification = {
22+
expression: '0 0 * * *',
23+
request: { method: 'foo' },
24+
};
25+
26+
expect(getCronjobSpecificationSchedule(specification)).toBe('0 0 * * *');
27+
});
28+
});
29+
630
describe('getCurrentDate', () => {
731
it('returns the current date in ISO 8601 format without milliseconds', () => {
832
expect(getCurrentDate()).toBe('2025-05-23T09:55:47Z');

packages/snaps-controllers/src/cronjob/utils.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,29 @@
1-
import { assert } from '@metamask/utils';
1+
import type { CronjobSpecification } from '@metamask/snaps-utils';
2+
import { assert, hasProperty } from '@metamask/utils';
23
import { parseExpression } from 'cron-parser';
34
import { DateTime, Duration } from 'luxon';
45

6+
/**
7+
* Get the schedule from a cronjob specification.
8+
*
9+
* This function assumes the cronjob specification is valid and contains
10+
* either a `duration` or an `expression` property.
11+
*
12+
* @param specification - The cronjob specification to extract the schedule
13+
* from.
14+
* @returns The schedule of the cronjob, which can be either an ISO 8601
15+
* duration or a cron expression.
16+
*/
17+
export function getCronjobSpecificationSchedule(
18+
specification: CronjobSpecification,
19+
) {
20+
if (hasProperty(specification, 'duration')) {
21+
return specification.duration as string;
22+
}
23+
24+
return specification.expression;
25+
}
26+
527
/**
628
* Get a duration with a minimum of 1 second. This function assumes the provided
729
* duration is valid.

packages/snaps-utils/coverage.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,5 +2,5 @@
22
"branches": 99.76,
33
"functions": 99.02,
44
"lines": 98.66,
5-
"statements": 97.25
5+
"statements": 97.32
66
}
Lines changed: 88 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -1,69 +1,113 @@
1+
import { create } from '@metamask/superstruct';
2+
13
import {
4+
CronjobSpecificationStruct,
25
isCronjobSpecification,
36
isCronjobSpecificationArray,
4-
parseCronExpression,
57
} from './cronjob';
68

7-
describe('Cronjob Utilities', () => {
8-
describe('isCronjobSpecification', () => {
9-
it('returns true for a valid cronjob specification', () => {
10-
const cronjobSpecification = {
11-
expression: '* * * * *',
9+
describe('CronjobSpecificationStruct', () => {
10+
it.each([
11+
['100 * * * * *', 'Expected an object, but received: "100 * * * * *"'],
12+
['PT10S', 'Expected an object, but received: "PT10S"'],
13+
[
14+
{
15+
expression: '100 * * * * *',
1216
request: {
1317
method: 'exampleMethodOne',
1418
params: ['p1'],
1519
},
16-
};
17-
expect(isCronjobSpecification(cronjobSpecification)).toBe(true);
18-
});
19-
20-
it('returns false for an invalid cronjob specification', () => {
21-
const cronjobSpecification = {
22-
expression: '* * * * * * * * * * *',
20+
},
21+
'At path: expression -- Expected a cronjob expression, but received: "100 * * * * *"',
22+
],
23+
[
24+
{
25+
duration: '10S',
26+
request: {
27+
method: 'exampleMethodOne',
28+
params: ['p1'],
29+
},
30+
},
31+
'At path: duration -- Not a valid ISO 8601 duration',
32+
],
33+
[
34+
{
2335
request: {
2436
method: 'exampleMethodOne',
2537
params: ['p1'],
2638
},
27-
};
28-
expect(isCronjobSpecification(cronjobSpecification)).toBe(false);
29-
});
39+
},
40+
'At path: expression -- Expected a string, but received: undefined',
41+
],
42+
])('return a readable error for %p', (value, error) => {
43+
expect(() => create(value, CronjobSpecificationStruct)).toThrow(error);
3044
});
45+
});
3146

32-
describe('isCronjobSpecificationArray', () => {
33-
it('returns true for a valid cronjob specification array', () => {
34-
const cronjobSpecificationArray = [
35-
{
36-
expression: '* * * * *',
37-
request: {
38-
method: 'exampleMethodOne',
39-
params: ['p1'],
40-
},
41-
},
42-
];
43-
expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe(true);
44-
});
47+
describe('isCronjobSpecification', () => {
48+
it('returns true for a valid cronjob specification with an expression', () => {
49+
const cronjobSpecification = {
50+
expression: '* * * * *',
51+
request: {
52+
method: 'exampleMethodOne',
53+
params: ['p1'],
54+
},
55+
};
56+
expect(isCronjobSpecification(cronjobSpecification)).toBe(true);
57+
});
4558

46-
it('returns false for an invalid cronjob specification array', () => {
47-
const cronjobSpecificationArray = {
59+
it('returns true for a valid cronjob specification with a duration', () => {
60+
const cronjobSpecification = {
61+
duration: 'PT10S',
62+
request: {
63+
method: 'exampleMethodOne',
64+
params: ['p1'],
65+
},
66+
};
67+
expect(isCronjobSpecification(cronjobSpecification)).toBe(true);
68+
});
69+
70+
it('returns false for an invalid cronjob specification', () => {
71+
const cronjobSpecification = {
72+
expression: '* * * * * * * * * * *',
73+
request: {
74+
method: 'exampleMethodOne',
75+
params: ['p1'],
76+
},
77+
};
78+
expect(isCronjobSpecification(cronjobSpecification)).toBe(false);
79+
});
80+
});
81+
82+
describe('isCronjobSpecificationArray', () => {
83+
it('returns true for a valid cronjob specification array', () => {
84+
const cronjobSpecificationArray = [
85+
{
4886
expression: '* * * * *',
4987
request: {
5088
method: 'exampleMethodOne',
5189
params: ['p1'],
5290
},
53-
};
54-
expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe(
55-
false,
56-
);
57-
});
91+
},
92+
{
93+
duration: 'PT10S',
94+
request: {
95+
method: 'exampleMethodOne',
96+
params: ['p1'],
97+
},
98+
},
99+
];
100+
expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe(true);
58101
});
59102

60-
describe('parseCronExpression', () => {
61-
it('successfully parses cronjob expression that is provided as a string', () => {
62-
const cronjobExpression = '* * * * *';
63-
64-
const parsedExpression = parseCronExpression(cronjobExpression);
65-
expect(parsedExpression.next()).toBeDefined();
66-
expect(typeof parsedExpression.next().getTime()).toBe('number');
67-
});
103+
it('returns false for an invalid cronjob specification array', () => {
104+
const cronjobSpecificationArray = {
105+
expression: '* * * * *',
106+
request: {
107+
method: 'exampleMethodOne',
108+
params: ['p1'],
109+
},
110+
};
111+
expect(isCronjobSpecificationArray(cronjobSpecificationArray)).toBe(false);
68112
});
69113
});

packages/snaps-utils/src/cronjob.ts

Lines changed: 18 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { union } from '@metamask/snaps-sdk';
1+
import { selectiveUnion } from '@metamask/snaps-sdk';
22
import type { Infer } from '@metamask/superstruct';
33
import {
44
array,
@@ -9,6 +9,8 @@ import {
99
string,
1010
} from '@metamask/superstruct';
1111
import {
12+
hasProperty,
13+
isObject,
1214
JsonRpcIdStruct,
1315
JsonRpcParamsStruct,
1416
JsonRpcVersionStruct,
@@ -34,29 +36,31 @@ export const CronExpressionStruct = refine(
3436
parseExpression(value);
3537
return true;
3638
} catch {
37-
return false;
39+
return `Expected a cronjob expression, but received: "${value}"`;
3840
}
3941
},
4042
);
4143

4244
export type CronExpression = Infer<typeof CronExpressionStruct>;
4345

44-
/**
45-
* Parses a cron expression.
46-
*
47-
* @param expression - Expression to parse.
48-
* @returns A CronExpression class instance.
49-
*/
50-
export function parseCronExpression(expression: string | object) {
51-
const ensureStringExpression = create(expression, CronExpressionStruct);
52-
return parseExpression(ensureStringExpression);
53-
}
46+
const CronjobExpressionSpecificationStruct = object({
47+
expression: CronExpressionStruct,
48+
request: CronjobRpcRequestStruct,
49+
});
5450

55-
export const CronjobSpecificationStruct = object({
56-
expression: union([CronExpressionStruct, ISO8601DurationStruct]),
51+
const CronjobDurationSpecificationStruct = object({
52+
duration: ISO8601DurationStruct,
5753
request: CronjobRpcRequestStruct,
5854
});
5955

56+
export const CronjobSpecificationStruct = selectiveUnion((value) => {
57+
if (isObject(value) && hasProperty(value, 'duration')) {
58+
return CronjobDurationSpecificationStruct;
59+
}
60+
61+
return CronjobExpressionSpecificationStruct;
62+
});
63+
6064
export type CronjobSpecification = Infer<typeof CronjobSpecificationStruct>;
6165

6266
/**

0 commit comments

Comments
 (0)