From b1a09f7b3f094f54dcb415aac08ddde544a8d510 Mon Sep 17 00:00:00 2001 From: DocFast Backend Agent Date: Tue, 10 Mar 2026 11:06:34 +0100 Subject: [PATCH] refactor(demo): Use handlePdfRoute to reduce boilerplate - Refactored demo routes to use shared handlePdfRoute utility - Added handleDemoPdfRoute wrapper to preserve attachment disposition - Preserved watermark injection and demo.pdf default filename - Added comprehensive TDD tests for Content-Disposition behavior - Reduced demo.ts from 269 to 238 lines (31 lines removed) - All 628 tests pass including 6 new behavioral tests Fixes duplicated error handling, validation, and concurrency logic while maintaining existing demo route behavior. --- src/__tests__/demo.test.ts | 70 +++++++++++++++++++ src/routes/demo.ts | 137 ++++++++++++++----------------------- 2 files changed, 123 insertions(+), 84 deletions(-) diff --git a/src/__tests__/demo.test.ts b/src/__tests__/demo.test.ts index f6a3ec4..a742c4f 100644 --- a/src/__tests__/demo.test.ts +++ b/src/__tests__/demo.test.ts @@ -199,4 +199,74 @@ describe("POST /v1/demo/markdown", () => { expect(res.status).toBe(400); expect(res.body.error).toMatch(/format/); }); + + // NEW TDD TESTS - These should verify current behavior before refactoring + it("returns Content-Disposition attachment header", async () => { + const res = await request(app) + .post("/v1/demo/markdown") + .set("content-type", "application/json") + .send({ markdown: "# Hello" }); + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toMatch(/attachment/); + expect(res.headers["content-disposition"]).toMatch(/filename="demo\.pdf"/); + }); + + it("returns custom filename in attachment header", async () => { + const res = await request(app) + .post("/v1/demo/markdown") + .set("content-type", "application/json") + .send({ markdown: "# Hello", filename: "custom.pdf" }); + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toMatch(/attachment/); + expect(res.headers["content-disposition"]).toMatch(/filename="custom\.pdf"/); + }); + + it("injects watermark into HTML content", async () => { + const { renderPdf } = await import("../services/browser.js"); + const res = await request(app) + .post("/v1/demo/markdown") + .set("content-type", "application/json") + .send({ markdown: "# Hello" }); + expect(res.status).toBe(200); + + const calledHtml = vi.mocked(renderPdf).mock.calls[0][0]; + expect(calledHtml).toContain("Generated by DocFast — docfast.dev"); + expect(calledHtml).toContain("Upgrade to Pro for clean PDFs"); + expect(calledHtml).toContain("position:fixed;top:0;left:0;width:100%;height:100%"); // watermark overlay + }); + + // NEW TDD TESTS - These should verify current behavior before refactoring + it("returns Content-Disposition attachment header", async () => { + const res = await request(app) + .post("/v1/demo/html") + .set("content-type", "application/json") + .send({ html: "

Hello

" }); + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toMatch(/attachment/); + expect(res.headers["content-disposition"]).toMatch(/filename="demo\.pdf"/); + }); + + it("returns custom filename in attachment header", async () => { + const res = await request(app) + .post("/v1/demo/html") + .set("content-type", "application/json") + .send({ html: "

Hello

", filename: "custom.pdf" }); + expect(res.status).toBe(200); + expect(res.headers["content-disposition"]).toMatch(/attachment/); + expect(res.headers["content-disposition"]).toMatch(/filename="custom\.pdf"/); + }); + + it("injects watermark into HTML content", async () => { + const { renderPdf } = await import("../services/browser.js"); + const res = await request(app) + .post("/v1/demo/html") + .set("content-type", "application/json") + .send({ html: "

Hello

