refactor(demo): Use handlePdfRoute to reduce boilerplate
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 18m8s
All checks were successful
Build & Deploy to Staging / Build & Deploy to Staging (push) Successful in 18m8s
- 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.
This commit is contained in:
parent
7ae20ea280
commit
b1a09f7b3f
2 changed files with 123 additions and 84 deletions
|
|
@ -199,4 +199,74 @@ describe("POST /v1/demo/markdown", () => {
|
||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
expect(res.body.error).toMatch(/format/);
|
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: "<h1>Hello</h1>" });
|
||||||
|
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: "<h1>Hello</h1>", 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: "<h1>Hello</h1>" });
|
||||||
|
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
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
|
||||||
|
|
@ -2,10 +2,7 @@ import { Router, Request, Response } from "express";
|
||||||
import rateLimit from "express-rate-limit";
|
import rateLimit from "express-rate-limit";
|
||||||
import { renderPdf } from "../services/browser.js";
|
import { renderPdf } from "../services/browser.js";
|
||||||
import { markdownToHtml, wrapHtml } from "../services/markdown.js";
|
import { markdownToHtml, wrapHtml } from "../services/markdown.js";
|
||||||
import logger from "../services/logger.js";
|
import { handlePdfRoute, type SanitizedPdfOptions } from "../utils/pdf-handler.js";
|
||||||
import { errorMessage } from "../utils/errors.js";
|
|
||||||
import { sanitizeFilename } from "../utils/sanitize.js";
|
|
||||||
import { validatePdfOptions } from "../utils/pdf-options.js";
|
|
||||||
|
|
||||||
const router = Router();
|
const router = Router();
|
||||||
|
|
||||||
|
|
@ -22,6 +19,36 @@ function injectWatermark(html: string): string {
|
||||||
return html + WATERMARK_HTML;
|
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<void> {
|
||||||
|
// 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
|
// 5 requests per hour per IP
|
||||||
const demoLimiter = rateLimit({
|
const demoLimiter = rateLimit({
|
||||||
windowMs: 60 * 60 * 1000,
|
windowMs: 60 * 60 * 1000,
|
||||||
|
|
@ -99,29 +126,12 @@ interface DemoBody {
|
||||||
* description: PDF generation timed out
|
* description: PDF generation timed out
|
||||||
*/
|
*/
|
||||||
router.post("/html", async (req: Request, res: Response) => {
|
router.post("/html", async (req: Request, res: Response) => {
|
||||||
let slotAcquired = false;
|
await handleDemoPdfRoute(req, res, async (sanitizedOptions) => {
|
||||||
try {
|
|
||||||
const ct = req.headers["content-type"] || "";
|
|
||||||
if (!ct.includes("application/json")) {
|
|
||||||
res.status(415).json({ error: "Unsupported Content-Type. Use application/json." });
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
const body: DemoBody = typeof req.body === "string" ? { html: req.body } : req.body;
|
const body: DemoBody = typeof req.body === "string" ? { html: req.body } : req.body;
|
||||||
|
|
||||||
if (!body.html) {
|
if (!body.html) {
|
||||||
res.status(400).json({ error: "Missing 'html' field" });
|
res.status(400).json({ error: "Missing 'html' field" });
|
||||||
return;
|
return null;
|
||||||
}
|
|
||||||
|
|
||||||
const validation = validatePdfOptions(body);
|
|
||||||
if (!validation.valid) {
|
|
||||||
res.status(400).json({ error: validation.error });
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (req.acquirePdfSlot) {
|
|
||||||
await req.acquirePdfSlot();
|
|
||||||
slotAcquired = true;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const fullHtml = body.html.includes("<html")
|
const fullHtml = body.html.includes("<html")
|
||||||
|
|
@ -134,30 +144,18 @@ router.post("/html", async (req: Request, res: Response) => {
|
||||||
printBackground: true,
|
printBackground: true,
|
||||||
margin: { top: "0", right: "0", bottom: "0", left: "0" },
|
margin: { top: "0", right: "0", bottom: "0", left: "0" },
|
||||||
};
|
};
|
||||||
|
|
||||||
const { pdf, durationMs } = await renderPdf(fullHtml, {
|
const { pdf, durationMs } = await renderPdf(fullHtml, {
|
||||||
...defaultOpts,
|
...defaultOpts,
|
||||||
...validation.sanitized,
|
...sanitizedOptions,
|
||||||
});
|
});
|
||||||
|
|
||||||
const filename = sanitizeFilename(body.filename || "demo.pdf");
|
return {
|
||||||
res.setHeader("Content-Type", "application/pdf");
|
pdf,
|
||||||
res.setHeader("Content-Disposition", `attachment; filename="${filename}"`);
|
durationMs,
|
||||||
res.setHeader("Content-Length", pdf.length);
|
filename: body.filename || "demo.pdf"
|
||||||
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();
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|
@ -206,29 +204,12 @@ router.post("/html", async (req: Request, res: Response) => {
|
||||||
* description: PDF generation timed out
|
* description: PDF generation timed out
|
||||||
*/
|
*/
|
||||||
router.post("/markdown", async (req: Request, res: Response) => {
|
router.post("/markdown", async (req: Request, res: Response) => {
|
||||||
let slotAcquired = false;
|
await handleDemoPdfRoute(req, res, async (sanitizedOptions) => {
|
||||||
try {
|
|
||||||
const ct = req.headers["content-type"] || "";
|
|
||||||
if (!ct.includes("application/json")) {
|
|
||||||
res.status(415).json({ error: "Unsupported Content-Type. Use application/json." });
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
const body: DemoBody = typeof req.body === "string" ? { markdown: req.body } : req.body;
|
const body: DemoBody = typeof req.body === "string" ? { markdown: req.body } : req.body;
|
||||||
|
|
||||||
if (!body.markdown) {
|
if (!body.markdown) {
|
||||||
res.status(400).json({ error: "Missing 'markdown' field" });
|
res.status(400).json({ error: "Missing 'markdown' field" });
|
||||||
return;
|
return null;
|
||||||
}
|
|
||||||
|
|
||||||
const validation = validatePdfOptions(body);
|
|
||||||
if (!validation.valid) {
|
|
||||||
res.status(400).json({ error: validation.error });
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if (req.acquirePdfSlot) {
|
|
||||||
await req.acquirePdfSlot();
|
|
||||||
slotAcquired = true;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const htmlContent = markdownToHtml(body.markdown);
|
const htmlContent = markdownToHtml(body.markdown);
|
||||||
|
|
@ -240,30 +221,18 @@ router.post("/markdown", async (req: Request, res: Response) => {
|
||||||
printBackground: true,
|
printBackground: true,
|
||||||
margin: { top: "0", right: "0", bottom: "0", left: "0" },
|
margin: { top: "0", right: "0", bottom: "0", left: "0" },
|
||||||
};
|
};
|
||||||
|
|
||||||
const { pdf, durationMs } = await renderPdf(fullHtml, {
|
const { pdf, durationMs } = await renderPdf(fullHtml, {
|
||||||
...defaultOpts,
|
...defaultOpts,
|
||||||
...validation.sanitized,
|
...sanitizedOptions,
|
||||||
});
|
});
|
||||||
|
|
||||||
const filename = sanitizeFilename(body.filename || "demo.pdf");
|
return {
|
||||||
res.setHeader("Content-Type", "application/pdf");
|
pdf,
|
||||||
res.setHeader("Content-Disposition", `attachment; filename="${filename}"`);
|
durationMs,
|
||||||
res.setHeader("Content-Length", pdf.length);
|
filename: body.filename || "demo.pdf"
|
||||||
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();
|
|
||||||
}
|
|
||||||
});
|
});
|
||||||
|
|
||||||
export { router as demoRouter };
|
export { router as demoRouter };
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue