From 19470c8cd2295eaff3d4f893718a671c4539e1d5 Mon Sep 17 00:00:00 2001 From: Amruth Pillai Date: Sat, 4 Jul 2026 20:33:34 +0200 Subject: [PATCH] =?UTF-8?q?refactor(api,server):=20storage=20=E2=80=94=20s?= =?UTF-8?q?ync=20S3=20client,=20dead=20upload=20types,=20shared=20inferCon?= =?UTF-8?q?tentType?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - S3StorageService: replace async createClient/getClient/clientPromise chain with a synchronous constructor field; new S3Client() is synchronous (finding 7) - uploadFile: delete speculative screenshot/pdf upload types (never called); hardcode picture key and simplify to 3 lines (finding 6) - Export inferContentType from @reactive-resume/api/features/storage (finding 8) - uploads.ts: import inferContentType from storage instead of local duplicate; inline buildResponseHeaders into handleUpload (findings 11, 12) Claude-Session: https://claude.ai/code/session_012Bnvt1MghwHj4qQRxuQUGa --- apps/server/src/static/uploads.ts | 84 +++++------------- packages/api/src/features/storage/index.ts | 2 +- packages/api/src/features/storage/router.ts | 7 +- packages/api/src/features/storage/service.ts | 90 ++++---------------- 4 files changed, 37 insertions(+), 146 deletions(-) diff --git a/apps/server/src/static/uploads.ts b/apps/server/src/static/uploads.ts index 6020c8bb2..b3e37dfe6 100644 --- a/apps/server/src/static/uploads.ts +++ b/apps/server/src/static/uploads.ts @@ -1,6 +1,6 @@ import { createHash } from "node:crypto"; import { basename, extname, normalize } from "node:path"; -import { getStorageService } from "@reactive-resume/api/features/storage"; +import { getStorageService, inferContentType } from "@reactive-resume/api/features/storage"; import { env } from "@reactive-resume/env/server"; export async function handleUpload(request: Request) { @@ -18,41 +18,32 @@ export async function handleUpload(request: Request) { const filename = filePath.split("/").pop() ?? filePath; const ext = extname(filename).toLowerCase(); - const contentType = storedFile.contentType ?? inferContentTypeFromExtension(ext); + const contentType = storedFile.contentType ?? inferContentType(filename); const etag = createEtag(storedFile); if (isNotModified(request.headers, etag)) return makeNotModifiedResponse(etag); const shouldForceDownload = [".pdf"].includes(ext); - const headers = buildResponseHeaders({ - filename, - storedFile, - contentType, - etag, - shouldForceDownload, - }); - const buffer = toArrayBuffer(storedFile.data); + const headers = new Headers(); + headers.set("Content-Type", shouldForceDownload ? "application/octet-stream" : contentType); + headers.set("Content-Length", storedFile.size.toString()); - return new Response(buffer, { headers }); -} - -function inferContentTypeFromExtension(ext: string): string { - switch (ext) { - case ".webp": - return "image/webp"; - case ".png": - return "image/png"; - case ".jpg": - case ".jpeg": - return "image/jpeg"; - case ".gif": - return "image/gif"; - case ".pdf": - return "application/pdf"; - default: - return "application/octet-stream"; + if (shouldForceDownload) { + headers.set("Content-Disposition", `attachment; filename="${encodeURIComponent(basename(filename))}"`); } + + headers.set("Cache-Control", "public, max-age=31536000, immutable"); + headers.set("ETag", etag); + headers.set("X-Content-Type-Options", "nosniff"); + headers.set("X-Robots-Tag", "noindex, nofollow"); + headers.set("Cross-Origin-Resource-Policy", "same-site"); + headers.set("Referrer-Policy", "strict-origin-when-cross-origin"); + headers.set("X-Frame-Options", "DENY"); + headers.set("X-Download-Options", "noopen"); + headers.set("Access-Control-Allow-Origin", env.APP_URL); + + return new Response(toArrayBuffer(storedFile.data), { headers }); } function parseRouteParams(url: string): { userId: string | undefined; filePath: string | undefined } { @@ -101,43 +92,6 @@ function makeNotModifiedResponse(etag: string): Response { }); } -type BuildResponseHeaderArgs = { - filename: string; - storedFile: { size: number }; - contentType: string; - etag: string; - shouldForceDownload: boolean; -}; - -function buildResponseHeaders({ - filename, - storedFile, - contentType, - etag, - shouldForceDownload, -}: BuildResponseHeaderArgs): Headers { - const headers = new Headers(); - - headers.set("Content-Type", shouldForceDownload ? "application/octet-stream" : contentType); - headers.set("Content-Length", storedFile.size.toString()); - - if (shouldForceDownload) { - headers.set("Content-Disposition", `attachment; filename="${encodeURIComponent(basename(filename))}"`); - } - - headers.set("Cache-Control", "public, max-age=31536000, immutable"); - headers.set("ETag", etag); - headers.set("X-Content-Type-Options", "nosniff"); - headers.set("X-Robots-Tag", "noindex, nofollow"); - headers.set("Cross-Origin-Resource-Policy", "same-site"); - headers.set("Referrer-Policy", "strict-origin-when-cross-origin"); - headers.set("X-Frame-Options", "DENY"); - headers.set("X-Download-Options", "noopen"); - headers.set("Access-Control-Allow-Origin", env.APP_URL); - - return headers; -} - function toArrayBuffer(data: Uint8Array): ArrayBuffer { return data.byteOffset === 0 && data.byteLength === data.buffer.byteLength ? (data.buffer as ArrayBuffer) diff --git a/packages/api/src/features/storage/index.ts b/packages/api/src/features/storage/index.ts index e98168e41..054719129 100644 --- a/packages/api/src/features/storage/index.ts +++ b/packages/api/src/features/storage/index.ts @@ -1 +1 @@ -export { getStorageService } from "./service"; +export { getStorageService, inferContentType } from "./service"; diff --git a/packages/api/src/features/storage/router.ts b/packages/api/src/features/storage/router.ts index 167619f36..ece346467 100644 --- a/packages/api/src/features/storage/router.ts +++ b/packages/api/src/features/storage/router.ts @@ -56,12 +56,7 @@ export const storageRouter = { contentType = originalMimeType; } - const result = await uploadFile({ - userId: context.user.id, - data, - contentType, - type: "picture", - }); + const result = await uploadFile({ userId: context.user.id, data, contentType }); return { url: result.url, diff --git a/packages/api/src/features/storage/service.ts b/packages/api/src/features/storage/service.ts index 8fb22f46d..78fe27034 100644 --- a/packages/api/src/features/storage/service.ts +++ b/packages/api/src/features/storage/service.ts @@ -55,22 +55,11 @@ const DEFAULT_CONTENT_TYPE = "application/octet-stream"; const IMAGE_MIME_TYPES = ["image/gif", "image/png", "image/jpeg", "image/webp"]; -// Key builders for different upload types function buildPictureKey(userId: string): string { const timestamp = Date.now(); return `uploads/${userId}/pictures/${timestamp}.jpeg`; } -function buildScreenshotKey(userId: string, resumeId: string): string { - const timestamp = Date.now(); - return `uploads/${userId}/screenshots/${resumeId}/${timestamp}.jpeg`; -} - -function buildPdfKey(userId: string, resumeId: string): string { - const timestamp = Date.now(); - return `uploads/${userId}/pdfs/${resumeId}/${timestamp}.pdf`; -} - function buildPublicUrl(path: string): string { const normalizedPath = path.startsWith("/") ? path : `/${path}`; const apiPath = normalizedPath.startsWith("/api/") ? normalizedPath : `/api${normalizedPath}`; @@ -224,10 +213,7 @@ class LocalStorageService implements StorageService { class S3StorageService implements StorageService { private readonly bucket: string; - private readonly accessKeyId: string; - private readonly secretAccessKey: string; - private readonly endpoint: string | undefined; - private readonly clientPromise: Promise; + private readonly client: S3Client; constructor() { if (!env.S3_ACCESS_KEY_ID || !env.S3_SECRET_ACCESS_KEY || !env.S3_BUCKET) { @@ -235,38 +221,25 @@ class S3StorageService implements StorageService { } this.bucket = env.S3_BUCKET; - this.accessKeyId = env.S3_ACCESS_KEY_ID; - this.secretAccessKey = env.S3_SECRET_ACCESS_KEY; - this.endpoint = env.S3_ENDPOINT; - this.clientPromise = this.createClient(); - } - - private async createClient(): Promise { - return new S3Client({ + this.client = new S3Client({ region: env.S3_REGION, forcePathStyle: env.S3_FORCE_PATH_STYLE, - ...(this.endpoint ? { endpoint: this.endpoint } : {}), + ...(env.S3_ENDPOINT ? { endpoint: env.S3_ENDPOINT } : {}), credentials: { - accessKeyId: this.accessKeyId, - secretAccessKey: this.secretAccessKey, + accessKeyId: env.S3_ACCESS_KEY_ID, + secretAccessKey: env.S3_SECRET_ACCESS_KEY, }, }); } - private async getClient(): Promise { - return this.clientPromise; - } - async list(prefix: string): Promise { - const client = await this.getClient(); const command = new ListObjectsV2Command({ Bucket: this.bucket, Prefix: prefix }); - const response = await client.send(command); + const response = await this.client.send(command); if (!response.Contents) return []; return response.Contents.map((object) => object.Key ?? ""); } async write({ key, data, contentType, private: isPrivate }: StorageWriteInput): Promise { - const client = await this.getClient(); const command = new PutObjectCommand({ Bucket: this.bucket, Key: key, @@ -275,14 +248,13 @@ class S3StorageService implements StorageService { ContentType: contentType, }); - await client.send(command); + await this.client.send(command); } async read(key: string): Promise { try { - const client = await this.getClient(); const command = new GetObjectCommand({ Bucket: this.bucket, Key: key }); - const response = await client.send(command); + const response = await this.client.send(command); if (!response.Body) return null; const arrayBuffer = await response.Body.transformToByteArray(); @@ -301,13 +273,13 @@ class S3StorageService implements StorageService { async delete(keyOrPrefix: string): Promise { // Use list to find all matching keys (handles both single file and folder/prefix) - const [client, keys] = await Promise.all([this.getClient(), this.list(keyOrPrefix)]); + const keys = await this.list(keyOrPrefix); if (keys.length === 0) return false; // Delete all matching keys using Promise.allSettled const deleteCommands = keys.map((k) => new DeleteObjectCommand({ Bucket: this.bucket, Key: k })); - const results = await Promise.allSettled(deleteCommands.map((c) => client.send(c))); + const results = await Promise.allSettled(deleteCommands.map((c) => this.client.send(c))); // Return true if at least one deletion succeeded return results.some((r) => r.status === "fulfilled"); @@ -315,12 +287,11 @@ class S3StorageService implements StorageService { async healthcheck(): Promise { try { - const client = await this.getClient(); const putCommand = new PutObjectCommand({ Bucket: this.bucket, Key: "healthcheck", Body: "OK" }); - await client.send(putCommand); + await this.client.send(putCommand); const deleteCommand = new DeleteObjectCommand({ Bucket: this.bucket, Key: "healthcheck" }); - await client.send(deleteCommand); + await this.client.send(deleteCommand); return { type: "s3", @@ -355,15 +326,10 @@ export function getStorageService(): StorageService { return cachedService; } -// High-level upload types -type UploadType = "picture" | "screenshot" | "pdf"; - interface UploadFileInput { userId: string; data: Uint8Array; contentType: string; - type: UploadType; - resumeId?: string; } interface UploadFileResult { @@ -371,33 +337,9 @@ interface UploadFileResult { key: string; } +// ponytail: only "picture" uploads exist in production; screenshot/pdf types were speculative dead code export async function uploadFile(input: UploadFileInput): Promise { - const storageService = getStorageService(); - - let key: string; - - switch (input.type) { - case "picture": - key = buildPictureKey(input.userId); - break; - case "screenshot": - if (!input.resumeId) throw new Error("resumeId is required for screenshot uploads"); - key = buildScreenshotKey(input.userId, input.resumeId); - break; - case "pdf": - if (!input.resumeId) throw new Error("resumeId is required for pdf uploads"); - key = buildPdfKey(input.userId, input.resumeId); - break; - } - - await storageService.write({ - key, - data: input.data, - contentType: input.contentType, - }); - - return { - key, - url: buildPublicUrl(key), - }; + const key = buildPictureKey(input.userId); + await getStorageService().write({ key, data: input.data, contentType: input.contentType }); + return { key, url: buildPublicUrl(key) }; }