" }); + expect(res.status).toBe(200); + + const calledHtml = vi.mocked(renderPdf).mock.calls[0][0]; + expect(calledHtml).toContain("Generated by DocFast — docfast.dev"); + expect(calledHtml).toContain("Upgrade to Pro for clean PDFs"); + expect(calledHtml).toContain("position:fixed;top:0;left:0;width:100%;height:100%"); // watermark overlay + }); }); diff --git a/src/routes/demo.ts b/src/routes/demo.ts index da7ad63..6ff375d 100644 --- a/src/routes/demo.ts +++ b/src/routes/demo.ts @@ -2,10 +2,7 @@ import { Router, Request, Response } from "express"; import rateLimit from "express-rate-limit"; import { renderPdf } from "../services/browser.js"; import { markdownToHtml, wrapHtml } from "../services/markdown.js"; -import logger from "../services/logger.js"; -import { errorMessage } from "../utils/errors.js"; -import { sanitizeFilename } from "../utils/sanitize.js"; -import { validatePdfOptions } from "../utils/pdf-options.js"; +import { handlePdfRoute, type SanitizedPdfOptions } from "../utils/pdf-handler.js"; const router = Router(); @@ -22,6 +19,36 @@ function injectWatermark(html: string): string { return html + WATERMARK_HTML; } +/** + * Demo-specific PDF handler that uses handlePdfRoute but customizes: + * - Content-Disposition to "attachment" (not "inline") + * - Filename defaults to "demo.pdf" + */ +async function handleDemoPdfRoute( + req: Request, + res: Response, + renderFn: (sanitizedOptions: SanitizedPdfOptions) => Promise<{ pdf: Buffer; durationMs: number; filename: string } | null> +): Promise { + // Intercept the response to modify headers + const originalSend = res.send; + let intercepted = false; + + res.send = function(body: any) { + if (!intercepted && res.getHeader("Content-Type") === "application/pdf") { + intercepted = true; + const disposition = res.getHeader("Content-Disposition") as string; + if (disposition) { + // Change "inline" to "attachment" + const attachmentDisposition = disposition.replace(/^inline/, "attachment"); + res.setHeader("Content-Disposition", attachmentDisposition); + } + } + return originalSend.call(this, body); + }; + + await handlePdfRoute(req, res, renderFn); +} + // 5 requests per hour per IP const demoLimiter = rateLimit({ windowMs: 60 * 60 * 1000, @@ -99,29 +126,12 @@ interface DemoBody { * description: PDF generation timed out */ router.post("/html", async (req: Request, res: Response) => { - let slotAcquired = false; - try { - const ct = req.headers["content-type"] || ""; - if (!ct.includes("application/json")) { - res.status(415).json({ error: "Unsupported Content-Type. Use application/json." }); - return; - } - + await handleDemoPdfRoute(req, res, async (sanitizedOptions) => { const body: DemoBody = typeof req.body === "string" ? { html: req.body } : req.body; + if (!body.html) { res.status(400).json({ error: "Missing 'html' field" }); - return; - } - - const validation = validatePdfOptions(body); - if (!validation.valid) { - res.status(400).json({ error: validation.error }); - return; - } - - if (req.acquirePdfSlot) { - await req.acquirePdfSlot(); - slotAcquired = true; + return null; } const fullHtml = body.html.includes(" { printBackground: true, margin: { top: "0", right: "0", bottom: "0", left: "0" }, }; + const { pdf, durationMs } = await renderPdf(fullHtml, { ...defaultOpts, - ...validation.sanitized, + ...sanitizedOptions, }); - const filename = sanitizeFilename(body.filename || "demo.pdf"); - res.setHeader("Content-Type", "application/pdf"); - res.setHeader("Content-Disposition", `attachment; filename="${filename}"`); - res.setHeader("Content-Length", pdf.length); - res.setHeader("X-Render-Time", String(durationMs)); - res.send(pdf); - } catch (err: unknown) { - const msg = errorMessage(err); - if (msg === "QUEUE_FULL") { - res.status(503).json({ error: "Server busy. Please try again in a moment." }); - } else if (msg === "PDF_TIMEOUT") { - res.status(504).json({ error: "PDF generation timed out." }); - } else { - logger.error({ err }, "Demo HTML conversion failed"); - res.status(500).json({ error: "PDF generation failed." }); - } - } finally { - if (slotAcquired && req.releasePdfSlot) req.releasePdfSlot(); - } + return { + pdf, + durationMs, + filename: body.filename || "demo.pdf" + }; + }); }); /** @@ -206,29 +204,12 @@ router.post("/html", async (req: Request, res: Response) => { * description: PDF generation timed out */ router.post("/markdown", async (req: Request, res: Response) => { - let slotAcquired = false; - try { - const ct = req.headers["content-type"] || ""; - if (!ct.includes("application/json")) { - res.status(415).json({ error: "Unsupported Content-Type. Use application/json." }); - return; - } - + await handleDemoPdfRoute(req, res, async (sanitizedOptions) => { const body: DemoBody = typeof req.body === "string" ? { markdown: req.body } : req.body; + if (!body.markdown) { res.status(400).json({ error: "Missing 'markdown' field" }); - return; - } - - const validation = validatePdfOptions(body); - if (!validation.valid) { - res.status(400).json({ error: validation.error }); - return; - } - - if (req.acquirePdfSlot) { - await req.acquirePdfSlot(); - slotAcquired = true; + return null; } const htmlContent = markdownToHtml(body.markdown); @@ -240,30 +221,18 @@ router.post("/markdown", async (req: Request, res: Response) => { printBackground: true, margin: { top: "0", right: "0", bottom: "0", left: "0" }, }; + const { pdf, durationMs } = await renderPdf(fullHtml, { ...defaultOpts, - ...validation.sanitized, + ...sanitizedOptions, }); - const filename = sanitizeFilename(body.filename || "demo.pdf"); - res.setHeader("Content-Type", "application/pdf"); - res.setHeader("Content-Disposition", `attachment; filename="${filename}"`); - res.setHeader("Content-Length", pdf.length); - res.setHeader("X-Render-Time", String(durationMs)); - res.send(pdf); - } catch (err: unknown) { - const msg = errorMessage(err); - if (msg === "QUEUE_FULL") { - res.status(503).json({ error: "Server busy. Please try again in a moment." }); - } else if (msg === "PDF_TIMEOUT") { - res.status(504).json({ error: "PDF generation timed out." }); - } else { - logger.error({ err }, "Demo markdown conversion failed"); - res.status(500).json({ error: "PDF generation failed." }); - } - } finally { - if (slotAcquired && req.releasePdfSlot) req.releasePdfSlot(); - } + return { + pdf, + durationMs, + filename: body.filename || "demo.pdf" + }; + }); }); export { router as demoRouter };