diff --git a/apps/web/modules/core/rate-limit/rate-limit-configs.test.ts b/apps/web/modules/core/rate-limit/rate-limit-configs.test.ts index eef3e49033..77c0d86f3e 100644 --- a/apps/web/modules/core/rate-limit/rate-limit-configs.test.ts +++ b/apps/web/modules/core/rate-limit/rate-limit-configs.test.ts @@ -132,6 +132,8 @@ describe("rateLimitConfigs", () => { { config: rateLimitConfigs.api.client, identifier: "client-api-key" }, { config: rateLimitConfigs.api.syncUserIdentification, identifier: "sync-user-id" }, { config: rateLimitConfigs.actions.emailUpdate, identifier: "user-profile" }, + { config: rateLimitConfigs.storage.upload, identifier: "storage-upload" }, + { config: rateLimitConfigs.storage.delete, identifier: "storage-delete" }, ]; for (const { config, identifier } of testCases) { @@ -175,5 +177,23 @@ describe("rateLimitConfigs", () => { expect(exceededResult.data.allowed).toBe(false); } }); + + test("should properly configure storage upload rate limit", async () => { + const config = rateLimitConfigs.storage.upload; + + // Verify configuration values + expect(config.interval).toBe(60); // 1 minute + expect(config.allowedPerInterval).toBe(5); // 5 requests per minute + expect(config.namespace).toBe("storage:upload"); + }); + + test("should properly configure storage delete rate limit", async () => { + const config = rateLimitConfigs.storage.delete; + + // Verify configuration values + expect(config.interval).toBe(60); // 1 minute + expect(config.allowedPerInterval).toBe(5); // 5 requests per minute + expect(config.namespace).toBe("storage:delete"); + }); }); }); diff --git a/apps/web/modules/core/rate-limit/rate-limit-configs.ts b/apps/web/modules/core/rate-limit/rate-limit-configs.ts index 69639057b1..5866e5f45b 100644 --- a/apps/web/modules/core/rate-limit/rate-limit-configs.ts +++ b/apps/web/modules/core/rate-limit/rate-limit-configs.ts @@ -32,7 +32,6 @@ export const rateLimitConfigs = { storage: { upload: { interval: 60, allowedPerInterval: 5, namespace: "storage:upload" }, // 5 per minute - download: { interval: 60, allowedPerInterval: 5, namespace: "storage:download" }, // 5 per minute delete: { interval: 60, allowedPerInterval: 5, namespace: "storage:delete" }, // 5 per minute }, }; diff --git a/packages/storage/src/client.test.ts b/packages/storage/src/client.test.ts index 1edf92d1d4..d90aaa6bda 100644 --- a/packages/storage/src/client.test.ts +++ b/packages/storage/src/client.test.ts @@ -268,4 +268,72 @@ describe("client.ts", () => { expect(result).toBeUndefined(); }); }); + + describe("getCachedS3Client (singleton)", () => { + test("returns the same instance on multiple calls and constructs once", async () => { + vi.doMock("./constants", () => ({ + ...mockConstants, + })); + + const { getCachedS3Client } = await import("./client"); + const typedGetCachedS3Client = getCachedS3Client as unknown as () => S3Client; + + const first = typedGetCachedS3Client(); + const second = typedGetCachedS3Client(); + + expect(first).toBeDefined(); + expect(second).toBeDefined(); + expect(first).toBe(second); + expect(mockS3Client).toHaveBeenCalledTimes(1); + }); + + test("returns undefined when env is invalid and does not construct client", async () => { + vi.doMock("./constants", () => ({ + ...mockConstants, + S3_ACCESS_KEY: undefined, + S3_SECRET_KEY: undefined, + })); + + const { getCachedS3Client } = await import("./client"); + const typedGetCachedS3Client = getCachedS3Client as unknown as () => S3Client; + + const client = typedGetCachedS3Client(); + expect(client).toBeUndefined(); + expect(mockS3Client).not.toHaveBeenCalled(); + }); + + test("createS3Client uses cached instance when available", async () => { + vi.doMock("./constants", () => ({ + ...mockConstants, + })); + + const { getCachedS3Client, createS3Client } = await import("./client"); + const typedGetCachedS3Client = getCachedS3Client as unknown as () => S3Client; + + const cached = typedGetCachedS3Client(); + const created = createS3Client(); + + expect(cached).toBeDefined(); + expect(created).toBe(cached); + expect(mockS3Client).toHaveBeenCalledTimes(1); + }); + + test("createS3Client returns provided client even if cache exists", async () => { + vi.doMock("./constants", () => ({ + ...mockConstants, + })); + + const { getCachedS3Client, createS3Client } = await import("./client"); + const typedGetCachedS3Client = getCachedS3Client as unknown as () => S3Client; + const cached = typedGetCachedS3Client(); + expect(cached).toBeDefined(); + + const injected = new S3Client({}); + const result = createS3Client(injected); + + expect(result).toBe(injected); + // One construction for cached, one for injected + expect(mockS3Client).toHaveBeenCalledTimes(2); + }); + }); }); diff --git a/packages/storage/src/client.ts b/packages/storage/src/client.ts index c415dab212..7b7268cf12 100644 --- a/packages/storage/src/client.ts +++ b/packages/storage/src/client.ts @@ -10,6 +10,9 @@ import { S3_SECRET_KEY, } from "./constants"; +// Cached singleton instance of S3Client +let cachedS3Client: S3Client | undefined; + /** * Create an S3 client from environment variables * @returns A Result containing the S3 client or an error: S3CredentialsError | UnknownError @@ -39,18 +42,28 @@ export const createS3ClientFromEnv = (): Result => { } }; +/** + * Get a cached singleton S3 client instance. Lazily initializes from env on first successful call. + * Subsequent calls return the same instance. + */ +export const getCachedS3Client = (): S3Client | undefined => { + if (!cachedS3Client) { + const result = createS3ClientFromEnv(); + if (result.ok) { + cachedS3Client = result.data; + } + } + return cachedS3Client; +}; + /** * Create an S3 client from an existing client or from environment variables * @param s3Client - An existing S3 client * @returns An S3 client or undefined if the S3 credentials are not set in the environment variables or if there is an error creating the client */ export const createS3Client = (s3Client?: S3Client): S3Client | undefined => { - if (!s3Client) { - const result = createS3ClientFromEnv(); - if (result.ok) return result.data; - - return undefined; + if (s3Client) { + return s3Client; } - - return s3Client; + return getCachedS3Client(); };