refactor: extract shared PDF route handler to eliminate convert route duplication
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 19m19s
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 19m19s
- New src/utils/pdf-handler.ts with handlePdfRoute() helper - Handles: content-type validation, PDF option validation, slot acquire/release, error mapping, response headers - Refactored convert.ts from 388 to 233 lines (40% reduction) - 10 TDD tests for the new helper (RED→GREEN verified) - All 618 tests passing, zero tsc --noEmit errors
This commit is contained in:
parent
54316d45cf
commit
76b2179be9
3 changed files with 265 additions and 179 deletions
149
src/__tests__/pdf-handler.test.ts
Normal file
149
src/__tests__/pdf-handler.test.ts
Normal file
|
|
@ -0,0 +1,149 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import { Request, Response } from "express";
|
||||
|
||||
// We'll test the handlePdfRoute helper
|
||||
// RED phase: these tests should fail because pdf-handler.ts doesn't exist yet
|
||||
|
||||
describe("handlePdfRoute", () => {
|
||||
let mockReq: Partial<Request>;
|
||||
let mockRes: Partial<Response>;
|
||||
let statusFn: ReturnType<typeof vi.fn>;
|
||||
let jsonFn: ReturnType<typeof vi.fn>;
|
||||
let setHeaderFn: ReturnType<typeof vi.fn>;
|
||||
let sendFn: ReturnType<typeof vi.fn>;
|
||||
|
||||
beforeEach(() => {
|
||||
jsonFn = vi.fn();
|
||||
sendFn = vi.fn();
|
||||
setHeaderFn = vi.fn();
|
||||
statusFn = vi.fn().mockReturnValue({ json: jsonFn, send: sendFn, end: vi.fn() });
|
||||
mockRes = {
|
||||
status: statusFn,
|
||||
json: jsonFn,
|
||||
send: sendFn,
|
||||
setHeader: setHeaderFn,
|
||||
} as Partial<Response>;
|
||||
mockReq = {
|
||||
headers: { "content-type": "application/json" },
|
||||
body: {},
|
||||
acquirePdfSlot: undefined,
|
||||
releasePdfSlot: undefined,
|
||||
} as Partial<Request>;
|
||||
});
|
||||
|
||||
it("rejects non-JSON content type with 415", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
mockReq.headers = { "content-type": "text/plain" };
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => ({
|
||||
pdf: Buffer.from("test"),
|
||||
durationMs: 10,
|
||||
filename: "test.pdf",
|
||||
}));
|
||||
expect(statusFn).toHaveBeenCalledWith(415);
|
||||
expect(jsonFn).toHaveBeenCalledWith({ error: "Unsupported Content-Type. Use application/json." });
|
||||
});
|
||||
|
||||
it("rejects invalid PDF options with 400", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
mockReq.body = { scale: 99 };
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => ({
|
||||
pdf: Buffer.from("test"),
|
||||
durationMs: 10,
|
||||
filename: "test.pdf",
|
||||
}));
|
||||
expect(statusFn).toHaveBeenCalledWith(400);
|
||||
});
|
||||
|
||||
it("returns 503 on QUEUE_FULL error", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => {
|
||||
throw new Error("QUEUE_FULL");
|
||||
});
|
||||
expect(statusFn).toHaveBeenCalledWith(503);
|
||||
});
|
||||
|
||||
it("returns 504 on PDF_TIMEOUT error", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => {
|
||||
throw new Error("PDF_TIMEOUT");
|
||||
});
|
||||
expect(statusFn).toHaveBeenCalledWith(504);
|
||||
});
|
||||
|
||||
it("returns 500 on generic error", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => {
|
||||
throw new Error("Something broke");
|
||||
});
|
||||
expect(statusFn).toHaveBeenCalledWith(500);
|
||||
});
|
||||
|
||||
it("sets correct response headers on success", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
const pdfBuf = Buffer.from("fake-pdf");
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => ({
|
||||
pdf: pdfBuf,
|
||||
durationMs: 42,
|
||||
filename: "report.pdf",
|
||||
}));
|
||||
expect(setHeaderFn).toHaveBeenCalledWith("Content-Type", "application/pdf");
|
||||
expect(setHeaderFn).toHaveBeenCalledWith("Content-Disposition", 'inline; filename="report.pdf"');
|
||||
expect(setHeaderFn).toHaveBeenCalledWith("X-Render-Time", "42");
|
||||
expect(sendFn).toHaveBeenCalledWith(pdfBuf);
|
||||
});
|
||||
|
||||
it("acquires and releases PDF slot when available", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
const acquireFn = vi.fn();
|
||||
const releaseFn = vi.fn();
|
||||
mockReq.acquirePdfSlot = acquireFn;
|
||||
mockReq.releasePdfSlot = releaseFn;
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => ({
|
||||
pdf: Buffer.from("test"),
|
||||
durationMs: 10,
|
||||
filename: "test.pdf",
|
||||
}));
|
||||
expect(acquireFn).toHaveBeenCalledOnce();
|
||||
expect(releaseFn).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("releases PDF slot even on error", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
const acquireFn = vi.fn();
|
||||
const releaseFn = vi.fn();
|
||||
mockReq.acquirePdfSlot = acquireFn;
|
||||
mockReq.releasePdfSlot = releaseFn;
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => {
|
||||
throw new Error("boom");
|
||||
});
|
||||
expect(releaseFn).toHaveBeenCalledOnce();
|
||||
});
|
||||
|
||||
it("sanitizes filename with special characters", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async () => ({
|
||||
pdf: Buffer.from("test"),
|
||||
durationMs: 10,
|
||||
filename: 'file"with\nspecial.pdf',
|
||||
}));
|
||||
const dispositionCall = setHeaderFn.mock.calls.find(
|
||||
(c: unknown[]) => c[0] === "Content-Disposition"
|
||||
);
|
||||
expect(dispositionCall).toBeTruthy();
|
||||
// Quotes and newlines should be sanitized
|
||||
expect(dispositionCall![1]).not.toContain('"with');
|
||||
expect(dispositionCall![1]).not.toContain('\n');
|
||||
});
|
||||
|
||||
it("passes validated/sanitized options to renderFn", async () => {
|
||||
const { handlePdfRoute } = await import("../utils/pdf-handler.js");
|
||||
let receivedOptions: Record<string, unknown> | undefined;
|
||||
mockReq.body = { format: "a4", landscape: true };
|
||||
await handlePdfRoute(mockReq as Request, mockRes as Response, async (sanitizedOptions) => {
|
||||
receivedOptions = sanitizedOptions as Record<string, unknown>;
|
||||
return { pdf: Buffer.from("test"), durationMs: 10, filename: "test.pdf" };
|
||||
});
|
||||
expect(receivedOptions).toBeDefined();
|
||||
expect(receivedOptions!.format).toBe("A4"); // validatePdfOptions normalizes a4 → A4
|
||||
});
|
||||
});
|
||||
Loading…
Add table
Add a link
Reference in a new issue