From e648c19c2066e23cdfaed09a71e8437ea8d99c0b Mon Sep 17 00:00:00 2001 From: Giulio Montagner Date: Mon, 28 Jul 2025 14:58:05 +0200 Subject: [PATCH] chore: replace node-schedule with strapi.cron service --- .../services/__tests__/audit-logs.test.ts | 25 +++++----- .../src/audit-logs/services/lifecycles.ts | 17 ++++--- packages/core/admin/package.json | 1 - packages/core/content-manager/package.json | 1 - .../services/__tests__/lifecycles.test.ts | 23 ++++----- .../server/src/history/services/lifecycles.ts | 48 +++++++++--------- packages/core/content-releases/package.json | 1 - .../content-releases/server/src/destroy.ts | 7 ++- .../src/services/__tests__/scheduling.test.ts | 50 +++++++------------ .../server/src/services/scheduling.ts | 28 ++++++----- .../services/metrics/__tests__/index.test.ts | 14 ++++++ .../core/core/src/services/metrics/index.ts | 19 +++---- yarn.lock | 3 -- 13 files changed, 119 insertions(+), 118 deletions(-) diff --git a/packages/core/admin/ee/server/src/audit-logs/services/__tests__/audit-logs.test.ts b/packages/core/admin/ee/server/src/audit-logs/services/__tests__/audit-logs.test.ts index 707ad35ce8..4a36a7233a 100644 --- a/packages/core/admin/ee/server/src/audit-logs/services/__tests__/audit-logs.test.ts +++ b/packages/core/admin/ee/server/src/audit-logs/services/__tests__/audit-logs.test.ts @@ -1,12 +1,6 @@ import { createAuditLogsLifecycleService } from '../lifecycles'; -import { scheduleJob } from 'node-schedule'; - import '@strapi/types'; -jest.mock('node-schedule', () => ({ - scheduleJob: jest.fn(), -})); - describe('Audit logs service', () => { const mockSubscribe = jest.fn(); @@ -37,6 +31,10 @@ describe('Audit logs service', () => { get: jest.fn(() => ({ deleteExpiredEvents: jest.fn(), })), + cron: { + add: jest.fn(), + remove: jest.fn(), + }, config: { get(key: any) { switch (key) { @@ -103,15 +101,18 @@ describe('Audit logs service', () => { }); it('should create a cron job that executed one time a day', async () => { - // @ts-expect-error scheduleJob - const mockScheduleJob = scheduleJob.mockImplementationOnce( - jest.fn((rule, callback) => callback()) - ); + // Mock Strapi EE feature to be enabled for this test + jest.mocked(strapi.ee.features.isEnabled).mockReturnValueOnce(true); const lifecycle = createAuditLogsLifecycleService(strapi); await lifecycle.register(); - expect(mockScheduleJob).toHaveBeenCalledTimes(1); - expect(mockScheduleJob).toHaveBeenCalledWith('0 0 * * *', expect.any(Function)); + // Verify that strapi.cron.add was called with the correct job configuration + expect(strapi.cron.add).toHaveBeenCalledWith({ + deleteExpiredAuditLogs: { + task: expect.any(Function), + options: '0 0 * * *', + }, + }); }); }); diff --git a/packages/core/admin/ee/server/src/audit-logs/services/lifecycles.ts b/packages/core/admin/ee/server/src/audit-logs/services/lifecycles.ts index de4236a62e..5ccdfa42d5 100644 --- a/packages/core/admin/ee/server/src/audit-logs/services/lifecycles.ts +++ b/packages/core/admin/ee/server/src/audit-logs/services/lifecycles.ts @@ -1,5 +1,4 @@ import type { Core } from '@strapi/types'; -import { scheduleJob } from 'node-schedule'; const DEFAULT_RETENTION_DAYS = 90; @@ -156,9 +155,15 @@ const createAuditLogsLifecycleService = (strapi: Core.Strapi) => { // Manage audit logs auto deletion const retentionDays = getRetentionDays(strapi); - state.deleteExpiredJob = scheduleJob('0 0 * * *', () => { - const expirationDate = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000); - auditLogsService.deleteExpiredEvents(expirationDate); + + strapi.cron.add({ + deleteExpiredAuditLogs: { + task: async () => { + const expirationDate = new Date(Date.now() - retentionDays * 24 * 60 * 60 * 1000); + auditLogsService.deleteExpiredEvents(expirationDate); + }, + options: '0 0 * * *', + }, }); return this; @@ -173,9 +178,7 @@ const createAuditLogsLifecycleService = (strapi: Core.Strapi) => { state.eventHubUnsubscribe(); } - if (state.deleteExpiredJob) { - state.deleteExpiredJob.cancel(); - } + strapi.cron.remove('deleteExpiredAuditLogs'); return this; }, diff --git a/packages/core/admin/package.json b/packages/core/admin/package.json index 8f9ce7c704..34956f43b7 100644 --- a/packages/core/admin/package.json +++ b/packages/core/admin/package.json @@ -118,7 +118,6 @@ "koa-static": "5.0.0", "koa2-ratelimit": "^1.1.3", "lodash": "4.17.21", - "node-schedule": "2.1.1", "ora": "5.4.1", "p-map": "4.0.0", "passport-local": "1.0.0", diff --git a/packages/core/content-manager/package.json b/packages/core/content-manager/package.json index a876b631d2..9758276ada 100644 --- a/packages/core/content-manager/package.json +++ b/packages/core/content-manager/package.json @@ -88,7 +88,6 @@ "markdown-it-mark": "^3.0.1", "markdown-it-sub": "^1.0.0", "markdown-it-sup": "1.0.0", - "node-schedule": "2.1.1", "prismjs": "1.30.0", "qs": "6.11.1", "react-dnd": "16.0.1", diff --git a/packages/core/content-manager/server/src/history/services/__tests__/lifecycles.test.ts b/packages/core/content-manager/server/src/history/services/__tests__/lifecycles.test.ts index ab8058a7c0..dd5002f76b 100644 --- a/packages/core/content-manager/server/src/history/services/__tests__/lifecycles.test.ts +++ b/packages/core/content-manager/server/src/history/services/__tests__/lifecycles.test.ts @@ -1,12 +1,7 @@ import type { UID } from '@strapi/types'; -import { scheduleJob } from 'node-schedule'; import { HISTORY_VERSION_UID } from '../../constants'; import { createLifecyclesService } from '../lifecycles'; -jest.mock('node-schedule', () => ({ - scheduleJob: jest.fn(), -})); - const mockGetRequestContext = jest.fn(() => { return { state: { @@ -73,6 +68,9 @@ const mockStrapi = { config: { get: () => undefined, }, + cron: { + add: jest.fn(), + }, }; // @ts-expect-error - ignore mockStrapi.documents.use = jest.fn(); @@ -93,14 +91,15 @@ describe('history lifecycles service', () => { }); it('should create a cron job that runs once a day', async () => { - // @ts-expect-error - this is a mock - const mockScheduleJob = scheduleJob.mockImplementationOnce( - jest.fn((rule, callback) => callback()) - ); - await lifecyclesService.bootstrap(); - expect(mockScheduleJob).toHaveBeenCalledTimes(1); - expect(mockScheduleJob).toHaveBeenCalledWith('historyDaily', '0 0 * * *', expect.any(Function)); + expect(mockStrapi.cron.add).toHaveBeenCalledTimes(1); + expect(mockStrapi.cron.add).toHaveBeenCalledWith( + expect.objectContaining({ + deleteHistoryDaily: expect.objectContaining({ + task: expect.any(Function), + }), + }) + ); }); }); diff --git a/packages/core/content-manager/server/src/history/services/lifecycles.ts b/packages/core/content-manager/server/src/history/services/lifecycles.ts index ffa54d4d35..9f363735d0 100644 --- a/packages/core/content-manager/server/src/history/services/lifecycles.ts +++ b/packages/core/content-manager/server/src/history/services/lifecycles.ts @@ -3,8 +3,6 @@ import { contentTypes } from '@strapi/utils'; import { omit, castArray } from 'lodash/fp'; -import { scheduleJob } from 'node-schedule'; - import { getService } from '../utils'; import { FIELDS_TO_IGNORE, HISTORY_VERSION_UID } from '../constants'; @@ -93,10 +91,8 @@ const getSchemas = (uid: UID.CollectionType) => { const createLifecyclesService = ({ strapi }: { strapi: Core.Strapi }) => { const state: { - deleteExpiredJob: ReturnType | null; isInitialized: boolean; } = { - deleteExpiredJob: null, isInitialized: false, }; @@ -176,33 +172,37 @@ const createLifecyclesService = ({ strapi }: { strapi: Core.Strapi }) => { }); // Schedule a job to delete expired history versions every day at midnight - state.deleteExpiredJob = scheduleJob('historyDaily', '0 0 * * *', () => { - const retentionDaysInMilliseconds = serviceUtils.getRetentionDays() * 24 * 60 * 60 * 1000; - const expirationDate = new Date(Date.now() - retentionDaysInMilliseconds); + strapi.cron.add({ + deleteHistoryDaily: { + async task() { + const retentionDaysInMilliseconds = + serviceUtils.getRetentionDays() * 24 * 60 * 60 * 1000; + const expirationDate = new Date(Date.now() - retentionDaysInMilliseconds); - strapi.db - .query(HISTORY_VERSION_UID) - .deleteMany({ - where: { - created_at: { - $lt: expirationDate, - }, - }, - }) - .catch((error) => { - if (error instanceof Error) { - strapi.log.error('Error deleting expired history versions', error.message); - } - }); + strapi.db + .query(HISTORY_VERSION_UID) + .deleteMany({ + where: { + created_at: { + $lt: expirationDate, + }, + }, + }) + .catch((error) => { + if (error instanceof Error) { + strapi.log.error('Error deleting expired history versions', error.message); + } + }); + }, + options: '0 0 * * *', + }, }); state.isInitialized = true; }, async destroy() { - if (state.deleteExpiredJob) { - state.deleteExpiredJob.cancel(); - } + strapi.cron.remove('deleteHistoryDaily'); }, }; }; diff --git a/packages/core/content-releases/package.json b/packages/core/content-releases/package.json index 2f04ab820b..1852dfb4e3 100644 --- a/packages/core/content-releases/package.json +++ b/packages/core/content-releases/package.json @@ -68,7 +68,6 @@ "date-fns-tz": "2.0.1", "formik": "2.4.5", "lodash": "4.17.21", - "node-schedule": "2.1.1", "qs": "6.11.1", "react-intl": "6.6.2", "react-redux": "8.1.3", diff --git a/packages/core/content-releases/server/src/destroy.ts b/packages/core/content-releases/server/src/destroy.ts index 431e3e150f..745bfb979d 100644 --- a/packages/core/content-releases/server/src/destroy.ts +++ b/packages/core/content-releases/server/src/destroy.ts @@ -1,15 +1,14 @@ -import { Job } from 'node-schedule'; import type { Core } from '@strapi/types'; import { Release } from '../../shared/contracts/releases'; import { getService } from './utils'; export const destroy = async ({ strapi }: { strapi: Core.Strapi }) => { - const scheduledJobs: Map = getService('scheduling', { + const scheduledJobs: Map = getService('scheduling', { strapi, }).getAll(); - for (const [, job] of scheduledJobs) { - job.cancel(); + for (const [, taskName] of scheduledJobs) { + strapi.cron.remove(taskName); } }; diff --git a/packages/core/content-releases/server/src/services/__tests__/scheduling.test.ts b/packages/core/content-releases/server/src/services/__tests__/scheduling.test.ts index 050bc1fe17..0a89a095a9 100644 --- a/packages/core/content-releases/server/src/services/__tests__/scheduling.test.ts +++ b/packages/core/content-releases/server/src/services/__tests__/scheduling.test.ts @@ -1,4 +1,3 @@ -import { scheduleJob } from 'node-schedule'; import createSchedulingService from '../scheduling'; const baseStrapiMock = { @@ -7,13 +6,17 @@ const baseStrapiMock = { isEnabled: jest.fn().mockReturnValue(true), }, }, + cron: { + add: jest.fn(), + remove: jest.fn(), + }, }; -jest.mock('node-schedule', () => ({ - scheduleJob: jest.fn(), -})); - describe('Scheduling service', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + describe('set', () => { it('should throw an error if the release does not exist', async () => { const strapiMock = { @@ -33,10 +36,6 @@ describe('Scheduling service', () => { }); it('should cancel the previous job if it exists and create the new one', async () => { - const mockScheduleJob = jest.fn().mockReturnValue({ cancel: jest.fn() }); - // @ts-expect-error - scheduleJob is a mock - scheduleJob.mockImplementation(mockScheduleJob); - const strapiMock = { ...baseStrapiMock, db: { @@ -53,21 +52,17 @@ describe('Scheduling service', () => { const schedulingService = createSchedulingService({ strapi: strapiMock }); const scheduledJobs = await schedulingService.set('1', oldJobDate); expect(scheduledJobs.size).toBe(1); - expect(mockScheduleJob).toHaveBeenCalledWith(oldJobDate, expect.any(Function)); + expect(strapiMock.cron.add).toHaveBeenCalledTimes(1); - const oldJob = scheduledJobs.get('1')!; + const oldTaskName = scheduledJobs.get('1')!; await schedulingService.set('1', newJobDate); - expect(oldJob.cancel).toHaveBeenCalled(); - expect(mockScheduleJob).toHaveBeenCalledWith(newJobDate, expect.any(Function)); + expect(strapiMock.cron.remove).toHaveBeenCalledWith(oldTaskName); + expect(strapiMock.cron.add).toHaveBeenCalledTimes(2); }); it('should create a new job', async () => { - const mockScheduleJob = jest.fn().mockReturnValue({ cancel: jest.fn() }); - // @ts-expect-error - scheduleJob is a mock - scheduleJob.mockImplementation(mockScheduleJob); - const strapiMock = { ...baseStrapiMock, db: { @@ -83,16 +78,12 @@ describe('Scheduling service', () => { const schedulingService = createSchedulingService({ strapi: strapiMock }); const scheduledJobs = await schedulingService.set('1', date); expect(scheduledJobs.size).toBe(1); - expect(mockScheduleJob).toHaveBeenCalledWith(date, expect.any(Function)); + expect(strapiMock.cron.add).toHaveBeenCalledTimes(1); }); }); describe('cancel', () => { it('should cancel the job if it exists', async () => { - const mockScheduleJob = jest.fn().mockReturnValue({ cancel: jest.fn() }); - // @ts-expect-error - scheduleJob is a mock - scheduleJob.mockImplementation(mockScheduleJob); - const strapiMock = { ...baseStrapiMock, db: { @@ -108,19 +99,18 @@ describe('Scheduling service', () => { const schedulingService = createSchedulingService({ strapi: strapiMock }); const scheduledJobs = await schedulingService.set('1', date); expect(scheduledJobs.size).toBe(1); - expect(mockScheduleJob).toHaveBeenCalledWith(date, expect.any(Function)); + expect(strapiMock.cron.add).toHaveBeenCalledTimes(1); + const taskName = scheduledJobs.get('1')!; schedulingService.cancel('1'); + + expect(strapiMock.cron.remove).toHaveBeenCalledWith(taskName); expect(scheduledJobs.size).toBe(0); }); }); describe('getAll', () => { it('should return all the scheduled jobs', async () => { - const mockScheduleJob = jest.fn().mockReturnValue({ cancel: jest.fn() }); - // @ts-expect-error - scheduleJob is a mock - scheduleJob.mockImplementation(mockScheduleJob); - const strapiMock = { ...baseStrapiMock, db: { @@ -141,10 +131,6 @@ describe('Scheduling service', () => { describe('syncFromDatabase', () => { it('should sync the scheduled jobs from the database', async () => { - const mockScheduleJob = jest.fn().mockReturnValue({ cancel: jest.fn() }); - // @ts-expect-error - scheduleJob is a mock - scheduleJob.mockImplementation(mockScheduleJob); - const strapiMock = { ...baseStrapiMock, db: { @@ -161,7 +147,7 @@ describe('Scheduling service', () => { const schedulingService = createSchedulingService({ strapi: strapiMock }); const scheduledJobs = await schedulingService.syncFromDatabase(); expect(scheduledJobs.size).toBe(1); - expect(mockScheduleJob).toHaveBeenCalledWith(expect.any(Date), expect.any(Function)); + expect(strapiMock.cron.add).toHaveBeenCalledTimes(1); }); }); }); diff --git a/packages/core/content-releases/server/src/services/scheduling.ts b/packages/core/content-releases/server/src/services/scheduling.ts index baeacfd908..849a96f8ac 100644 --- a/packages/core/content-releases/server/src/services/scheduling.ts +++ b/packages/core/content-releases/server/src/services/scheduling.ts @@ -1,4 +1,3 @@ -import { scheduleJob, Job } from 'node-schedule'; import type { Core } from '@strapi/types'; import { errors } from '@strapi/utils'; @@ -7,7 +6,7 @@ import { getService } from '../utils'; import { RELEASE_MODEL_UID } from '../constants'; const createSchedulingService = ({ strapi }: { strapi: Core.Strapi }) => { - const scheduledJobs = new Map(); + const scheduledJobs = new Map(); return { async set(releaseId: Release['id'], scheduleDate: Date) { @@ -19,29 +18,34 @@ const createSchedulingService = ({ strapi }: { strapi: Core.Strapi }) => { throw new errors.NotFoundError(`No release found for id ${releaseId}`); } - const job = scheduleJob(scheduleDate, async () => { - try { - await getService('release', { strapi }).publish(releaseId); - // @TODO: Trigger webhook with success message - } catch (error) { - // @TODO: Trigger webhook with error message - } + const taskName = `publishRelease_${releaseId}`; - this.cancel(releaseId); + strapi.cron.add({ + [taskName]: { + async task() { + try { + await getService('release', { strapi }).publish(releaseId); + // @TODO: Trigger webhook with success message + } catch (error) { + // @TODO: Trigger webhook with error message + } + }, + options: scheduleDate, + }, }); if (scheduledJobs.has(releaseId)) { this.cancel(releaseId); } - scheduledJobs.set(releaseId, job); + scheduledJobs.set(releaseId, taskName); return scheduledJobs; }, cancel(releaseId: Release['id']) { if (scheduledJobs.has(releaseId)) { - scheduledJobs.get(releaseId)!.cancel(); + strapi.cron.remove(scheduledJobs.get(releaseId)!); scheduledJobs.delete(releaseId); } diff --git a/packages/core/core/src/services/metrics/__tests__/index.test.ts b/packages/core/core/src/services/metrics/__tests__/index.test.ts index 30781e101d..051ecfa8bb 100644 --- a/packages/core/core/src/services/metrics/__tests__/index.test.ts +++ b/packages/core/core/src/services/metrics/__tests__/index.test.ts @@ -9,6 +9,7 @@ describe('metrics', () => { }); test('Initializes a middleware', () => { const use = jest.fn(); + const add = jest.fn(); const metricsInstance = metrics({ config: { @@ -32,6 +33,9 @@ describe('metrics', () => { requestContext: { get: jest.fn(() => ({})), }, + cron: { + add, + }, fetch, } as any); @@ -44,6 +48,7 @@ describe('metrics', () => { test('Does not init middleware if disabled', () => { const use = jest.fn(); + const add = jest.fn(); const metricsInstance = metrics({ config: { @@ -67,6 +72,9 @@ describe('metrics', () => { requestContext: { get: jest.fn(() => ({})), }, + cron: { + add, + }, fetch, } as any); @@ -100,6 +108,9 @@ describe('metrics', () => { requestContext: { get: jest.fn(() => ({})), }, + cron: { + add: jest.fn(), + }, fetch, } as any); @@ -147,6 +158,9 @@ describe('metrics', () => { root: process.cwd(), }, }, + cron: { + add: jest.fn(), + }, requestContext: { get: jest.fn(() => ({})), }, diff --git a/packages/core/core/src/services/metrics/index.ts b/packages/core/core/src/services/metrics/index.ts index a2ce68c51f..a2f9cbe400 100644 --- a/packages/core/core/src/services/metrics/index.ts +++ b/packages/core/core/src/services/metrics/index.ts @@ -3,7 +3,6 @@ * You can learn more at https://docs.strapi.io/developer-docs/latest/getting-started/usage-information.html */ -import { Job, scheduleJob } from 'node-schedule'; import type { Core } from '@strapi/types'; import wrapWithRateLimit from './rate-limiter'; @@ -25,7 +24,6 @@ const createTelemetryInstance = (strapi: Core.Strapi) => { const isDisabled = !uuid || isTruthy(process.env.STRAPI_TELEMETRY_DISABLED) || isTruthy(telemetryDisabled); - const crons: Job[] = []; const sender = createSender(strapi); const sendEvent = wrapWithRateLimit(sender, { limitedEvents: LIMITED_EVENTS }); @@ -36,8 +34,12 @@ const createTelemetryInstance = (strapi: Core.Strapi) => { register() { if (!isDisabled) { - const pingCron = scheduleJob('0 0 12 * * *', () => sendEvent('ping')); - crons.push(pingCron); + strapi.cron.add({ + sendPingEvent: { + task: () => sendEvent('ping'), + options: '0 0 12 * * *', + }, + }); strapi.server.use(createMiddleware({ sendEvent })); } @@ -45,15 +47,14 @@ const createTelemetryInstance = (strapi: Core.Strapi) => { bootstrap() {}, - destroy() { - // Clear open handles - crons.forEach((cron) => cron.cancel()); - }, - async send(event: string, payload: Record = {}) { if (isDisabled) return true; return sendEvent(event, payload); }, + + destroy() { + // Clean up resources if needed + }, }; }; diff --git a/yarn.lock b/yarn.lock index 025b9f09c2..a6853db22f 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8692,7 +8692,6 @@ __metadata: koa2-ratelimit: "npm:^1.1.3" lodash: "npm:4.17.21" msw: "npm:1.3.0" - node-schedule: "npm:2.1.1" ora: "npm:5.4.1" p-map: "npm:4.0.0" passport-local: "npm:1.0.0" @@ -8804,7 +8803,6 @@ __metadata: markdown-it-sub: "npm:^1.0.0" markdown-it-sup: "npm:1.0.0" msw: "npm:1.3.0" - node-schedule: "npm:2.1.1" prismjs: "npm:1.30.0" qs: "npm:6.11.1" react: "npm:18.3.1" @@ -8855,7 +8853,6 @@ __metadata: koa: "npm:2.16.1" lodash: "npm:4.17.21" msw: "npm:1.3.0" - node-schedule: "npm:2.1.1" qs: "npm:6.11.1" react: "npm:18.3.1" react-dom: "npm:18.3.1"