From cddb01f037f4b25a6c3fc0b9f4a0088c1a2167cf Mon Sep 17 00:00:00 2001 From: Amruth Pillai Date: Sat, 5 Sep 2026 09:34:52 -0700 Subject: [PATCH] fix(sharing): record public PDF download statistics (#3414) * fix(sharing): record public PDF download statistics * docs(api): explain download statistics access cookie --- .../resume/export/use-resume-export.test.tsx | 49 ++++++++++++++ .../resume/export/use-resume-export.ts | 7 ++ docs/spec.json | 40 +++++++++++ .../api/src/features/resume/service.test.ts | 66 +++++++++++++++++++ packages/api/src/features/resume/service.ts | 33 ++++++++++ .../src/features/resume/statistics.test.ts | 64 ++++++++++++++++++ .../api/src/features/resume/statistics.ts | 26 +++++++- .../api/src/middleware/rate-limit/index.ts | 10 +++ tests/e2e/specs/public-sharing.spec.ts | 25 ++++++- 9 files changed, 318 insertions(+), 2 deletions(-) create mode 100644 packages/api/src/features/resume/statistics.test.ts diff --git a/apps/web/src/features/resume/export/use-resume-export.test.tsx b/apps/web/src/features/resume/export/use-resume-export.test.tsx index 54d87d6f6..8140cfd72 100644 --- a/apps/web/src/features/resume/export/use-resume-export.test.tsx +++ b/apps/web/src/features/resume/export/use-resume-export.test.tsx @@ -11,6 +11,11 @@ const mocks = vi.hoisted(() => ({ downloadWithAnchor: vi.fn(), fetch: vi.fn(async (_input: string | URL) => new Response(new Blob(["server"], { type: "application/pdf" }))), toastAdd: vi.fn(() => "toast"), + recordDownload: vi.fn(async () => true), +})); + +vi.mock("@/libs/orpc/client", () => ({ + client: { resume: { statistics: { recordDownload: mocks.recordDownload } } }, })); vi.mock("@/features/resume/export/pdf-document", () => ({ @@ -34,6 +39,7 @@ beforeEach(() => { mocks.downloadWithAnchor.mockClear(); mocks.fetch.mockClear(); mocks.toastAdd.mockClear(); + mocks.recordDownload.mockClear(); vi.stubGlobal("fetch", mocks.fetch); }); @@ -60,6 +66,49 @@ describe("useResumeExport public PDF", () => { expect(mocks.fetch).toHaveBeenCalledTimes(1); const blob = mocks.downloadWithAnchor.mock.calls[0]?.[0] as Blob; expect(await blob.text()).toBe("server"); + expect(mocks.recordDownload).toHaveBeenCalledExactlyOnceWith({ username: "amruth", slug: "sample" }); + }); + + const publicResume = { username: "amruth", slug: "sample" }; + const resume = { name: "Sample", slug: "sample", data: sampleResumeData }; + + it("records a public download only after handing the PDF to the browser", async () => { + const { result } = renderHook(() => useResumeExport(resume, { publicResumePdf: { publicResume } })); + expect(mocks.recordDownload).not.toHaveBeenCalled(); + + await act(() => result.current.onDownloadPDF()); + + expect(mocks.recordDownload).toHaveBeenCalledExactlyOnceWith(publicResume); + expect(mocks.downloadWithAnchor.mock.invocationCallOrder[0]).toBeLessThan( + mocks.recordDownload.mock.invocationCallOrder[0] ?? 0, + ); + }); + + it("does not record exports from the builder", async () => { + const { result } = renderHook(() => useResumeExport(resume)); + await act(() => result.current.onDownloadPDF()); + expect(mocks.downloadWithAnchor).toHaveBeenCalledOnce(); + expect(mocks.recordDownload).not.toHaveBeenCalled(); + }); + + it("does not record a failed browser download", async () => { + mocks.downloadWithAnchor.mockImplementationOnce(() => { + throw new Error("download failed"); + }); + const { result } = renderHook(() => useResumeExport(resume, { publicResumePdf: { publicResume } })); + await act(() => result.current.onDownloadPDF()); + expect(mocks.recordDownload).not.toHaveBeenCalled(); + expect(mocks.toastAdd).toHaveBeenCalledWith(expect.objectContaining({ type: "error" })); + }); + + it("keeps a successful download successful when recording statistics fails", async () => { + mocks.recordDownload.mockRejectedValueOnce(new Error("statistics unavailable")); + const { result } = renderHook(() => useResumeExport(resume, { publicResumePdf: { publicResume } })); + await act(() => result.current.onDownloadPDF()); + expect(mocks.downloadWithAnchor).toHaveBeenCalledOnce(); + expect(mocks.recordDownload).toHaveBeenCalledOnce(); + expect(mocks.toastAdd).not.toHaveBeenCalledWith(expect.objectContaining({ type: "error" })); + expect(result.current.isExporting).toBe(false); }); it("does not download a PDF when the renderer rejects", async () => { diff --git a/apps/web/src/features/resume/export/use-resume-export.ts b/apps/web/src/features/resume/export/use-resume-export.ts index 05d9bfdc6..25fd1989a 100644 --- a/apps/web/src/features/resume/export/use-resume-export.ts +++ b/apps/web/src/features/resume/export/use-resume-export.ts @@ -10,6 +10,7 @@ import { buildMarkdown } from "@reactive-resume/resume/markdown"; import { toast } from "@reactive-resume/ui/components/toast"; import { downloadWithAnchor, generateFilename } from "@reactive-resume/utils/file"; import { resolvePublicResumePdfBlob } from "@/features/resume/public/public-pdf"; +import { client } from "@/libs/orpc/client"; import { createSectionTitleResolverForLocale } from "@/libs/resume/section-title-locale"; import { createResumePdfBlob } from "./pdf-document"; @@ -107,6 +108,12 @@ export function useResumeExport(resume: ExportableResume | undefined, exportOpti : undefined, ); downloadWithAnchor(blob, generateFilename(getTargetExportName(resume, target), "pdf")); + if (exportOptions.publicResumePdf) { + // Statistics are best effort and must not delay or fail a completed browser download. + void client.resume.statistics + .recordDownload(exportOptions.publicResumePdf.publicResume) + .catch(() => undefined); + } } catch { toast.add({ type: "error", description: t`Could not generate the PDF. Please try again.` }); } finally { diff --git a/docs/spec.json b/docs/spec.json index 614ef2579..7425350a5 100644 --- a/docs/spec.json +++ b/docs/spec.json @@ -28879,6 +28879,46 @@ } } }, + "/resumes/{username}/{slug}/statistics/download": { + "post": { + "operationId": "recordResumeDownload", + "summary": "Record a public resume PDF download", + "description": "Records a visitor's explicit PDF download after the browser starts saving the file. Requires access to the public resume. For password-protected resumes, first call verifyResumePassword (POST /resumes/{username}/{slug}/password/verify) with the password, then send the returned HttpOnly resume_access_ cookie with this request. A missing or invalid access cookie returns NEED_PASSWORD (HTTP 401); the cookie expires after 10 minutes. Owner downloads are excluded. Rate limited per resume and visitor.", + "tags": [ + "Resume Statistics" + ], + "parameters": [ + { + "name": "username", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "slug", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + } + ], + "responses": { + "200": { + "description": "The download event was accepted.", + "content": { + "application/json": { + "schema": { + "type": "boolean" + } + } + } + } + } + } + }, "/api/health": { "get": { "operationId": "getHealth", diff --git a/packages/api/src/features/resume/service.test.ts b/packages/api/src/features/resume/service.test.ts index 45ec47f0b..26a56277a 100644 --- a/packages/api/src/features/resume/service.test.ts +++ b/packages/api/src/features/resume/service.test.ts @@ -971,3 +971,69 @@ describe("statistics.increment", () => { expect(txInsert).toHaveBeenCalledTimes(2); }); }); + +describe("statistics.recordDownload", () => { + const input = { username: "owner", slug: "resume", requestHeaders: new Headers() }; + const publicResume = { id: "r1", userId: "u1", isPublic: true, passwordHash: null }; + const selectResume = (rows: unknown[]) => + dbMock.select.mockReturnValueOnce({ + from: () => ({ innerJoin: () => ({ where: () => Promise.resolve(rows) }) }), + }); + const captureWrites = () => { + const values = vi.fn((_input: unknown) => ({ onConflictDoUpdate: vi.fn(async () => undefined) })); + dbMock.transaction.mockImplementationOnce(async (callback: (tx: unknown) => Promise) => + callback({ insert: () => ({ values }) }), + ); + return values; + }; + + it.each([undefined, "another-user"])( + "counts a public visitor (%s) in totals and daily downloads without adding views", + async (currentUserId) => { + selectResume([publicResume]); + const values = captureWrites(); + await resumeService.statistics.recordDownload({ ...input, ...(currentUserId ? { currentUserId } : {}) }); + expect(values).toHaveBeenCalledTimes(2); + expect(values.mock.calls[0]?.[0]).toMatchObject({ resumeId: "r1", views: 0, downloads: 1 }); + expect(values.mock.calls[0]?.[0]).toHaveProperty("lastDownloadedAt"); + expect(values.mock.calls[0]?.[0]).toHaveProperty("lastViewedAt", undefined); + expect(values.mock.calls[1]?.[0]).toMatchObject({ + resumeId: "r1", + views: 0, + downloads: 1, + date: expect.stringMatching(/^\d{4}-\d{2}-\d{2}$/), + }); + }, + ); + + it("does not count the owner's own download", async () => { + selectResume([publicResume]); + await resumeService.statistics.recordDownload({ ...input, currentUserId: "u1" }); + expect(dbMock.transaction).not.toHaveBeenCalled(); + }); + + it.each([{ rows: [] }, { rows: [{ ...publicResume, isPublic: false }] }])( + "rejects unavailable resumes without recording a download", + async ({ rows }) => { + selectResume(rows); + await expect(resumeService.statistics.recordDownload(input)).rejects.toMatchObject({ code: "NOT_FOUND" }); + expect(dbMock.transaction).not.toHaveBeenCalled(); + }, + ); + + it("requires current password access before recording a download", async () => { + selectResume([{ ...publicResume, passwordHash: "hash" }]); + hasResumeAccessMock.mockReturnValueOnce(false); + await expect(resumeService.statistics.recordDownload(input)).rejects.toMatchObject({ code: "NEED_PASSWORD" }); + expect(hasResumeAccessMock).toHaveBeenCalledWith(input.requestHeaders, "r1", "hash"); + expect(dbMock.transaction).not.toHaveBeenCalled(); + }); + + it("records a visitor with valid password access", async () => { + selectResume([{ ...publicResume, passwordHash: "hash" }]); + hasResumeAccessMock.mockReturnValueOnce(true); + const values = captureWrites(); + await resumeService.statistics.recordDownload(input); + expect(values).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/api/src/features/resume/service.ts b/packages/api/src/features/resume/service.ts index 17b543dd8..673d567bf 100644 --- a/packages/api/src/features/resume/service.ts +++ b/packages/api/src/features/resume/service.ts @@ -210,6 +210,39 @@ const tags = { }; const statistics = { + recordDownload: async (input: { + username: string; + slug: string; + requestHeaders: Headers; + currentUserId?: string; + }): Promise => { + const [resume] = await db + .select({ + id: schema.resume.id, + userId: schema.resume.userId, + isPublic: schema.resume.isPublic, + passwordHash: schema.resume.password, + }) + .from(schema.resume) + .innerJoin(schema.user, eq(schema.resume.userId, schema.user.id)) + .where(and(eq(schema.resume.slug, input.slug), eq(schema.user.username, input.username))); + + if (!resume) throw new ORPCError("NOT_FOUND"); + const viewer = input.currentUserId ? { id: input.currentUserId } : null; + assertCanView(resume, viewer); + if (resume.passwordHash && !hasResumeAccess(input.requestHeaders, resume.id, resume.passwordHash)) { + throw new ORPCError("NEED_PASSWORD", { + status: 401, + data: { username: input.username, slug: input.slug }, + }); + } + + if (shouldCountForStatistics(resume, viewer)) { + await statistics.increment({ id: resume.id, downloads: true }); + } + return true; + }, + getById: async (input: { id: string; userId: string }) => { const [statistics] = await db .select({ diff --git a/packages/api/src/features/resume/statistics.test.ts b/packages/api/src/features/resume/statistics.test.ts new file mode 100644 index 000000000..68adf5c2d --- /dev/null +++ b/packages/api/src/features/resume/statistics.test.ts @@ -0,0 +1,64 @@ +import { beforeAll, beforeEach, describe, expect, it, vi } from "vitest"; +import { createRouterClient } from "@orpc/server"; + +const mocks = vi.hoisted(() => ({ recordDownload: vi.fn(async () => true), user: null as { id: string } | null })); + +vi.mock("../../context", async () => { + const { os } = await vi.importActual("@orpc/server"); + const procedure = os + .$context<{ reqHeaders: Headers; locale: "en-US" }>() + .use(({ context, next }) => next({ context: { ...context, user: mocks.user } })); + return { publicProcedure: procedure, protectedProcedure: procedure }; +}); +vi.mock("./service", () => ({ resumeService: { statistics: { recordDownload: mocks.recordDownload } } })); + +beforeAll(() => { + vi.stubEnv("NODE_ENV", "production"); +}); +beforeEach(() => { + mocks.recordDownload.mockClear(); +}); + +describe("public download statistics procedure", () => { + const makeClient = async (user: { id: string } | null = null, ip = "127.0.0.1") => { + const { resumeStatisticsRouter } = await import("./statistics"); + mocks.user = user; + const headers = new Headers({ "x-forwarded-for": ip }); + return { + client: createRouterClient(resumeStatisticsRouter, { context: { reqHeaders: headers, locale: "en-US" } }), + headers, + }; + }; + + it("passes anonymous access headers to the download service", async () => { + const { client, headers } = await makeClient(); + await expect(client.recordDownload({ username: "owner", slug: "anonymous" })).resolves.toBe(true); + expect(mocks.recordDownload).toHaveBeenCalledExactlyOnceWith({ + username: "owner", + slug: "anonymous", + requestHeaders: headers, + }); + }); + + it("passes authenticated identity for owner exclusion", async () => { + const { client, headers } = await makeClient({ id: "owner-id" }); + await client.recordDownload({ username: "owner", slug: "authenticated" }); + expect(mocks.recordDownload).toHaveBeenCalledExactlyOnceWith({ + username: "owner", + slug: "authenticated", + requestHeaders: headers, + currentUserId: "owner-id", + }); + }); + + it("rate limits repeated reports independently per resume and visitor", async () => { + const { client } = await makeClient(); + const input = { username: "owner", slug: "rate-limit" }; + for (let count = 0; count < 5; count++) await client.recordDownload(input); + await expect(client.recordDownload(input)).rejects.toMatchObject({ code: "TOO_MANY_REQUESTS" }); + expect(mocks.recordDownload).toHaveBeenCalledTimes(5); + await expect(client.recordDownload({ ...input, slug: "different-resume" })).resolves.toBe(true); + const anotherVisitor = await makeClient(null, "127.0.0.2"); + await expect(anotherVisitor.client.recordDownload(input)).resolves.toBe(true); + }); +}); diff --git a/packages/api/src/features/resume/statistics.ts b/packages/api/src/features/resume/statistics.ts index 64727716e..2268db6d7 100644 --- a/packages/api/src/features/resume/statistics.ts +++ b/packages/api/src/features/resume/statistics.ts @@ -1,8 +1,32 @@ import z from "zod"; -import { protectedProcedure } from "../../context"; +import { protectedProcedure, publicProcedure } from "../../context"; +import { resumeDto } from "../../dto/resume"; +import { resumeDownloadRateLimit } from "../../middleware/rate-limit"; import { resumeService } from "./service"; export const resumeStatisticsRouter = { + recordDownload: publicProcedure + .route({ + method: "POST", + path: "/resumes/{username}/{slug}/statistics/download", + tags: ["Resume Statistics"], + operationId: "recordResumeDownload", + summary: "Record a public resume PDF download", + description: + "Records a visitor's explicit PDF download after the browser starts saving the file. Requires access to the public resume. For password-protected resumes, first call verifyResumePassword (POST /resumes/{username}/{slug}/password/verify) with the password, then send the returned HttpOnly resume_access_ cookie with this request. A missing or invalid access cookie returns NEED_PASSWORD (HTTP 401); the cookie expires after 10 minutes. Owner downloads are excluded. Rate limited per resume and visitor.", + successDescription: "The download event was accepted.", + }) + .input(resumeDto.getBySlug.input) + .use(resumeDownloadRateLimit) + .output(z.boolean()) + .handler(({ context, input }) => + resumeService.statistics.recordDownload({ + ...input, + requestHeaders: context.reqHeaders, + ...(context.user?.id ? { currentUserId: context.user.id } : {}), + }), + ), + getById: protectedProcedure .route({ method: "GET", diff --git a/packages/api/src/middleware/rate-limit/index.ts b/packages/api/src/middleware/rate-limit/index.ts index 4ed5ebbe0..2304fea49 100644 --- a/packages/api/src/middleware/rate-limit/index.ts +++ b/packages/api/src/middleware/rate-limit/index.ts @@ -64,6 +64,7 @@ function getInputKeyPart(input: unknown): string { const resumePasswordLimiter = new MemoryRatelimiter(rateLimitConfig.orpc.resumePassword); const pdfLimiter = new MemoryRatelimiter(rateLimitConfig.orpc.pdfExport); +const resumeDownloadLimiter = new MemoryRatelimiter(rateLimitConfig.orpc.pdfExport); const aiLimiter = new MemoryRatelimiter(rateLimitConfig.orpc.aiRequest); const storageUploadLimiter = new MemoryRatelimiter(rateLimitConfig.orpc.storageUpload); const storageDeleteLimiter = new MemoryRatelimiter(rateLimitConfig.orpc.storageDelete); @@ -91,6 +92,15 @@ export const pdfExportRateLimit = createRatelimitMiddleware `pdf-export:${getUserKey(context)}:${input.id}`, }); +export const resumeDownloadRateLimit = createRatelimitMiddleware< + ContextWithHeaders, + { username: string; slug: string } +>({ + limiter: productionLimiter(resumeDownloadLimiter), + key: ({ context }, input) => + `resume-download:${input.username}:${input.slug}:${getUserKey(context)}:${getClientKey(context.reqHeaders)}`, +}); + export const aiRequestRateLimit = createRatelimitMiddleware({ limiter: productionLimiter(aiLimiter), key: ({ context }, input) => `ai-request:${getUserKey(context)}:${getInputKeyPart(input)}`, diff --git a/tests/e2e/specs/public-sharing.spec.ts b/tests/e2e/specs/public-sharing.spec.ts index c2e633b27..6ce3983fa 100644 --- a/tests/e2e/specs/public-sharing.spec.ts +++ b/tests/e2e/specs/public-sharing.spec.ts @@ -1,8 +1,16 @@ import { createSampleResumeFromDashboard, openSidebarSection } from "../fixtures/resume"; import { expect, test } from "../fixtures/test"; -test("publishes a resume and renders it for an anonymous visitor", async ({ browser, authPage: page }, testInfo) => { +test("counts a visitor's PDF download without counting the preview", async ({ browser, authPage: page }, testInfo) => { + test.setTimeout(60_000); await createSampleResumeFromDashboard(page, testInfo); + const resumeId = new URL(page.url()).pathname.split("/")[2]; + const statisticsUrl = `/api/openapi/resumes/${resumeId}/statistics`; + const readStatistics = async () => { + const response = await page.request.get(statisticsUrl); + expect(response.ok()).toBe(true); + return response.json(); + }; await openSidebarSection(page, "Sharing"); await page.getByRole("switch", { name: /Allow Public Access/ }).click(); @@ -15,7 +23,22 @@ test("publishes a resume and renders it for an anonymous visitor", async ({ brow try { await anonymous.goto(publicUrl); await expect(anonymous.getByRole("button", { name: "Download PDF" }).first()).toBeVisible(); + await expect.poll(readStatistics).toMatchObject({ views: 1, downloads: 0, lastDownloadedAt: null }); + + const downloaded = anonymous.waitForEvent("download"); + await anonymous.getByRole("button", { name: "Download PDF" }).first().click(); + const download = await downloaded; + expect(await download.failure()).toBeNull(); + const downloadPath = await download.path(); + if (!downloadPath) throw new Error("The browser did not save the PDF"); + expect((await readFile(downloadPath)).subarray(0, 5).toString()).toBe("%PDF-"); + await expect.poll(readStatistics).toMatchObject({ views: 1, downloads: 1, lastDownloadedAt: expect.any(String) }); + const daily = await page.request.get(`${statisticsUrl}/daily?days=1`); + expect(daily.ok()).toBe(true); + expect(await daily.json()).toEqual([{ date: expect.any(String), views: 1, downloads: 1 }]); } finally { await anonymous.close(); } }); + +import { readFile } from "node:fs/promises";