diff --git a/docs/codebase/jobs.md b/docs/codebase/jobs.md index 205417e5808..e1d82afe87c 100644 --- a/docs/codebase/jobs.md +++ b/docs/codebase/jobs.md @@ -1,12 +1,16 @@ # Jobs System -Ghost's jobs system runs work inline or in a worker thread. Jobs can run once or -on a schedule. +Ghost's jobs system runs work inline, in a worker thread, or in-process through +the class-based jobs service in +`ghost/core/core/server/services/jobs-service/`. Jobs can run once or on a +schedule. Use inline jobs for short work which does not block the event loop. Inline jobs -cannot be scheduled. Scheduled and offloaded jobs run in worker threads, so -they must initialize their own dependencies and cannot rely on the main Ghost -process's memory. +cannot be scheduled. Scheduled and offloaded jobs registered through the legacy +Bree-based service run in worker threads, so they must initialize their own +dependencies and cannot rely on the main Ghost process's memory. Jobs migrated +to the class-based service (token cleanup, gift cleanup, update checks) run +in-process and share the main process's initialized services. ## Adding a job @@ -17,7 +21,7 @@ events. Existing examples include: -- Update checks, which run in a worker on a schedule. +- Gift reminders, which run in a worker on a schedule. - Imports, which run as inline jobs. - Email analytics, which uses scheduled worker jobs. @@ -31,13 +35,14 @@ first request. ## Testing -Tests for the jobs wrapper live in -`ghost/core/test/unit/server/services/jobs/`, with update-check integration -coverage in `ghost/core/test/integration/jobs/`. Tests should cover the job's -result and failure behavior. +Tests for the legacy jobs wrapper live in +`ghost/core/test/unit/server/services/jobs/`, and tests for the class-based +service in `ghost/core/test/unit/server/services/jobs-service/`. Tests should +cover the job's result and failure behavior. ## Scheduling -The jobs system uses Bree for scheduled work. Schedules use the server's system -timezone. Offloaded jobs should have unique names, be safe to run more than -once, and receive identifiers rather than large objects where possible. +The legacy jobs service uses Bree for scheduled work; the class-based service +schedules with cron expressions through its backend. Schedules use the server's +system timezone. Jobs should have unique names, be safe to run more than once, +and receive identifiers rather than large objects where possible. diff --git a/ghost/core/core/boot.js b/ghost/core/core/boot.js index fe0a7b12935..8e184506ba6 100644 --- a/ghost/core/core/boot.js +++ b/ghost/core/core/boot.js @@ -483,12 +483,14 @@ async function initBackgroundServices({ config }) { const giftService = require('./server/services/gifts'); giftService.recoverPendingDeliveries(); + const jobsService = require('./server/services/jobs-service').getInstance(); + // Runs before activitypub.init for the same reason as the send recovery // above: gifts would otherwise go uncleaned for the life of the process // if an unrelated background service fails. try { const giftJobs = require('./server/services/gifts/jobs'); - await giftJobs.scheduleGiftCleanupJob(require('./server/services/jobs-service').getInstance()); + await giftJobs.scheduleGiftCleanupJob(jobsService); } catch (err) { const logging = require('@tryghost/logging'); logging.error(err); @@ -526,10 +528,12 @@ async function initBackgroundServices({ config }) { ]); } - const updateCheck = require('./server/services/update-check'); - updateCheck.scheduleRecurringJobs(); - if (config.get('updateCheck:forceUpdate')) { - updateCheck.scheduleBootJob(); + try { + const updateCheck = require('./server/services/update-check'); + await updateCheck.scheduleJobs(jobsService); + } catch (err) { + const logging = require('@tryghost/logging'); + logging.error(err); } // Remote feature-flag overrides (config-gated; inert unless explicitly configured). diff --git a/ghost/core/core/server/services/jobs-service/register-job-handlers.ts b/ghost/core/core/server/services/jobs-service/register-job-handlers.ts index 166ca2bce9a..eab6405e1c3 100644 --- a/ghost/core/core/server/services/jobs-service/register-job-handlers.ts +++ b/ghost/core/core/server/services/jobs-service/register-job-handlers.ts @@ -10,6 +10,9 @@ import ExternalMediaInliner from '../media-inliner/external-media-inliner'; import ExternalMediaInlinerJob from '../media-inliner/external-media-inliner-job'; import ContentCSVImportJob from '../content-import/jobs/content-csv-import-job'; import * as contentImport from '../content-import'; +import UpdateCheckJob from '../update-check/jobs/update-check-job'; + +const updateCheck = require('../update-check'); interface RegisterJobHandlersDependencies { jobsService: JobsService; @@ -63,4 +66,8 @@ export default function registerJobHandlers({ jobsService.handle(ContentCSVImportJob, async (job) => { await contentImport.handleJob(job); }); + + jobsService.handle(UpdateCheckJob, async () => { + await updateCheck({ rethrowErrors: true }); + }); } diff --git a/ghost/core/core/server/services/update-check/index.js b/ghost/core/core/server/services/update-check/index.js index 41e24a29180..712703fc70e 100644 --- a/ghost/core/core/server/services/update-check/index.js +++ b/ghost/core/core/server/services/update-check/index.js @@ -2,7 +2,7 @@ const api = require('../../api').endpoints; const config = require('../../../shared/config'); const logging = require('@tryghost/logging'); const urlUtils = require('../../../shared/url-utils').default; -const jobsService = require('../jobs'); +const UpdateCheckJob = require('./jobs/update-check-job').default; const request = require('@tryghost/request'); const ghostVersion = require('@tryghost/version'); @@ -66,7 +66,7 @@ module.exports = async ({ await updateChecker.check(); }; -module.exports.scheduleRecurringJobs = () => { +const scheduleRecurringJob = async (jobsService) => { // use a random seconds/minutes/hours value to avoid spikes to the update service API const s = Math.floor(Math.random() * 60); // 0-59 const m = Math.floor(Math.random() * 60); // 0-59 @@ -74,17 +74,17 @@ module.exports.scheduleRecurringJobs = () => { const at = `${s} ${m} ${h} * * *`; logging.info(`[Background Job] update-check scheduled at ${at}`); - jobsService.addJob({ - at, // Every day - job: require('path').resolve(__dirname, 'run-update-check.js'), - name: 'update-check', - }); + await jobsService.scheduleRecurring(new UpdateCheckJob(), { cron: at }); }; -module.exports.scheduleBootJob = () => { - logging.info('[Background Job] update-check-boot queued'); - jobsService.addJob({ - job: require('path').resolve(__dirname, 'run-update-check.js'), - name: 'update-check-boot', - }); +const scheduleBootJob = async (jobsService) => { + logging.info('[Background Job] update-check boot run queued'); + await jobsService.dispatch(new UpdateCheckJob()); +}; + +module.exports.scheduleJobs = async (jobsService) => { + await scheduleRecurringJob(jobsService); + if (config.get('updateCheck:forceUpdate')) { + await scheduleBootJob(jobsService); + } }; diff --git a/ghost/core/core/server/services/update-check/jobs/update-check-job.ts b/ghost/core/core/server/services/update-check/jobs/update-check-job.ts new file mode 100644 index 00000000000..44cbf042e34 --- /dev/null +++ b/ghost/core/core/server/services/update-check/jobs/update-check-job.ts @@ -0,0 +1,5 @@ +import { Job } from '../../jobs-service/job'; + +export default class UpdateCheckJob extends Job { + static type = 'update-check'; +} diff --git a/ghost/core/core/server/services/update-check/run-update-check.js b/ghost/core/core/server/services/update-check/run-update-check.js deleted file mode 100644 index 64ac8d9a436..00000000000 --- a/ghost/core/core/server/services/update-check/run-update-check.js +++ /dev/null @@ -1,63 +0,0 @@ -const { parentPort, workerData } = require('worker_threads'); - -const postParentPortMessage = (message) => { - if (parentPort) { - parentPort.postMessage(message); - } -}; - -// Exit early when cancelled to prevent stalling shutdown. No cleanup needed when cancelling as everything is idempotent and will pick up -// where it left off on next run -function cancel() { - postParentPortMessage('cancelled before completion'); - - if (parentPort) { - postParentPortMessage('cancelled'); - } else { - setTimeout(() => { - process.exit(0); - }, 1000); - } -} - -if (parentPort) { - parentPort.once('message', (message) => { - if (message === 'cancel') { - return cancel(); - } - }); -} - -(async () => { - const startedAt = Date.now(); - postParentPortMessage('execution started'); - const updateCheck = require('./'); - - // INIT required services - const permissions = require('../permissions'); - await permissions.init(); - - const settings = require('../settings/settings-service'); - await settings.init(); - - const emailAddress = require('../email-address'); - emailAddress.init(); - // Finished INIT - - await updateCheck({ - rethrowErrors: true, - forceUpdate: workerData.forceUpdate, - updateCheckUrl: workerData.updateCheckUrl, - }); - - postParentPortMessage(`completed in ${Date.now() - startedAt}ms`); - - if (parentPort) { - postParentPortMessage('done'); - } else { - // give the logging pipes time finish writing before exit - setTimeout(() => { - process.exit(0); - }, 1000); - } -})(); diff --git a/ghost/core/test/integration/jobs/update-check.test.js b/ghost/core/test/integration/jobs/update-check.test.js deleted file mode 100644 index cfa6f1ee87a..00000000000 --- a/ghost/core/test/integration/jobs/update-check.test.js +++ /dev/null @@ -1,142 +0,0 @@ -const assert = require('node:assert/strict'); -const http = require('http'); -const path = require('path'); -const testUtils = require('../../utils'); -const jobService = require('../../../core/server/services/jobs/job-service'); -const models = require('../../../core/server/models'); - -const JOB_NAME = 'update-check'; -const JOB_PATH = path.resolve( - __dirname, - '../../../core/server/services/update-check/run-update-check.js', -); - -describe('Run Update Check', function () { - let mockUpdateServer; - let baselineMailTransport; - - beforeAll(testUtils.setup('default')); - - beforeEach(function () { - baselineMailTransport = process.env.mail__transport; - }); - - afterEach(async function () { - if (mockUpdateServer) { - mockUpdateServer.close(); - } - // Reset notifications between tests so each starts clean - await models.Settings.edit( - { key: 'notifications', value: '[]' }, - { context: { internal: true } }, - ); - // Remove the job so the next test can re-register it - await jobService.removeJob(JOB_NAME).catch(() => {}); - if (baselineMailTransport === undefined) { - delete process.env.mail__transport; - } else { - process.env.mail__transport = baselineMailTransport; - } - }); - - it('successfully executes the update checker', async function () { - let mockUpdateServerRequestCount = 0; - - // Initialise mock update server - We use a mock server here instead of - // nock because the update-check job will be executed in a separate - // process which will prevent nock from intercepting HTTP requests - mockUpdateServer = http.createServer((req, res) => { - mockUpdateServerRequestCount += 1; - - res.writeHead(200, { 'Content-Type': 'application/json' }); - - res.end(JSON.stringify({ hello: 'world' })); - }); - - mockUpdateServer.listen(0); // Listen on random port - - const mockUpdateServerPort = mockUpdateServer.address().port; - - // Trigger the update-check job and wait for it to finish - await jobService.addJob({ - name: JOB_NAME, - job: JOB_PATH, - data: { - forceUpdate: true, - updateCheckUrl: `http://127.0.0.1:${mockUpdateServerPort}`, - }, - }); - - await jobService.awaitCompletion(JOB_NAME); - - // Assert that the mock update server received a request (which means the update-check job ran successfully) - assert.equal(mockUpdateServerRequestCount, 1, 'Expected mock server to receive 1 request'); - }); - - it('stores an alert-type custom notification end-to-end', async function () { - // Default fixtures leave the Owner inactive, so the alert branch's - // users.browse returns no recipients and the email send returns - // early without exercising the mailer pipeline. Activate the Owner - // so the worker actually drives the full notificationEmailService - // path that production hits. - const owner = await models.User.findOne( - { email: 'ghost@example.com' }, - { context: { internal: true }, withRelated: ['roles'] }, - ); - await owner.save({ status: 'active' }, { patch: true, context: { internal: true } }); - - // Worker threads inherit process.env, so this routes the worker's - // GhostMailer to nodemailer-stub-transport. Without it the worker - // hits the default SMTP transport and ECONNREFUSEs on localhost:587. - process.env.mail__transport = 'stub'; - - mockUpdateServer = http.createServer((req, res) => { - res.writeHead(200, { 'Content-Type': 'application/json' }); - res.end( - JSON.stringify({ - id: 99999, - version: 'all-test', - messages: [ - { - id: 'integration-test-alert-msg', - version: '^6', - content: '
Integration test alert
', - top: true, - dismissible: false, - type: 'alert', - }, - ], - created_at: '2026-06-08T00:00:00.000Z', - custom: true, - next_check: Math.floor(Date.now() / 1000) + 86400, - }), - ); - }); - - mockUpdateServer.listen(0); - const port = mockUpdateServer.address().port; - - await jobService.addJob({ - name: JOB_NAME, - job: JOB_PATH, - data: { - forceUpdate: true, - updateCheckUrl: `http://127.0.0.1:${port}`, - }, - }); - - await jobService.awaitCompletion(JOB_NAME); - - const setting = await models.Settings.findOne( - { key: 'notifications' }, - { context: { internal: true } }, - ); - const stored = JSON.parse(setting.get('value')); - const ourNotification = stored.find((n) => n.id === 'integration-test-alert-msg'); - - assert.ok(ourNotification, 'Expected the alert notification to be stored in settings'); - assert.equal(ourNotification.type, 'alert'); - assert.equal(ourNotification.message, 'Integration test alert
'); - assert.equal(ourNotification.custom, true); - }); -}); diff --git a/ghost/core/test/integration/jobs/update-check.test.ts b/ghost/core/test/integration/jobs/update-check.test.ts new file mode 100644 index 00000000000..c16eafd3510 --- /dev/null +++ b/ghost/core/test/integration/jobs/update-check.test.ts @@ -0,0 +1,151 @@ +import assert from 'node:assert/strict'; +import http from 'node:http'; +import sinon from 'sinon'; +import type { AddressInfo } from 'node:net'; +import UpdateCheckJob from '../../../core/server/services/update-check/jobs/update-check-job'; + +const logging = require('@tryghost/logging'); +const models = require('../../../core/server/models'); +const { agentProvider, configUtils } = require('../../utils/e2e-framework'); +const { getInstance: getJobsService } = require('../../../core/server/services/jobs-service'); + +async function waitFor( + check: () => PromiseIntegration test alert
', + top: true, + dismissible: false, + type: 'alert', + }, + ], + created_at: '2026-06-08T00:00:00.000Z', + custom: true, + next_check: nextCheckTimestamp, + }), + ); + }); + mockUpdateServer.listen(0); + const { port } = mockUpdateServer.address() as AddressInfo; + + configUtils.set('updateCheck:forceUpdate', true); + configUtils.set('updateCheck:url', `http://127.0.0.1:${port}`); + // The job runs in-process now, so the mailer honours runtime config: + // route it to the stub transport instead of a real SMTP connection. + configUtils.set('mail:transport', 'stub'); + + const loggingInfoSpy = sinon.spy(logging, 'info'); + + await getJobsService().dispatch(new UpdateCheckJob()); + + const completed = await waitFor(() => jobCompleted(loggingInfoSpy, 'update-check')); + assert.ok(completed, 'the dispatched job completes under the update-check type'); + + const setting = await models.Settings.findOne( + { key: 'notifications' }, + { context: { internal: true } }, + ); + const stored = JSON.parse(setting.get('value')); + const ourNotification = stored.find( + (n: { id: string }) => n.id === 'integration-test-alert-msg', + ); + + assert.ok(ourNotification, 'Expected the alert notification to be stored in settings'); + assert.equal(ourNotification.type, 'alert'); + assert.equal(ourNotification.message, 'Integration test alert
'); + assert.equal(ourNotification.custom, true); + + const nextCheckSetting = await models.Settings.findOne( + { key: 'next_update_check' }, + { context: { internal: true } }, + ); + assert.equal( + Number(nextCheckSetting.get('value')), + nextCheckTimestamp, + 'a successful check advances next_update_check to the endpoint-provided time', + ); + }); +}); diff --git a/ghost/core/test/unit/server/services/jobs-service/index.test.js b/ghost/core/test/unit/server/services/jobs-service/index.test.js index 04a5cf2ddc9..a18170c6f28 100644 --- a/ghost/core/test/unit/server/services/jobs-service/index.test.js +++ b/ghost/core/test/unit/server/services/jobs-service/index.test.js @@ -1,16 +1,30 @@ const assert = require('node:assert/strict'); const sinon = require('sinon'); -const jobsService = require('../../../../../core/server/services/jobs-service'); -const adapterManager = require('../../../../../core/server/services/adapter-manager').default; +const JOBS_SERVICE_PATH = '../../../../../core/server/services/jobs-service'; + +let jobsService; +let adapterManager; describe('jobs-service wrapper', function () { + // The wrapper holds its instance in module state, and the unit project shares + // modules across files, so reload it per test. vi.resetModules() only clears + // the Vite module graph, not the CommonJS require cache these requires hit, + // so evict the module directly. Otherwise whether the uninitialised cases + // below hold depends on some other file's (or earlier test's) init(). + beforeEach(function () { + delete require.cache[require.resolve(JOBS_SERVICE_PATH)]; + jobsService = require(JOBS_SERVICE_PATH); + adapterManager = require('../../../../../core/server/services/adapter-manager').default; + }); + afterEach(function () { sinon.restore(); + // Evict again so an init() from this file's tests never leaks an + // initialised singleton to other files sharing this worker. + delete require.cache[require.resolve(JOBS_SERVICE_PATH)]; }); - // These two run before any init() in this file, so the module singleton is - // still undefined. it('shutdown before init resolves without constructing a service', async function () { await assert.doesNotReject(() => jobsService.shutdown()); }); diff --git a/ghost/core/test/unit/server/services/jobs-service/register-job-handlers.test.ts b/ghost/core/test/unit/server/services/jobs-service/register-job-handlers.test.ts index 48c976f3206..f48bcffe6a0 100644 --- a/ghost/core/test/unit/server/services/jobs-service/register-job-handlers.test.ts +++ b/ghost/core/test/unit/server/services/jobs-service/register-job-handlers.test.ts @@ -6,6 +6,7 @@ import { JobsService } from '../../../../../core/server/services/jobs-service/jo import ExternalMediaInliner from '../../../../../core/server/services/media-inliner/external-media-inliner'; import ExternalMediaInlinerJob from '../../../../../core/server/services/media-inliner/external-media-inliner-job'; import ContentCSVImportJob from '../../../../../core/server/services/content-import/jobs/content-csv-import-job'; +import UpdateCheckJob from '../../../../../core/server/services/update-check/jobs/update-check-job'; const registerJobHandlers = require('../../../../../core/server/services/jobs-service/register-job-handlers').default; @@ -133,4 +134,16 @@ describe('register-job-handlers', function () { /Content import service used before init/, ); }); + + // Under the test env the update check executor exits at its environment + // gate, so invoking the registered handler proves the wiring without + // touching the network. + it('registers the update-check handler', async function () { + // handlerFor matches on the type string, not class identity: the module + // under test loads its job class through the CJS cache, a different + // instance from this file's ESM import. + const updateCheckHandler = handlerFor('update-check'); + + await updateCheckHandler(new UpdateCheckJob()); + }); }); diff --git a/ghost/core/test/unit/server/services/update-check/jobs/update-check-jobs.test.ts b/ghost/core/test/unit/server/services/update-check/jobs/update-check-jobs.test.ts new file mode 100644 index 00000000000..32f546e2dfb --- /dev/null +++ b/ghost/core/test/unit/server/services/update-check/jobs/update-check-jobs.test.ts @@ -0,0 +1,21 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'vitest'; +import { Job } from '../../../../../../core/server/services/jobs-service/job'; +import UpdateCheckJob from '../../../../../../core/server/services/update-check/jobs/update-check-job'; + +describe('UpdateCheckJob', function () { + it('has a stable type', function () { + assert.equal(UpdateCheckJob.type, 'update-check'); + }); + + it('is a job', function () { + assert.ok(new UpdateCheckJob() instanceof Job); + }); + + it('has an empty, serialisable payload', function () { + const job = new UpdateCheckJob(); + + assert.equal(JSON.stringify(job), '{}'); + assert.deepEqual(JSON.parse(JSON.stringify(job)), {}); + }); +}); diff --git a/ghost/core/test/unit/server/services/update-check/schedule-update-check.test.ts b/ghost/core/test/unit/server/services/update-check/schedule-update-check.test.ts new file mode 100644 index 00000000000..16b28072f9e --- /dev/null +++ b/ghost/core/test/unit/server/services/update-check/schedule-update-check.test.ts @@ -0,0 +1,70 @@ +import assert from 'node:assert/strict'; +import sinon from 'sinon'; +import { describe, it, beforeEach, afterEach } from 'vitest'; +import logging from '@tryghost/logging'; + +// require, not import: these must resolve to the same CommonJS module instances +// that core/server/services/update-check/index.js loads - so a stray addJob() +// call is visible here, and the scheduled job is instanceof the class below. +const legacyJobsManager = require('../../../../../core/server/services/jobs'); +const config = require('../../../../../core/shared/config'); +const updateCheck = require('../../../../../core/server/services/update-check'); +const UpdateCheckJob = + require('../../../../../core/server/services/update-check/jobs/update-check-job').default; + +describe('update-check scheduling', function () { + let jobsService: { scheduleRecurring: sinon.SinonStub; dispatch: sinon.SinonStub }; + let addJob: sinon.SinonStub; + let loggingInfo: sinon.SinonStub; + + beforeEach(function () { + jobsService = { + scheduleRecurring: sinon.stub().resolves(), + dispatch: sinon.stub().resolves(), + }; + addJob = sinon.stub(legacyJobsManager, 'addJob'); + loggingInfo = sinon.stub(logging, 'info'); + }); + + afterEach(function () { + sinon.restore(); + }); + + it('schedules a daily update-check job at a random time of day', async function () { + await updateCheck.scheduleJobs(jobsService); + + assert.ok(jobsService.scheduleRecurring.calledOnce); + const [job, schedule] = jobsService.scheduleRecurring.firstCall.args; + assert.ok(job instanceof UpdateCheckJob); + assert.match( + schedule.cron, + /^\d{1,2} \d{1,2} (1?\d|2[0-3]) \* \* \*$/, + 'a random daily cron spread across the full 24 hours', + ); + assert.ok( + loggingInfo.calledWith(`[Background Job] update-check scheduled at ${schedule.cron}`), + 'the scheduled log line is preserved verbatim', + ); + assert.ok( + jobsService.dispatch.notCalled, + 'no boot run is dispatched unless updateCheck:forceUpdate is set', + ); + assert.ok(addJob.notCalled, 'update-check is no longer registered with the legacy job manager'); + }); + + it('also dispatches a one-off boot run when updateCheck:forceUpdate is set', async function () { + sinon.stub(config, 'get').withArgs('updateCheck:forceUpdate').returns(true); + + await updateCheck.scheduleJobs(jobsService); + + assert.ok(jobsService.scheduleRecurring.calledOnce, 'the recurring job is still scheduled'); + assert.ok(jobsService.dispatch.calledOnce); + const [job] = jobsService.dispatch.firstCall.args; + assert.ok(job instanceof UpdateCheckJob); + assert.ok( + loggingInfo.calledWith('[Background Job] update-check boot run queued'), + 'the boot dispatch is logged', + ); + assert.ok(addJob.notCalled, 'the boot run is no longer registered with the legacy job manager'); + }); +});