From ff64ae47565a002bf46b2bbce2f8c0aec60a056e Mon Sep 17 00:00:00 2001 From: Steve Larson <9larsons@gmail.com> Date: Fri, 28 Aug 2026 14:24:34 -0500 Subject: [PATCH 1/2] Fixed E2E runner disk exhaustion (#30353) no ref Artifact-backed Docker image transfers leave the compressed image archive in the workspace after docker load. Analytics E2E shards already pull several large service images, so that retained archive can consume the remaining runner disk and cause ENOSPC failures. The canonical registry-backed path does not retain this archive, which makes the failure specific to forks, mirrors, and cross-repository pull requests even though they use the same workflow. Analytics shards also provisioned a separate Buildx builder before knowing whether the prebuilt Tinybird CLI image was available. That added a BuildKit container and image during the disk-critical setup phase, while source-build cache remained until post-job cleanup. --- .github/actions/load-docker-image/action.yml | 8 +++-- .github/workflows/ci.yml | 35 +++++++++++++------- 2 files changed, 28 insertions(+), 15 deletions(-) diff --git a/.github/actions/load-docker-image/action.yml b/.github/actions/load-docker-image/action.yml index ad1197cfe4e..bc0573974f6 100644 --- a/.github/actions/load-docker-image/action.yml +++ b/.github/actions/load-docker-image/action.yml @@ -8,7 +8,7 @@ inputs: description: 'Docker image tags (multi-line string)' required: true artifact-name: - description: 'Name of the artifact to download (fork PRs only)' + description: 'Name of the image artifact to download' required: false default: 'docker-image' @@ -20,15 +20,17 @@ runs: uses: actions/download-artifact@3e5f45b2cfb9172054b4087a40e8e0b5a5461e7c # v8 with: name: ${{ inputs.artifact-name }} + path: ${{ runner.temp }}/${{ inputs.artifact-name }} - name: Load image from artifact (artifact) if: inputs.use-artifact == 'true' shell: bash env: - ARTIFACT_NAME: ${{ inputs.artifact-name }} + ARTIFACT_PATH: ${{ runner.temp }}/${{ inputs.artifact-name }}/${{ inputs.artifact-name }}.tar.gz run: | + trap 'rm -f -- "$ARTIFACT_PATH"' EXIT echo "Loading Docker image from artifact..." - gunzip -c "${ARTIFACT_NAME}.tar.gz" | docker load + gunzip -c "$ARTIFACT_PATH" | docker load echo "Available images after load:" docker images diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ae3e4c7c5b6..831d6393257 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -2060,28 +2060,33 @@ jobs: - name: Setup Docker Registry Mirrors uses: ./.github/actions/setup-docker-registry-mirrors - - name: Set up Docker Buildx - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4 + - name: Pull Tinybird CLI image + id: pull_tb_cli + if: matrix.analytics == 'true' && needs.job_setup.outputs.changed_tb_cli != 'true' + run: | + COMPOSE_IMAGE="${COMPOSE_PROJECT_NAME:-ghost-dev}-tb-cli" + if docker pull ghcr.io/tryghost/tb-cli:latest; then + docker tag ghcr.io/tryghost/tb-cli:latest "$COMPOSE_IMAGE" + echo "Pulled tb-cli from GHCR" + echo "build_required=false" >> "$GITHUB_OUTPUT" + else + echo "GHCR image not available, building from source" + echo "build_required=true" >> "$GITHUB_OUTPUT" + fi - - name: Pull or build Tinybird CLI Image - if: matrix.analytics == 'true' + - name: Build Tinybird CLI image + if: matrix.analytics == 'true' && (needs.job_setup.outputs.changed_tb_cli == 'true' || steps.pull_tb_cli.outputs.build_required == 'true') env: CHANGED_TB_CLI: ${{ needs.job_setup.outputs.changed_tb_cli }} run: | COMPOSE_IMAGE="${COMPOSE_PROJECT_NAME:-ghost-dev}-tb-cli" - # GHCR's :latest is only republished from main, so it can't reflect a - # Dockerfile change under review — build from source when this PR - # touches it, otherwise take the prebuilt fast path. if [[ "$CHANGED_TB_CLI" == 'true' ]]; then echo "docker/tb-cli changed, building from source" - docker buildx build --load -t "$COMPOSE_IMAGE" -f docker/tb-cli/Dockerfile . - elif docker pull ghcr.io/tryghost/tb-cli:latest 2>/dev/null; then - echo "Pulled tb-cli from GHCR" - docker tag ghcr.io/tryghost/tb-cli:latest "$COMPOSE_IMAGE" else echo "GHCR image not available, building from source" - docker buildx build --load -t "$COMPOSE_IMAGE" -f docker/tb-cli/Dockerfile . fi + docker build --tag "$COMPOSE_IMAGE" --file docker/tb-cli/Dockerfile . + docker builder prune --all --force - name: Load Image uses: ./.github/actions/load-docker-image @@ -2103,6 +2108,12 @@ jobs: # (admin, apps, ghost/core). Scope the install to cut shard setup time. run: pnpm install --frozen-lockfile --filter @tryghost/e2e... + - name: Report Analytics runner disk usage + if: matrix.analytics == 'true' + run: | + df -h / + docker system df + - name: Prepare E2E CI job env: GHOST_E2E_IMAGE: ${{ steps.load.outputs.image-tag }} From 964a148f4755f6c752bfd4c4cf7cc8f3c985c868 Mon Sep 17 00:00:00 2001 From: Paul Davis Date: Fri, 28 Aug 2026 21:48:52 +0100 Subject: [PATCH 2/2] Migrated CSV content imports to the class-based jobs service (#30319) no ref CSV imports need to exercise the serializable jobs contract before M8 adds durable processing. Stage uploads so validation completes before the response while the handler can reparse the staged upload from a pure-data job payload, preserving the temporary in-memory outcomes and 100-row limit. Keep the migration scoped to the current in-memory phase: - Add a serializable job class and central handler registration. - Stage uploads in private temporary files because Multer removes the request upload after the response. - Preflight imports during the request to preserve validation, totals, and the 100-row limit. - Reparse staged uploads in the handler and always attempt cleanup. - Add an `allSettled()` seam so existing E2E tests can deterministically await queued imports. --- ghost/core/core/server/api/endpoints/posts.js | 5 +- .../content-import/import/importer.ts | 170 +++++++++----- .../content-import/import/staged-file.ts | 36 +++ .../services/content-import/import/store.ts | 25 +- .../server/services/content-import/index.ts | 29 ++- .../jobs/content-csv-import-job.ts | 30 +++ .../jobs-service/register-job-handlers.ts | 6 + .../test/e2e-api/admin/posts-importer.test.js | 92 ++++---- .../test/e2e-webhooks/posts-importer.test.ts | 8 +- .../content-import/import/importer.test.ts | 216 ++++++++++++++---- .../content-import/import/staged-file.test.ts | 56 +++++ .../content-import/import/store.test.ts | 29 +++ .../services/content-import/index.test.ts | 19 ++ .../register-job-handlers.test.ts | 17 ++ 14 files changed, 589 insertions(+), 149 deletions(-) create mode 100644 ghost/core/core/server/services/content-import/import/staged-file.ts create mode 100644 ghost/core/core/server/services/content-import/jobs/content-csv-import-job.ts create mode 100644 ghost/core/test/unit/server/services/content-import/import/staged-file.test.ts create mode 100644 ghost/core/test/unit/server/services/content-import/index.test.ts diff --git a/ghost/core/core/server/api/endpoints/posts.js b/ghost/core/core/server/api/endpoints/posts.js index add78359041..ce378911f08 100644 --- a/ghost/core/core/server/api/endpoints/posts.js +++ b/ghost/core/core/server/api/endpoints/posts.js @@ -129,9 +129,8 @@ const controller = { method: 'importContent', }, async query(frame) { - // The CSV must be parsed before the response goes out: the uploaded temp file is - // deleted as soon as it is sent. The posts are written by a background job behind - // the 202. + // The content-import service stages and validates the upload before this response; + // Multer deletes the request temp file as soon as the 202 is sent. const { importId, total } = await contentImportService.importCSV({ filePath: frame.file.path, fileName: frame.file.name, diff --git a/ghost/core/core/server/services/content-import/import/importer.ts b/ghost/core/core/server/services/content-import/import/importer.ts index c36b6fe3f33..836395f86ee 100644 --- a/ghost/core/core/server/services/content-import/import/importer.ts +++ b/ghost/core/core/server/services/content-import/import/importer.ts @@ -13,6 +13,8 @@ import type { Clock, ImportRun, ImportRunStore, RowOutcome } from './store'; import type { PreparedImportSource } from './source'; import type { PreparedPostRow, PreparedPostRows } from './reader'; import { MediaInliningFailure, type PostMediaInlining } from './media'; +import type { ImportFileStager, StagedImportFile } from './staged-file'; +import ContentCSVImportJob from '../jobs/content-csv-import-job'; export type { ImportRequest } from './schema'; @@ -20,9 +22,9 @@ const errors = require('@tryghost/errors'); const logging = require('@tryghost/logging'); const tpl = require('@tryghost/tpl'); -// The CSV is parsed inside the request (the uploaded temp file is deleted when the -// response is sent); the parsed rows are handed to an in-process background job -// that writes one post per row. +// The upload is staged because the request temp file is deleted with the response. +// Request-time parsing preserves synchronous validation and the current row count; +// the class-based job reparses the staged file so its payload stays serializable. // The id is what a completion report will be looked up by. export interface ImportAccepted { @@ -54,7 +56,7 @@ const messages = { function logLifecycle(message: string): void { try { - logging.info(`[Background Job] content-import ${message}`); + logging.info(`[Background Job] ${ContentCSVImportJob.type} ${message}`); } catch { // Observability must not change whether an import is queued or resolves. } @@ -72,7 +74,8 @@ interface ImporterDeps { getCleanHTML: () => CleanHTML; createMediaInliner: () => PostMediaInlining; email: EmailNotifications; - addJob: (job: { job: () => Promise; offloaded: boolean; name: string }) => void; + dispatchJob: (job: ContentCSVImportJob) => Promise; + fileStager: ImportFileStager; report: FailureReporter; store: ImportRunStore; urlForPost: (post: WrittenPost) => string; @@ -97,7 +100,8 @@ class ContentCSVImporter { private _getCleanHTML: () => CleanHTML; private _createMediaInliner: () => PostMediaInlining; private _email: EmailNotifications; - private _addJob: ImporterDeps['addJob']; + private _dispatchJob: ImporterDeps['dispatchJob']; + private _fileStager: ImportFileStager; private _report: FailureReporter; private _store: ImportRunStore; private _urlForPost: (post: WrittenPost) => string; @@ -114,7 +118,8 @@ class ContentCSVImporter { getCleanHTML, createMediaInliner, email, - addJob, + dispatchJob, + fileStager, report, store, urlForPost, @@ -130,7 +135,8 @@ class ContentCSVImporter { this._getCleanHTML = getCleanHTML; this._createMediaInliner = createMediaInliner; this._email = email; - this._addJob = addJob; + this._dispatchJob = dispatchJob; + this._fileStager = fileStager; this._report = report; this._store = store; this._urlForPost = urlForPost; @@ -141,66 +147,103 @@ class ContentCSVImporter { async importCSV(request: ImportRequest): Promise { const emailRecipient = request.requestUserEmail ?? (await this._email.getDefaultRecipient()); - const source = await this._prepareSource(request); - let preparedRows: PreparedPostRows; + let stagedFile: StagedImportFile; try { - const result = await this._readRows(source.filePath, request.mapping); - preparedRows = Array.isArray(result) - ? { columns: [], rows: result.map((data, index) => ({ data, line: index + 2 })) } - : result; + stagedFile = await this._fileStager.stage(request); } catch (error) { - await this.cleanupSource(source.cleanup); throw new errors.ValidationError({ message: tpl(messages.unreadableFile), err: error, }); } + let handedOff = false; - // Temporary while import state is held in memory: the durable job - // system milestone removes the cap. - if (preparedRows.rows.length > MAX_POSTS) { + try { + const stagedRequest = { + filePath: stagedFile.path, + fileName: stagedFile.name, + mapping: request.mapping, + requestUserEmail: request.requestUserEmail, + }; + const { source, preparedRows } = await this.readPreparedRows(stagedRequest); await this.cleanupSource(source.cleanup); - throw new errors.ValidationError({ - message: tpl(messages.tooManyPosts, { max: MAX_POSTS }), + + this.assertWithinPostLimit(preparedRows.rows); + + const runId = this._newRunId(); + const importTagNames = buildImportTagNames(runId, this._getTimezone(), this._now()); + this._store.create(runId, preparedRows.rows.length, preparedRows.columns); + const job = new ContentCSVImportJob({ + importId: runId, + file: stagedFile, + mapping: request.mapping, + importTagNames, + emailRecipient, }); + + logLifecycle('queued'); + try { + await this._dispatchJob(job); + } catch (error) { + this._store.fail(runId, messageOf(error)); + this._store.release(runId); + throw error; + } + + handedOff = true; + return { importId: runId, total: preparedRows.rows.length }; + } finally { + if (!handedOff) { + await this.cleanupStagedFile(stagedFile); + } } + } - const runId = this._newRunId(); - const importTagNames = buildImportTagNames(runId, this._getTimezone(), this._now()); - this._store.create(runId, preparedRows.rows.length, preparedRows.columns); + async handle(job: ContentCSVImportJob): Promise { + let source: PreparedImportSource | undefined; - logLifecycle('queued'); try { - this._addJob({ - job: () => - this.runImportJob(runId, importTagNames, preparedRows.rows, source, emailRecipient), - offloaded: false, - name: 'content-import', + const prepared = await this.readPreparedRows({ + filePath: job.file.path, + fileName: job.file.name, + mapping: job.mapping, }); + source = prepared.source; + this.assertWithinPostLimit(prepared.preparedRows.rows); + await this.processRows( + job.importId, + job.importTagNames, + prepared.preparedRows.rows, + prepared.source, + ); } catch (error) { - this._store.fail(runId, messageOf(error)); - await this.cleanupSource(source.cleanup); - this._store.release(runId); + this._store.fail(job.importId, messageOf(error)); throw error; + } finally { + if (source) { + await this.cleanupSource(source.cleanup); + } + const run = this._store.get(job.importId); + if (run) { + await this.settle(() => this._email.send(run, job.emailRecipient)); + } + this._store.release(job.importId); + await this.cleanupStagedFile(job.file); } + } - return { importId: runId, total: preparedRows.rows.length }; + allSettled(): Promise { + return this._store.allSettled(); } - // Must resolve in every case: the job manager reads a rejected inline job as a - // defect in the job itself, and there is no retry behind it. - private async runImportJob( + private async processRows( runId: string, importTagNames: string[], rows: PreparedPostRow[], source: PreparedImportSource, - emailRecipient: string, ): Promise { - const startedAt = Date.now(); - logLifecycle('started'); let urlFailureCount = 0; let firstUrlFailure: unknown; - let failed = false; try { if (source.assets) { @@ -347,19 +390,36 @@ class ContentCSVImporter { this.reportUrlFailures(urlFailureCount, firstUrlFailure); this._store.finish(runId); } catch (error) { - failed = true; this.reportUrlFailures(urlFailureCount, firstUrlFailure); - this._report(error); - this._store.fail(runId, messageOf(error)); - } finally { + throw error; + } + } + + private async readPreparedRows( + request: ImportRequest, + ): Promise<{ source: PreparedImportSource; preparedRows: PreparedPostRows }> { + const source = await this._prepareSource(request); + try { + const result = await this._readRows(source.filePath, request.mapping); + const preparedRows = Array.isArray(result) + ? { columns: [], rows: result.map((data, index) => ({ data, line: index + 2 })) } + : result; + return { source, preparedRows }; + } catch (error) { await this.cleanupSource(source.cleanup); - const run = this._store.get(runId); - if (run) { - await this.settle(() => this._email.send(run, emailRecipient)); - } - this._store.release(runId); - const outcome = failed ? 'failed after' : 'completed in'; - logLifecycle(`${outcome} ${Date.now() - startedAt}ms`); + throw new errors.ValidationError({ + message: tpl(messages.unreadableFile), + err: error, + }); + } + } + + private assertWithinPostLimit(rows: PreparedPostRow[]): void { + // Temporary while import state is held in memory: M8 removes the cap. + if (rows.length > MAX_POSTS) { + throw new errors.ValidationError({ + message: tpl(messages.tooManyPosts, { max: MAX_POSTS }), + }); } } @@ -393,6 +453,14 @@ class ContentCSVImporter { } } + private async cleanupStagedFile(file: StagedImportFile): Promise { + try { + await this._fileStager.remove(file); + } catch (error) { + this._report(error); + } + } + private async settle(operation: () => Promise): Promise { try { await operation(); diff --git a/ghost/core/core/server/services/content-import/import/staged-file.ts b/ghost/core/core/server/services/content-import/import/staged-file.ts new file mode 100644 index 00000000000..3ded349c1fd --- /dev/null +++ b/ghost/core/core/server/services/content-import/import/staged-file.ts @@ -0,0 +1,36 @@ +import crypto from 'node:crypto'; +import os from 'node:os'; +import path from 'node:path'; +import fs from 'fs-extra'; + +export interface StagedImportFile { + path: string; + name: string; +} + +export interface ImportFileStager { + stage(file: { filePath: string; fileName: string }): Promise; + remove(file: StagedImportFile): Promise; +} + +export function createImportFileStager(): ImportFileStager { + return { + async stage({ filePath, fileName }) { + const stagedPath = path.join(os.tmpdir(), `content-csv-import-${crypto.randomUUID()}`); + + try { + await fs.copyFile(filePath, stagedPath, fs.constants.COPYFILE_EXCL); + await fs.chmod(stagedPath, 0o600); + } catch (error) { + await fs.remove(stagedPath).catch(() => {}); + throw error; + } + + return { path: stagedPath, name: fileName }; + }, + + async remove(file) { + await fs.remove(file.path); + }, + }; +} diff --git a/ghost/core/core/server/services/content-import/import/store.ts b/ghost/core/core/server/services/content-import/import/store.ts index e25d42445f2..e82f3d2d569 100644 --- a/ghost/core/core/server/services/content-import/import/store.ts +++ b/ghost/core/core/server/services/content-import/import/store.ts @@ -47,6 +47,7 @@ const MAX_RUN_AGE_MS = 60 * 60 * 1000; export class ImportRunStore { // Insertion-ordered, so count-eviction drops the oldest run first. private _runs = new Map(); + private _settledWaiters = new Set<() => void>(); private _now: Clock; constructor({ now = () => new Date() }: { now?: Clock } = {}) { @@ -95,12 +96,34 @@ export class ImportRunStore { release(id: string): void { this._runs.delete(id); + this.resolveSettledWaiters(); + } + + allSettled(): Promise { + if (this._runs.size === 0) { + return Promise.resolve(); + } + + return new Promise((resolve) => { + this._settledWaiters.add(resolve); + }); + } + + private resolveSettledWaiters(): void { + if (this._runs.size > 0) { + return; + } + + for (const resolve of this._settledWaiters) { + resolve(); + } + this._settledWaiters.clear(); } // A running run is never evicted, whatever its age: the job holds only the runId, // so evicting mid-import would silently turn its record()/finish() calls into // no-ops and lose the report. The count cap can briefly overshoot while several - // imports run at once; the inline job queue bounds how many that can be. + // imports run at once; the jobs backend bounds how many that can be. private evict(): void { const cutoff = this._now().getTime() - MAX_RUN_AGE_MS; for (const [id, run] of this._runs) { diff --git a/ghost/core/core/server/services/content-import/index.ts b/ghost/core/core/server/services/content-import/index.ts index 5efde634691..43167a719ee 100644 --- a/ghost/core/core/server/services/content-import/index.ts +++ b/ghost/core/core/server/services/content-import/index.ts @@ -13,6 +13,9 @@ import { prepareImportSource } from './import/source'; import { PostMediaInliner } from './import/media'; import { isLocalMediaUrl } from './import/local-media-url'; import { urlForImportedPost } from './import/post-link'; +import { createImportFileStager } from './import/staged-file'; +import ContentCSVImportJob from './jobs/content-csv-import-job'; +import { getInstance as getJobsService } from '../jobs-service'; // The request is built from HTTP upload metadata, so it is validated at the // service boundary rather than trusted. @@ -30,7 +33,6 @@ function makeImporter(): ContentCSVImporter { // are guaranteed loaded. const models = require('../../models'); const lexicalLib = require('../../lib/lexical'); - const jobsService = require('../jobs'); const settingsCache = require('../../../shared/settings-cache'); const urlService = require('../url'); const urlUtils = require('../../../shared/url-utils').default; @@ -40,8 +42,8 @@ function makeImporter(): ContentCSVImporter { const { GhostMailer } = require('../mail'); const ghostMailer = new GhostMailer(); - // Inline jobs never reach the job manager's Sentry handler, which is wired to the - // offloaded worker path only, so a throw here would be seen by nobody. + // Row aggregates and best-effort cleanup are intentionally reported without + // failing a run. Fatal handler errors are reported by the class-based jobs service. const report: FailureReporter = (error) => { try { logging.error( @@ -82,7 +84,8 @@ function makeImporter(): ContentCSVImporter { }), }), email, - addJob: jobsService.addJob.bind(jobsService), + dispatchJob: (job) => getJobsService().dispatch(job), + fileStager: createImportFileStager(), report, store: new ImportRunStore(), urlForPost: (post) => @@ -122,3 +125,21 @@ export function importCSV(request: ImportRequest): Promise { return importer.importCSV(parsedRequest.data); } + +export function handleJob(job: ContentCSVImportJob): Promise { + if (!importer) { + throw new errors.InternalServerError({ message: 'Content import service used before init' }); + } + + return importer.handle(job); +} + +// Test-facing parity with the legacy inline queue while the import run store +// remains in memory. M8 removes this together with that store. +export function allSettled(): Promise { + if (!importer) { + return Promise.resolve(); + } + + return importer.allSettled(); +} diff --git a/ghost/core/core/server/services/content-import/jobs/content-csv-import-job.ts b/ghost/core/core/server/services/content-import/jobs/content-csv-import-job.ts new file mode 100644 index 00000000000..118fed22ca1 --- /dev/null +++ b/ghost/core/core/server/services/content-import/jobs/content-csv-import-job.ts @@ -0,0 +1,30 @@ +import { Job } from '../../jobs-service/job'; + +export interface ContentCSVImportJobData { + importId: string; + file: { + path: string; + name: string; + }; + mapping?: Record; + importTagNames: string[]; + emailRecipient: string; +} + +export default class ContentCSVImportJob extends Job { + static type = 'content-csv-import'; + readonly importId: string; + readonly file: ContentCSVImportJobData['file']; + readonly mapping?: Record; + readonly importTagNames: string[]; + readonly emailRecipient: string; + + constructor(data: ContentCSVImportJobData) { + super(); + this.importId = data.importId; + this.file = data.file; + this.mapping = data.mapping; + this.importTagNames = data.importTagNames; + this.emailRecipient = data.emailRecipient; + } +} 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 9e1451df159..166ca2bce9a 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 @@ -8,6 +8,8 @@ import * as gifts from '../gifts'; import CleanGiftsJob from '../gifts/jobs/clean-gifts-job'; 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'; interface RegisterJobHandlersDependencies { jobsService: JobsService; @@ -57,4 +59,8 @@ export default function registerJobHandlers({ jobsService.handle(ExternalMediaInlinerJob, async (job) => { await mediaInliner.inline(job.domains); }); + + jobsService.handle(ContentCSVImportJob, async (job) => { + await contentImport.handleJob(job); + }); } diff --git a/ghost/core/test/e2e-api/admin/posts-importer.test.js b/ghost/core/test/e2e-api/admin/posts-importer.test.js index 76fc0bc5a6e..11910d42fa2 100644 --- a/ghost/core/test/e2e-api/admin/posts-importer.test.js +++ b/ghost/core/test/e2e-api/admin/posts-importer.test.js @@ -14,7 +14,7 @@ const path = require('path'); const nock = require('nock'); const papaparse = require('papaparse'); const models = require('../../../core/server/models'); -const jobsService = require('../../../core/server/services/jobs'); +const contentImportService = require('../../../core/server/services/content-import'); const mediaInlinerService = require('../../../core/server/services/media-inliner'); const { PostMediaInliner, @@ -111,7 +111,7 @@ describe('Posts Importer API', function () { afterEach(async function () { // Every accepted upload schedules a background import — drain it so a job // doesn't run on into another test (or another file on this fork's DB) - await jobsService.allSettled(); + await contentImportService.allSettled(); await cleanupRemoteImportedMedia(); await Promise.all(getImportedAssetPaths().map((filePath) => fs.rm(filePath, { force: true }))); mockManager.restore(); @@ -142,7 +142,7 @@ describe('Posts Importer API', function () { .post('posts/upload/') .attach('postsfile', completionCsvPath) .expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const email = mockManager.assert.sentEmail({ subject: 'Your content import is complete', @@ -180,7 +180,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', preExistingPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); mockManager.assert.sentEmail({ subject: 'Your content import is complete' }); const reportPath = await csvFile( @@ -200,7 +200,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', reportPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const email = mockManager.assert.sentEmail({ subject: 'Your content import is complete' }); assert.match(email.html, /processed 6 rows/); @@ -275,7 +275,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').body(form).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const email = mockManager.assert.sentEmail({ subject: 'Your content import was unsuccessful', @@ -309,23 +309,27 @@ describe('Posts Importer API', function () { contentType: 'text/csv', }); await agent.post('posts/upload/').body(retryForm).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); mockManager.assert.sentEmail({ subject: 'Your content import was unsuccessful' }); }); it('Keeps content import initialization idempotent and rejects invalid service requests', async function () { - const contentImportService = + const isolatedContentImportService = await import('../../../core/server/services/content-import/index.ts?coverage-lifecycle'); assert.throws( - () => contentImportService.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }), + () => + isolatedContentImportService.importCSV({ + filePath: '/tmp/posts.csv', + fileName: 'posts.csv', + }), /Content import service used before init/, ); - contentImportService.init(); - contentImportService.init(); + isolatedContentImportService.init(); + isolatedContentImportService.init(); assert.throws( - () => contentImportService.importCSV({ filePath: '', fileName: '' }), + () => isolatedContentImportService.importCSV({ filePath: '', fileName: '' }), (error) => { assert.equal(error.errorType, 'ValidationError'); assert.match(error.message, /Too small/); @@ -333,7 +337,7 @@ describe('Posts Importer API', function () { }, ); await assert.rejects( - contentImportService.importCSV({ + isolatedContentImportService.importCSV({ filePath: path.join(tmpDir, 'missing.csv'), fileName: 'missing.csv', }), @@ -382,7 +386,7 @@ describe('Posts Importer API', function () { const filePath = await csvFile('remote-media.csv', csv); await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); for (const request of requests) { assert.equal(request.isDone(), true, request.pendingMocks().join(', ')); @@ -436,7 +440,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); assert.equal(request.isDone(), true, request.pendingMocks().join(', ')); const post = await models.Post.findOne({ title: 'Unsupported remote media', status: 'all' }); @@ -479,7 +483,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); sinon.assert.notCalled(importUrl); const post = await models.Post.findOne({ title: 'Local media', status: 'all' }); @@ -606,7 +610,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', filePath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); sinon.assert.calledOnceWithExactly(importUrl, sourceUrl); const post = await models.Post.findOne({ title: 'Unexpected media failure', status: 'all' }); @@ -631,7 +635,7 @@ describe('Posts Importer API', function () { const { body } = await agent.post('posts/upload/').body(form).expectStatus(202); assert.equal(body.meta.total, 1); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'ZIP mapping post', status: 'all' }); assert.ok(post); assert.match(post.get('html'), /Mapped from ZIP/); @@ -658,7 +662,7 @@ describe('Posts Importer API', function () { .attach('postsfile', zipPath) .expectStatus(202); assert.equal(body.meta.total, 3); - await jobsService.allSettled(); + await contentImportService.allSettled(); sinon.assert.notCalled(importUrl); @@ -719,7 +723,7 @@ describe('Posts Importer API', function () { .attach('postsfile', zipPath) .expectStatus(202); assert.equal(body.meta.total, 1); - await jobsService.allSettled(); + await contentImportService.allSettled(); for (const filePath of getImportedAssetPaths().slice(4, 7)) { assert.equal(await fs.stat(filePath).then(() => true), true, `${filePath} was stored`); @@ -760,7 +764,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').attach('postsfile', zipPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'ZIP failed assets', status: 'all' }); assert.equal(post, null); @@ -785,7 +789,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').attach('postsfile', zipPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'ZIP partial file failure', status: 'all' }); assert.equal(post, null); @@ -806,7 +810,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').attach('postsfile', zipPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'ZIP cross-group failure', status: 'all' }); assert.equal(post, null); @@ -835,7 +839,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').attach('postsfile', zipPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'ZIP incomplete rollback', status: 'all' }); assert.equal(post, null); @@ -1058,7 +1062,7 @@ describe('Posts Importer API', function () { assert.match(body.meta.import_id, /^[0-9a-f]{24}$/); assert.equal(body.meta.total, 2); - await jobsService.allSettled(); + await contentImportService.allSettled(); const { data: posts } = await models.Post.findPage({ filter: `title:~'Content check post'`, @@ -1133,9 +1137,9 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', duplicateCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); await agent.post('posts/upload/').attach('postsfile', duplicateCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const { data: posts } = await models.Post.findPage({ filter: "slug:'csv-deduplication-check'", @@ -1157,7 +1161,7 @@ describe('Posts Importer API', function () { 'CSV source ID original,csv-source-id-original,m5-source-id-primary\n', ); await agent.post('posts/upload/').attach('postsfile', originalCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const comparisonCsvPath = await csvFile( 'posts-import-source-id-comparisons.csv', @@ -1176,7 +1180,7 @@ describe('Posts Importer API', function () { contentType: 'text/csv', }); await agent.post('posts/upload/').body(form).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const original = await models.Post.findOne({ slug: 'csv-source-id-original', status: 'all' }); const sourceDuplicate = await models.Post.findOne({ @@ -1208,7 +1212,7 @@ describe('Posts Importer API', function () { 'CSV update slug original,csv-update-by-slug,,2025-01-01T00:00:00.000Z\n', ); await agent.post('posts/upload/').attach('postsfile', originalCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const updatesCsvPath = await csvFile( 'posts-import-update-comparisons.csv', @@ -1221,7 +1225,7 @@ describe('Posts Importer API', function () { 'CSV update after invalid,csv-update-after-invalid,,2025-04-01T00:00:00.000Z\n', ); await agent.post('posts/upload/').attach('postsfile', updatesCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const newer = await models.Post.findOne({ comment_id: 'm5-update-source', @@ -1258,7 +1262,7 @@ describe('Posts Importer API', function () { await agent.post('posts/upload/').attach('postsfile', paidSiteCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'Visibility check post', status: 'all' }); // left to the model, visibility would have followed default_content_visibility @@ -1298,7 +1302,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').body(form).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne( { title: 'Mapped field post', status: 'all' }, @@ -1361,7 +1365,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').body(form).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne( { title: 'CSV existing relations', status: 'all' }, @@ -1396,7 +1400,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', authorsCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const contributor = await models.User.findOne( { email: 'new-csv-contributor@example.com', status: 'all' }, @@ -1447,7 +1451,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', authorsCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); assert.equal( await models.User.findOne({ email: 'csv-rollback-contributor@example.com', status: 'all' }), @@ -1469,14 +1473,14 @@ describe('Posts Importer API', function () { 'CSV created tags two,"#CSV Internal Tag,New CSV Tag"\n', ); await agent.post('posts/upload/').attach('postsfile', firstCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const secondCsvPath = await csvFile( 'posts-import-reused-tags.csv', 'title,tags\nCSV reused tags,New CSV Tag\n', ); await agent.post('posts/upload/').attach('postsfile', secondCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const publicTags = await models.Tag.findAll({ filter: "name:'New CSV Tag'" }); const internalTags = await models.Tag.findAll({ filter: "name:'#CSV Internal Tag'" }); @@ -1534,7 +1538,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', tagsCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); assert.equal(await models.Tag.findOne({ name: 'CSV Rollback Tag' }), null); assert.equal(await models.Post.findOne({ title: 'CSV tag rollback', status: 'all' }), null); @@ -1556,7 +1560,7 @@ describe('Posts Importer API', function () { }); await agent.post('posts/upload/').body(form).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'Markdown field post', status: 'all' }); assert.ok(post); @@ -1573,7 +1577,7 @@ describe('Posts Importer API', function () { ); await agent.post('posts/upload/').attach('postsfile', cleanupCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const post = await models.Post.findOne({ title: 'Clean HTML post', status: 'all' }); assert.ok(post); @@ -1601,7 +1605,7 @@ describe('Posts Importer API', function () { await agent.post('posts/upload/').attach('postsfile', badRowsCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const { data: posts } = await models.Post.findPage({ filter: `title:~'Bad rows check'`, @@ -1632,7 +1636,7 @@ describe('Posts Importer API', function () { await agent.post('posts/upload/').attach('postsfile', garbageCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const { meta: { @@ -1661,7 +1665,7 @@ describe('Posts Importer API', function () { assert.match(body.errors[0].message, /more than 100 posts/); - await jobsService.allSettled(); + await contentImportService.allSettled(); const { data: posts } = await models.Post.findPage({ filter: `title:~'Over cap post'`, diff --git a/ghost/core/test/e2e-webhooks/posts-importer.test.ts b/ghost/core/test/e2e-webhooks/posts-importer.test.ts index e709ad138db..c9eb406a42b 100644 --- a/ghost/core/test/e2e-webhooks/posts-importer.test.ts +++ b/ghost/core/test/e2e-webhooks/posts-importer.test.ts @@ -6,7 +6,7 @@ import assert from 'node:assert/strict'; const DomainEvents = require('@tryghost/domain-events'); const { agentProvider, mockManager, fixtureManager, dbUtils } = require('../utils/e2e-framework'); const models = require('../../core/server/models'); -const jobsService = require('../../core/server/services/jobs'); +const contentImportService = require('../../core/server/services/content-import'); // The importer's key safety guarantee: a bulk import sends zero newsletter emails // and fires zero per-post webhooks. Every consumer of post events checks @@ -36,7 +36,7 @@ describe('CSV content import side-effects', function () { }); afterEach(async function () { - await jobsService.allSettled(); + await contentImportService.allSettled(); mockManager.restore(); }); @@ -99,7 +99,7 @@ describe('CSV content import side-effects', function () { ); await adminAPIAgent.post('posts/upload/').attach('postsfile', csvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); const updateCsvPath = path.join(tmpDir, 'posts-import-update-side-effects.csv'); await fs.writeFile( @@ -109,7 +109,7 @@ describe('CSV content import side-effects', function () { ); await adminAPIAgent.post('posts/upload/').attach('postsfile', updateCsvPath).expectStatus(202); - await jobsService.allSettled(); + await contentImportService.allSettled(); await DomainEvents.allSettled(); // The negative needs a settle window: a wrongly-fired webhook reaches nock a // beat after the model event, and asserting too early would pass vacuously diff --git a/ghost/core/test/unit/server/services/content-import/import/importer.test.ts b/ghost/core/test/unit/server/services/content-import/import/importer.test.ts index cbafbd96a70..1893f085381 100644 --- a/ghost/core/test/unit/server/services/content-import/import/importer.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/importer.test.ts @@ -8,6 +8,7 @@ import type { PostImportRow } from '../../../../../../core/server/services/conte import type { PostData } from '../../../../../../core/server/services/content-import/import/post-data'; import type { PostWriteMetadata } from '../../../../../../core/server/services/content-import/import/post-repository'; import type { ImportRun } from '../../../../../../core/server/services/content-import/import/store'; +import ContentCSVImportJob from '../../../../../../core/server/services/content-import/jobs/content-csv-import-job'; const row = (title: string, html = `

${title}

`): PostImportRow => ({ title, @@ -23,7 +24,9 @@ function harness( ) { const created: Array<{ data: PostData; options: object; metadata?: PostWriteMetadata }> = []; const reported: unknown[] = []; - const jobs: Array<{ name: string; offloaded: boolean; job: () => Promise }> = []; + const jobs: ContentCSVImportJob[] = []; + const stagedFiles: Array<{ path: string; name: string }> = []; + const removedFiles: Array<{ path: string; name: string }> = []; const createFailures = new Map(); const duplicateSlugs = new Set(); const updatedTitles = new Set(); @@ -99,9 +102,19 @@ function harness( send: sendEmail, getDefaultRecipient, }, - addJob: (job: { name: string; offloaded: boolean; job: () => Promise }) => { + dispatchJob: async (job: ContentCSVImportJob) => { jobs.push(job); }, + fileStager: { + stage: async ({ fileName }: { fileName: string }) => { + const file = { path: `/tmp/staged-${stagedFiles.length + 1}`, name: fileName }; + stagedFiles.push(file); + return file; + }, + remove: async (file: { path: string; name: string }) => { + removedFiles.push(file); + }, + }, report: (error: unknown) => { reported.push(error); }, @@ -120,14 +133,14 @@ function harness( const importer = new ContentCSVImporter(deps); - // The scheduled job is invoked directly rather than through the job manager. + // The dispatched job is invoked directly rather than through the jobs backend. const run = async () => { const accepted = await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv', }); for (const job of jobs) { - await job.job(); + await importer.handle(job); } return accepted; }; @@ -151,6 +164,8 @@ function harness( created, reported, jobs, + stagedFiles, + removedFiles, createFailures, duplicateSlugs, updatedTitles, @@ -184,7 +199,7 @@ describe('ContentCSVImporter', function () { sinon.restore(); }); - it('accepts the upload with the row count and defers the writes to one inline job', async function () { + it('accepts the upload with the row count and dispatches one serializable job', async function () { const h = harness(); const accepted = await h.importer.importCSV({ @@ -194,8 +209,15 @@ describe('ContentCSVImporter', function () { assert.deepEqual(accepted, { importId: 'run_test', total: 2 }); assert.equal(h.jobs.length, 1); - assert.equal(h.jobs[0].name, 'content-import'); - assert.equal(h.jobs[0].offloaded, false); + assert.equal(h.jobs[0].constructor, ContentCSVImportJob); + const serialized = JSON.parse(JSON.stringify(h.jobs[0])); + assert.deepEqual(serialized, { + importId: 'run_test', + file: { path: '/tmp/staged-1', name: 'posts.csv' }, + importTagNames: ['#Import 2026-01-01 11:30', '#Import Run run_test'], + emailRecipient: 'owner@example.com', + }); + assert.deepEqual(JSON.parse(JSON.stringify(new ContentCSVImportJob(serialized))), serialized); assert.equal(h.created.length, 0, 'nothing is written until the job runs'); assert.equal( h.store.get('run_test')?.status, @@ -204,14 +226,30 @@ describe('ContentCSVImporter', function () { ); }); - it('logs the searchable lifecycle of the inline job', async function () { + it('logs when the class-based job is queued', async function () { const h = harness(); await h.run(); - sinon.assert.calledWithExactly(infoLog, '[Background Job] content-import queued'); - sinon.assert.calledWithExactly(infoLog, '[Background Job] content-import started'); - sinon.assert.calledWithMatch(infoLog, /^\[Background Job\] content-import completed in \d+ms$/); + sinon.assert.calledWithExactly(infoLog, '[Background Job] content-csv-import queued'); + assert.deepEqual(h.removedFiles, [{ path: '/tmp/staged-1', name: 'posts.csv' }]); + }); + + it('settles only after the dispatched import has finished reporting and cleanup', async function () { + const h = harness(); + await h.importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); + let settled = false; + const waiting = h.importer.allSettled().then(() => { + settled = true; + }); + + await Promise.resolve(); + assert.equal(settled, false); + + h.releaseRun.restore(); + await h.importer.handle(h.jobs[0]); + await waiting; + assert.equal(settled, true); }); it('carries original source columns and cells into the completed run', async function () { @@ -262,7 +300,7 @@ describe('ContentCSVImporter', function () { fileName: 'posts.csv', requestUserEmail: 'requester@example.com', }); - await h.jobs[0].job(); + await h.importer.handle(h.jobs[0]); sinon.assert.notCalled(h.getDefaultRecipient); sinon.assert.calledOnce(h.sendEmail); @@ -315,7 +353,7 @@ describe('ContentCSVImporter', function () { throw failure; }); - await h.run(); + await assert.rejects(h.run(), failure); sinon.assert.calledOnce(h.sendEmail); assert.equal(h.sentRuns[0].status, 'failed'); @@ -329,7 +367,7 @@ describe('ContentCSVImporter', function () { await h.importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); h.store.release('run_test'); - await h.jobs[0].job(); + await h.importer.handle(h.jobs[0]); sinon.assert.notCalled(h.sendEmail); assert.equal(h.created.length, 2); @@ -341,7 +379,7 @@ describe('ContentCSVImporter', function () { const importer = new ContentCSVImporter(deps); await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); - await h.jobs[0].job(); + await importer.handle(h.jobs[0]); assert.ok(h.store.get('run_test')?.startedAt instanceof Date); assert.ok(h.store.get('run_test')?.finishedAt instanceof Date); @@ -365,11 +403,11 @@ describe('ContentCSVImporter', function () { it('passes a caller-supplied mapping to the CSV reader', async function () { const h = harness(); - let receivedMapping: Record | undefined; + const receivedMappings: Array | undefined> = []; const importer = new ContentCSVImporter({ ...h.deps, readRows: async (_path, mapping) => { - receivedMapping = mapping; + receivedMappings.push(mapping); return [row('Mapped')]; }, }); @@ -379,8 +417,10 @@ describe('ContentCSVImporter', function () { fileName: 'posts.csv', mapping: { Headline: 'title' }, }); + await importer.handle(h.jobs[0]); - assert.deepEqual(receivedMapping, { Headline: 'title' }); + assert.deepEqual(receivedMappings, [{ Headline: 'title' }, { Headline: 'title' }]); + assert.deepEqual(h.jobs[0].mapping, { Headline: 'title' }); }); it('reads a prepared archive source and cleans it after the job', async function () { @@ -399,9 +439,9 @@ describe('ContentCSVImporter', function () { await importer.importCSV({ filePath: '/tmp/upload', fileName: 'posts.zip' }); assert.equal(receivedPath, '/tmp/extracted/posts.csv'); - sinon.assert.notCalled(cleanup); - await h.jobs[0].job(); sinon.assert.calledOnce(cleanup); + await importer.handle(h.jobs[0]); + sinon.assert.calledTwice(cleanup); }); it('stores and rewrites every asset before resolving content converters', async function () { @@ -436,11 +476,11 @@ describe('ContentCSVImporter', function () { }); await importer.importCSV({ filePath: '/tmp/posts.zip', fileName: 'posts.zip' }); - await h.jobs[0].job(); + await importer.handle(h.jobs[0]); assert.deepEqual(events.slice(0, 4), ['store', 'rewrite', 'convert', 'inline']); assert.match(h.created[0].data.lexical ?? '', /unique\.jpg/); - sinon.assert.calledOnce(cleanup); + sinon.assert.calledTwice(cleanup); }); it('fails the run without converting or creating posts when asset storage fails', async function () { @@ -464,15 +504,15 @@ describe('ContentCSVImporter', function () { }); await importer.importCSV({ filePath: '/tmp/posts.zip', fileName: 'posts.zip' }); - await h.jobs[0].job(); + await assert.rejects(importer.handle(h.jobs[0]), failure); assert.equal(h.converterResolutions(), 0); assert.equal(h.created.length, 0); assert.equal(h.store.get('run_test')?.status, 'failed'); assert.equal(h.store.get('run_test')?.failureReason, 'storage unavailable'); - assert.equal(h.reported.at(-1), failure); + assert.deepEqual(h.reported, []); sinon.assert.notCalled(rewriteRows); - sinon.assert.calledOnce(cleanup); + sinon.assert.calledTwice(cleanup); }); it('writes one post per row, in order, under the importing options', async function () { @@ -520,7 +560,7 @@ describe('ContentCSVImporter', function () { }); await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); - await h.jobs[0].job(); + await importer.handle(h.jobs[0]); assert.deepEqual(events, ['convert', 'inline', 'write']); }); @@ -529,9 +569,9 @@ describe('ContentCSVImporter', function () { const h = harness([row('Cached media')]); await h.importer.importCSV({ filePath: '/tmp/first.csv', fileName: 'first.csv' }); - await h.jobs[0].job(); + await h.importer.handle(h.jobs[0]); await h.importer.importCSV({ filePath: '/tmp/second.csv', fileName: 'second.csv' }); - await h.jobs[1].job(); + await h.importer.handle(h.jobs[1]); sinon.assert.calledTwice(h.createMediaInliner); assert.notEqual( @@ -601,11 +641,11 @@ describe('ContentCSVImporter', function () { const failure = new Error('media importer defect'); h.inlineMedia.rejects(failure); - await h.run(); + await assert.rejects(h.run(), failure); assert.equal(h.created.length, 0); assert.equal(h.inlineMedia.callCount, 1); - assert.deepEqual(h.reported, [failure]); + assert.deepEqual(h.reported, []); assert.equal(h.store.get('run_test')?.status, 'failed'); assert.equal(h.store.get('run_test')?.failureReason, failure.message); }); @@ -681,6 +721,28 @@ describe('ContentCSVImporter', function () { assert.equal(h.jobs.length, 0, 'no job was scheduled'); sinon.assert.calledOnce(cleanup); assert.equal(h.reported.at(-1), cleanupError); + assert.deepEqual(h.removedFiles, [{ path: '/tmp/staged-1', name: 'posts.csv' }]); + }); + + it('reports an upload that cannot be staged as an unreadable file', async function () { + const h = harness(); + const importer = new ContentCSVImporter({ + ...h.deps, + fileStager: { + ...h.deps.fileStager, + stage: async () => { + throw new Error('copy failed'); + }, + }, + }); + + await assert.rejects( + importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }), + /The file could not be parsed as a CSV file/, + ); + + assert.equal(h.jobs.length, 0); + assert.equal(h.store.get('run_test'), undefined); }); it('resolves the html converter once per run, not per row', async function () { @@ -738,15 +800,65 @@ describe('ContentCSVImporter', function () { assert.equal(h.store.get('run_test'), undefined, 'no run was registered'); sinon.assert.notCalled(storeAssets); sinon.assert.calledOnce(cleanup); + assert.deepEqual(h.removedFiles, [{ path: '/tmp/staged-1', name: 'posts.zip' }]); }); - it('cleans a prepared source if scheduling throws', async function () { + it('rechecks the temporary cap inside the job before writing', async function () { + const h = harness(); + let readCount = 0; + const importer = new ContentCSVImporter({ + ...h.deps, + readRows: async () => { + readCount += 1; + return readCount === 1 + ? [row('Preflight')] + : Array.from({ length: 101 }, (_, i) => row(`Post ${i + 1}`)); + }, + }); + + await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); + await assert.rejects(importer.handle(h.jobs[0]), /more than 100 posts/); + + assert.equal(h.created.length, 0); + assert.equal(h.store.get('run_test')?.status, 'failed'); + assert.match(h.store.get('run_test')?.failureReason ?? '', /more than 100 posts/); + assert.deepEqual(h.removedFiles, [{ path: '/tmp/staged-1', name: 'posts.csv' }]); + }); + + it('fails and emails the run if the staged file cannot be reparsed by the job', async function () { + const h = harness(); + let readCount = 0; + const importer = new ContentCSVImporter({ + ...h.deps, + readRows: async () => { + readCount += 1; + if (readCount === 2) { + throw new Error('staged file unavailable'); + } + return [row('Preflight')]; + }, + }); + + await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); + await assert.rejects(importer.handle(h.jobs[0]), /could not be parsed as a CSV/); + + assert.equal(h.store.get('run_test')?.status, 'failed'); + assert.match(h.store.get('run_test')?.failureReason ?? '', /could not be parsed as a CSV/); + sinon.assert.calledOnceWithExactly( + h.sendEmail, + sinon.match({ status: 'failed' }), + 'owner@example.com', + ); + assert.deepEqual(h.removedFiles, [{ path: '/tmp/staged-1', name: 'posts.csv' }]); + }); + + it('cleans a prepared source if dispatch fails', async function () { const h = harness(); const cleanup = sinon.stub().resolves(); const importer = new ContentCSVImporter({ ...h.deps, prepareSource: async () => ({ filePath: '/tmp/extracted/posts.csv', cleanup }), - addJob: () => { + dispatchJob: async () => { throw new Error('queue unavailable'); }, }); @@ -760,6 +872,7 @@ describe('ContentCSVImporter', function () { sinon.assert.calledOnce(cleanup); sinon.assert.notCalled(h.sendEmail); sinon.assert.calledOnceWithExactly(h.releaseRun, 'run_test'); + assert.deepEqual(h.removedFiles, [{ path: '/tmp/staged-1', name: 'posts.zip' }]); }); it('reports cleanup failures without rejecting the completed job', async function () { @@ -776,10 +889,30 @@ describe('ContentCSVImporter', function () { }); await importer.importCSV({ filePath: '/tmp/posts.zip', fileName: 'posts.zip' }); - await h.jobs[0].job(); + await importer.handle(h.jobs[0]); assert.equal(h.store.get('run_test')?.status, 'complete'); - assert.equal(h.reported.at(-1), cleanupError); + assert.deepEqual(h.reported, [cleanupError, cleanupError]); + }); + + it('reports a staged-file cleanup failure without rejecting the completed job', async function () { + const h = harness(); + const cleanupError = new Error('staged cleanup failed'); + const importer = new ContentCSVImporter({ + ...h.deps, + fileStager: { + ...h.deps.fileStager, + remove: async () => { + throw cleanupError; + }, + }, + }); + + await importer.importCSV({ filePath: '/tmp/posts.csv', fileName: 'posts.csv' }); + await importer.handle(h.jobs[0]); + + assert.equal(h.store.get('run_test')?.status, 'complete'); + assert.deepEqual(h.reported, [cleanupError]); }); it('accepts a file exactly at the cap', async function () { @@ -1038,20 +1171,19 @@ describe('ContentCSVImporter', function () { ); }); - it("still reports a run-level failure that is nobody's row", async function () { + it("rejects a run-level failure that is nobody's row for the jobs service to report", async function () { const h = harness(); const failure = new Error('converter unavailable'); h.setHtmlToLexicalFactory(() => { throw failure; }); - await h.run(); + await assert.rejects(h.run(), failure); - assert.deepEqual(h.reported, [failure]); + assert.deepEqual(h.reported, []); assert.equal(h.store.get('run_test')?.status, 'failed'); assert.equal(h.store.get('run_test')?.failureReason, 'converter unavailable'); assert.ok(h.store.get('run_test')?.finishedAt instanceof Date); - sinon.assert.calledWithMatch(infoLog, /^\[Background Job\] content-import failed after \d+ms$/); }); it('stops the run for an unexpected row-processing failure', async function () { @@ -1064,9 +1196,9 @@ describe('ContentCSVImporter', function () { }); const h = harness([badRow, row('Never reached')]); - await h.run(); + await assert.rejects(h.run(), failure); - assert.deepEqual(h.reported, [failure]); + assert.deepEqual(h.reported, []); assert.equal(h.created.length, 0); assert.equal(h.store.get('run_test')?.status, 'failed'); assert.equal(h.store.get('run_test')?.failureReason, 'unexpected row failure'); @@ -1078,9 +1210,9 @@ describe('ContentCSVImporter', function () { throw {}; }); - await h.run(); + await assert.rejects(h.run(), () => true); - assert.deepEqual(h.reported, [{}]); + assert.deepEqual(h.reported, []); assert.equal(h.store.get('run_test')?.status, 'failed'); assert.equal(h.store.get('run_test')?.failureReason, 'Unknown error'); }); diff --git a/ghost/core/test/unit/server/services/content-import/import/staged-file.test.ts b/ghost/core/test/unit/server/services/content-import/import/staged-file.test.ts new file mode 100644 index 00000000000..26a1b011ae0 --- /dev/null +++ b/ghost/core/test/unit/server/services/content-import/import/staged-file.test.ts @@ -0,0 +1,56 @@ +import assert from 'node:assert/strict'; +import os from 'node:os'; +import path from 'node:path'; +import fs from 'fs-extra'; +import sinon from 'sinon'; +import { afterEach, beforeEach, describe, it } from 'vitest'; +import { createImportFileStager } from '../../../../../../core/server/services/content-import/import/staged-file'; + +describe('content import staged file', function () { + let sourceDirectory: string; + let sourcePath: string; + const stagedPaths: string[] = []; + + beforeEach(async function () { + sourceDirectory = await fs.mkdtemp(path.join(os.tmpdir(), 'content-import-stager-test-')); + sourcePath = path.join(sourceDirectory, 'upload'); + await fs.writeFile(sourcePath, 'title\nA staged post\n'); + }); + + afterEach(async function () { + sinon.restore(); + await Promise.all(stagedPaths.map((stagedPath) => fs.remove(stagedPath))); + stagedPaths.length = 0; + await fs.remove(sourceDirectory); + }); + + it('copies uploads to unique private files and removes them idempotently', async function () { + const stager = createImportFileStager(); + const first = await stager.stage({ filePath: sourcePath, fileName: 'posts.csv' }); + const second = await stager.stage({ filePath: sourcePath, fileName: 'posts.csv' }); + stagedPaths.push(first.path, second.path); + + assert.notEqual(first.path, second.path); + assert.equal(first.name, 'posts.csv'); + assert.equal(await fs.readFile(first.path, 'utf8'), 'title\nA staged post\n'); + assert.equal((await fs.stat(first.path)).mode & 0o777, 0o600); + + await stager.remove(first); + await stager.remove(first); + assert.equal(await fs.pathExists(first.path), false); + }); + + it('removes a partial staged file when securing it fails', async function () { + const stager = createImportFileStager(); + const remove = sinon.spy(fs, 'remove'); + sinon.stub(fs, 'chmod').rejects(new Error('chmod failed')); + + await assert.rejects( + stager.stage({ filePath: sourcePath, fileName: 'posts.csv' }), + /chmod failed/, + ); + + sinon.assert.calledOnce(remove); + assert.equal(await fs.pathExists(remove.firstCall.args[0]), false); + }); +}); diff --git a/ghost/core/test/unit/server/services/content-import/import/store.test.ts b/ghost/core/test/unit/server/services/content-import/import/store.test.ts index 4c9c1fcf19c..79c73f8989d 100644 --- a/ghost/core/test/unit/server/services/content-import/import/store.test.ts +++ b/ghost/core/test/unit/server/services/content-import/import/store.test.ts @@ -93,6 +93,35 @@ describe('ImportRunStore', function () { assert.equal(failed?.finishedAt, finishedAt); }); + it('settles immediately when no imports are running', async function () { + const store = new ImportRunStore(); + + await store.allSettled(); + }); + + it('settles waiters only after every import has finished its reporting and is released', async function () { + const store = new ImportRunStore(); + store.create('run_1', 1); + store.create('run_2', 1); + let settled = false; + const waiting = store.allSettled().then(() => { + settled = true; + }); + + store.finish('run_1'); + store.release('run_1'); + await Promise.resolve(); + assert.equal(settled, false); + + store.fail('run_2', 'failed'); + await Promise.resolve(); + assert.equal(settled, false); + + store.release('run_2'); + await waiting; + assert.equal(settled, true); + }); + it('keeps only the most recent finished runs', function () { const store = new ImportRunStore(); diff --git a/ghost/core/test/unit/server/services/content-import/index.test.ts b/ghost/core/test/unit/server/services/content-import/index.test.ts new file mode 100644 index 00000000000..52d16055c34 --- /dev/null +++ b/ghost/core/test/unit/server/services/content-import/index.test.ts @@ -0,0 +1,19 @@ +import assert from 'node:assert/strict'; +import { describe, it } from 'vitest'; +import ContentCSVImportJob from '../../../../../core/server/services/content-import/jobs/content-csv-import-job'; + +const contentImport = require('../../../../../core/server/services/content-import'); + +describe('content import service', function () { + it('fails loudly when a job is delivered before the service is initialised', async function () { + const job = new ContentCSVImportJob({ + importId: 'run_test', + file: { path: '/tmp/staged-import', name: 'posts.csv' }, + importTagNames: ['#Import 2026-01-01 10:30', '#Import Run run_test'], + emailRecipient: 'owner@example.com', + }); + + assert.throws(() => contentImport.handleJob(job), /Content import service used before init/); + await contentImport.allSettled(); + }); +}); 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 8717b7427ed..48c976f3206 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 @@ -5,6 +5,7 @@ import logging from '@tryghost/logging'; import { JobsService } from '../../../../../core/server/services/jobs-service/jobs-service'; 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'; const registerJobHandlers = require('../../../../../core/server/services/jobs-service/register-job-handlers').default; @@ -116,4 +117,20 @@ describe('register-job-handlers', function () { await externalMediaInlinerHandler(job); }, error); }); + + it('routes content CSV import jobs to the content import service', async function () { + const job = new ContentCSVImportJob({ + importId: 'run_test', + file: { path: '/tmp/staged-import', name: 'posts.zip' }, + mapping: { Headline: 'title' }, + importTagNames: ['#Import 2026-01-01 10:30', '#Import Run run_test'], + emailRecipient: 'owner@example.com', + }); + const contentImportHandler = handlerFor('content-csv-import'); + + await assert.rejects( + () => contentImportHandler(job), + /Content import service used before init/, + ); + }); });