diff --git a/apps/cyberstorm-remix/app/entry.client.tsx b/apps/cyberstorm-remix/app/entry.client.tsx index b1d1fc601..e66019b0b 100644 --- a/apps/cyberstorm-remix/app/entry.client.tsx +++ b/apps/cyberstorm-remix/app/entry.client.tsx @@ -4,8 +4,12 @@ import { hydrateRoot } from "react-dom/client"; import { useLocation, useMatches } from "react-router"; import { HydratedRouter } from "react-router/dom"; -import { getPublicEnvVariables } from "cyberstorm/security/publicEnvVariables"; +import { + getPublicEnvVariables, + getSessionTools, +} from "cyberstorm/security/publicEnvVariables"; import { denyUrls } from "cyberstorm/utils/sentry"; +import { initializeClientDapper } from "cyberstorm/utils/dapperSingleton"; const publicEnvVariables = getPublicEnvVariables([ "VITE_SITE_URL", @@ -13,6 +17,7 @@ const publicEnvVariables = getPublicEnvVariables([ "VITE_API_URL", "VITE_AUTH_BASE_URL", "VITE_CLIENT_SENTRY_DSN", + "VITE_COOKIE_DOMAIN", ]); Sentry.init({ @@ -69,6 +74,16 @@ Sentry.init({ denyUrls, }); +try { + const sessionTools = getSessionTools(); + + initializeClientDapper(() => + sessionTools.getConfig(publicEnvVariables.VITE_API_URL) + ); +} catch (error) { + Sentry.captureException(error); +} + startTransition(() => { hydrateRoot( document, diff --git a/apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts b/apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts index e0960ddea..a0358d0c4 100644 --- a/apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts +++ b/apps/cyberstorm-remix/cyberstorm/security/publicEnvVariables.ts @@ -1,5 +1,5 @@ import { getSessionContext } from "@thunderstore/ts-api-react/src/SessionContext"; -import { isRecord } from "cyberstorm/utils/typeChecks"; +import { isRecord } from "../utils/typeChecks"; export type publicEnvVariablesKeys = | "SITE_URL" @@ -57,7 +57,7 @@ export function getSessionTools() { !publicEnvVariables.VITE_COOKIE_DOMAIN ) { throw new Error( - "Enviroment variables did not load correctly, please hard refresh page" + "Environment variables did not load correctly, please hard refresh page" ); } return getSessionContext( diff --git a/apps/cyberstorm-remix/cyberstorm/utils/__tests__/dapperSingleton.test.ts b/apps/cyberstorm-remix/cyberstorm/utils/__tests__/dapperSingleton.test.ts new file mode 100644 index 000000000..15b138c32 --- /dev/null +++ b/apps/cyberstorm-remix/cyberstorm/utils/__tests__/dapperSingleton.test.ts @@ -0,0 +1,198 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"; +import { + initializeClientDapper, + getClientDapper, + getDapperForRequest, + resetDapperSingletonForTest, +} from "../dapperSingleton"; +import { deduplicatePromiseForRequest } from "../requestCache"; +import { DapperTs } from "@thunderstore/dapper-ts"; +import * as publicEnvVariables from "../../security/publicEnvVariables"; +import type { Community } from "../../../../../packages/thunderstore-api/src"; + +// Mock getSessionTools +vi.mock("../../security/publicEnvVariables", () => ({ + getSessionTools: vi.fn().mockReturnValue({ + getConfig: vi.fn().mockReturnValue({ + apiHost: "http://localhost", + sessionId: "test-session", + }), + }), +})); + +describe("dapperSingleton", () => { + beforeEach(() => { + // Reset window.Dapper + if (typeof window !== "undefined") { + // @ts-expect-error Dapper is not optional on Window + delete window.Dapper; + } + resetDapperSingletonForTest(); + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + describe("initializeClientDapper", () => { + it("initializes window.Dapper if it does not exist", () => { + initializeClientDapper(); + expect(window.Dapper).toBeDefined(); + expect(window.Dapper).toBeInstanceOf(DapperTs); + }); + + it("uses provided factory if supplied", () => { + const factory = vi.fn().mockReturnValue({ apiHost: "custom" }); + initializeClientDapper(factory); + expect(window.Dapper).toBeDefined(); + expect(window.Dapper.config()).toEqual({ apiHost: "custom" }); + expect(factory).toHaveBeenCalled(); + }); + + it("updates existing window.Dapper config if called again with factory", () => { + // First initialization + initializeClientDapper(); + const originalDapper = window.Dapper; + expect(originalDapper).toBeDefined(); + + // Second initialization with new factory + const newFactory = vi.fn().mockReturnValue({ apiHost: "updated" }); + initializeClientDapper(newFactory); + + expect(window.Dapper).toBe(originalDapper); // Should be same instance + expect(window.Dapper.config()).toEqual({ apiHost: "updated" }); + }); + + it("resolves config factory from session tools if no factory provided", () => { + initializeClientDapper(); + expect(publicEnvVariables.getSessionTools).toHaveBeenCalled(); + }); + }); + + describe("getClientDapper", () => { + it("returns window.Dapper if it exists", () => { + initializeClientDapper(); + const dapper = window.Dapper; + expect(getClientDapper()).toBe(dapper); + }); + + it("initializes and returns window.Dapper if it does not exist", () => { + expect(window.Dapper).toBeUndefined(); + const dapper = getClientDapper(); + expect(dapper).toBeDefined(); + expect(window.Dapper).toBe(dapper); + }); + }); + + describe("getDapperForRequest", () => { + it("returns client dapper if no request is provided", () => { + initializeClientDapper(); + const dapper = getDapperForRequest(); + expect(dapper).toBe(window.Dapper); + }); + + it("returns a proxy if request is provided", () => { + initializeClientDapper(); + const request = new Request("http://localhost"); + const dapper = getDapperForRequest(request); + expect(dapper).not.toBe(window.Dapper); + // It should be a proxy + expect(dapper).toBeInstanceOf(DapperTs); + }); + + it("caches the proxy for the same request", () => { + initializeClientDapper(); + const request = new Request("http://localhost"); + const dapper1 = getDapperForRequest(request); + const dapper2 = getDapperForRequest(request); + expect(dapper1).toBe(dapper2); + }); + + it("creates different proxies for different requests", () => { + initializeClientDapper(); + const request1 = new Request("http://localhost"); + const request2 = new Request("http://localhost"); + const dapper1 = getDapperForRequest(request1); + const dapper2 = getDapperForRequest(request2); + expect(dapper1).not.toBe(dapper2); + }); + + it("intercepts 'get' methods and caches promises", async () => { + initializeClientDapper(); + const request = new Request("http://localhost"); + const dapper = getDapperForRequest(request); + + // Mock the underlying method on window.Dapper + const mockGetCommunities = vi + .spyOn(window.Dapper, "getCommunities") + .mockResolvedValue({ count: 0, results: [], hasMore: false }); + + const result1 = await dapper.getCommunities(); + const result2 = await dapper.getCommunities(); + + expect(result1).toEqual({ count: 0, results: [], hasMore: false }); + expect(result2).toEqual({ count: 0, results: [], hasMore: false }); + + // Should be called only once due to caching + expect(mockGetCommunities).toHaveBeenCalledTimes(1); + }); + + it("does not intercept non-'get' methods", async () => { + initializeClientDapper(); + const request = new Request("http://localhost"); + const dapper = getDapperForRequest(request); + + // Mock a non-get method + // postTeamCreate is a good candidate + const mockPostTeamCreate = vi + .spyOn(window.Dapper, "postTeamCreate") + .mockResolvedValue({ + identifier: 1, + name: "test", + donation_link: null, + }); + + await dapper.postTeamCreate("test"); + await dapper.postTeamCreate("test"); + + // Should be called twice (no caching) + expect(mockPostTeamCreate).toHaveBeenCalledTimes(2); + }); + + it("shares cache between proxy calls and manual deduplicatePromiseForRequest calls", async () => { + initializeClientDapper(); + const request = new Request("http://localhost"); + const dapper = getDapperForRequest(request); + + // Mock the underlying method on window.Dapper + const mockGetCommunity = vi + .spyOn(window.Dapper, "getCommunity") + .mockResolvedValue({ + identifier: "1", + name: "Test Community", + } as Community); + + // 1. Call via proxy + const dapperResult = await dapper.getCommunity("1"); + + // 2. Call manually with same key and args + const manualFunc = vi.fn().mockResolvedValue("manual result"); + const manualResult = await deduplicatePromiseForRequest( + "getCommunity", + manualFunc, + ["1"], + request + ); + + // Assertions + expect(dapperResult).toEqual({ identifier: "1", name: "Test Community" }); + // Should return the cached result from the first call, NOT "manual result" + expect(manualResult).toBe(dapperResult); + // The manual function should NOT have been called + expect(manualFunc).not.toHaveBeenCalled(); + // The underlying dapper method should have been called once + expect(mockGetCommunity).toHaveBeenCalledTimes(1); + }); + }); +}); diff --git a/apps/cyberstorm-remix/cyberstorm/utils/__tests__/requestCache.test.ts b/apps/cyberstorm-remix/cyberstorm/utils/__tests__/requestCache.test.ts new file mode 100644 index 000000000..319925e4b --- /dev/null +++ b/apps/cyberstorm-remix/cyberstorm/utils/__tests__/requestCache.test.ts @@ -0,0 +1,169 @@ +import { describe, it, expect, vi } from "vitest"; +import { deduplicatePromiseForRequest } from "../requestCache"; + +describe("requestCache", () => { + it("caches result per request", async () => { + const mockFn = vi.fn().mockResolvedValue("result"); + const request = new Request("http://localhost"); + + const result1 = await deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + const result2 = await deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + + expect(result1).toBe("result"); + expect(result2).toBe("result"); + expect(mockFn).toHaveBeenCalledTimes(1); + }); + + it("returns the exact same promise instance", async () => { + const mockFn = vi.fn().mockResolvedValue("result"); + const request = new Request("http://localhost"); + + const promise1 = deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + const promise2 = deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + + expect(promise1).toBe(promise2); + }); + + it("distinguishes between calls with different arguments", async () => { + const mockFn = vi.fn().mockImplementation(async (arg) => arg); + const request = new Request("http://localhost"); + + const result1 = await deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + const result2 = await deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg2"], + request + ); + + expect(result1).toBe("arg1"); + expect(result2).toBe("arg2"); + expect(mockFn).toHaveBeenCalledTimes(2); + }); + + it("removes rejected promises from cache", async () => { + const mockFn = vi + .fn() + .mockRejectedValueOnce(new Error("fail")) + .mockResolvedValueOnce("success"); + const request = new Request("http://localhost"); + + // First call fails + await expect( + deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request) + ).rejects.toThrow("fail"); + + // Second call should retry and succeed + const result = await deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + + expect(result).toBe("success"); + expect(mockFn).toHaveBeenCalledTimes(2); + }); + + it("does not share cache between requests", async () => { + const mockFn = vi.fn().mockResolvedValue("result"); + const request1 = new Request("http://localhost"); + const request2 = new Request("http://localhost"); + + await deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request1); + await deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request2); + + expect(mockFn).toHaveBeenCalledTimes(2); + }); + + it("clears request cache on abort", async () => { + const mockFn = vi.fn().mockResolvedValue("result"); + const controller = new AbortController(); + const request = new Request("http://localhost", { + signal: controller.signal, + }); + + await deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request); + + // Abort the request + controller.abort(); + + await deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request); + + expect(mockFn).toHaveBeenCalledTimes(2); + }); + + it("should behave consistently if request is aborted BEFORE first use", async () => { + const mockFn = vi.fn().mockResolvedValue("result"); + const controller = new AbortController(); + const request = new Request("http://localhost", { + signal: controller.signal, + }); + + // Abort before first use + controller.abort(); + + // First use: should NOT cache (or cache is cleared immediately) + await deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request); + expect(mockFn).toHaveBeenCalledTimes(1); + + // Second use: should be re-executed because cache was not persisted/cleared + await deduplicatePromiseForRequest("mockFn", mockFn, ["arg1"], request); + + expect(mockFn).toHaveBeenCalledTimes(2); + }); + + it("removes from cache after timeout", async () => { + vi.useFakeTimers(); + const mockFn = vi.fn().mockResolvedValue("result"); + const request = new Request("http://localhost"); + + const promise = deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + + // Advance time by 60 seconds + 1ms + vi.advanceTimersByTime(60001); + + // The next call should create a new promise because the old one expired + const promise2 = deduplicatePromiseForRequest( + "mockFn", + mockFn, + ["arg1"], + request + ); + + expect(promise).not.toBe(promise2); + expect(mockFn).toHaveBeenCalledTimes(2); + + vi.useRealTimers(); + }); +}); diff --git a/apps/cyberstorm-remix/cyberstorm/utils/dapperSingleton.ts b/apps/cyberstorm-remix/cyberstorm/utils/dapperSingleton.ts new file mode 100644 index 000000000..0d1c0f735 --- /dev/null +++ b/apps/cyberstorm-remix/cyberstorm/utils/dapperSingleton.ts @@ -0,0 +1,107 @@ +import { DapperTs } from "@thunderstore/dapper-ts"; +import { type RequestConfig } from "@thunderstore/thunderstore-api"; +import { getSessionTools } from "../security/publicEnvVariables"; +import { deduplicatePromiseForRequest } from "./requestCache"; + +type ConfigFactory = () => RequestConfig; + +let currentConfigFactory: ConfigFactory | undefined; +let requestScopedProxies = new WeakMap(); + +function resolveConfigFactory(): ConfigFactory { + if (currentConfigFactory) { + return currentConfigFactory; + } + + const tools = getSessionTools(); + currentConfigFactory = () => tools.getConfig(); + return currentConfigFactory; +} + +function updateDapperConfig(factory: ConfigFactory) { + currentConfigFactory = factory; + if (typeof window !== "undefined" && window.Dapper) { + window.Dapper.config = factory; + } +} + +export function initializeClientDapper(factory?: ConfigFactory) { + if (typeof window === "undefined") { + return; + } + + if (factory) { + updateDapperConfig(factory); + } + + if (!window.Dapper) { + const resolvedFactory = resolveConfigFactory(); + window.Dapper = new DapperTs(resolvedFactory); + } +} + +export function getClientDapper(): DapperTs { + if (typeof window === "undefined") { + throw new Error("getClientDapper can only run in the browser"); + } + + if (!window.Dapper) { + initializeClientDapper(); + } + + return window.Dapper; +} + +/** + * Returns a Dapper instance scoped to the request. + * If a request is provided, it returns a proxy that deduplicates "get" requests. + * If no request is provided (e.g. client-side navigation), it returns the global client Dapper instance. + * + * @param request - The Request object to scope the Dapper instance to. + */ +export function getDapperForRequest(request?: Request): DapperTs { + if (!request) { + return getClientDapper(); + } + + let proxy = requestScopedProxies.get(request); + if (proxy) { + return proxy; + } + + const baseDapper = getClientDapper(); + const handler: ProxyHandler = { + get(target, propertyName, receiver) { + const value = Reflect.get(target, propertyName, receiver); + if (typeof value !== "function") { + return value; + } + + // We only cache methods starting with "get" to avoid caching mutations or other side effects. + if (typeof propertyName !== "string" || !propertyName.startsWith("get")) { + return value.bind(target); + } + + return (...args: unknown[]) => + deduplicatePromiseForRequest( + propertyName, + (...innerArgs: unknown[]) => + (value as (...i: unknown[]) => Promise).apply( + target, + innerArgs + ), + args as unknown[], + request + ); + }, + }; + + proxy = new Proxy(baseDapper, handler) as DapperTs; + requestScopedProxies.set(request, proxy); + return proxy; +} + +export function resetDapperSingletonForTest() { + currentConfigFactory = undefined; + requestScopedProxies = new WeakMap(); +} diff --git a/apps/cyberstorm-remix/cyberstorm/utils/requestCache.ts b/apps/cyberstorm-remix/cyberstorm/utils/requestCache.ts new file mode 100644 index 000000000..ecaf60f71 --- /dev/null +++ b/apps/cyberstorm-remix/cyberstorm/utils/requestCache.ts @@ -0,0 +1,114 @@ +import isEqual from "lodash/isEqual"; + +type CacheEntry = { + funcName: string; + inputs: unknown[]; + promise: Promise; +}; + +const requestScopedCaches = new WeakMap(); + +function getCache(request: Request) { + if (request.signal?.aborted) { + requestScopedCaches.delete(request); + return []; + } + + let cache = requestScopedCaches.get(request); + if (!cache) { + cache = []; + requestScopedCaches.set(request, cache); + + const signal = request.signal; + if (signal) { + signal.addEventListener( + "abort", + () => { + requestScopedCaches.delete(request); + }, + { once: true } + ); + } + } + + return cache; +} + +/** + * Deduplicates concurrent requests for the same function and arguments within the scope of a single Request. + * + * This is useful for clientLoaders where we have a Request that identifies a single navigation. + * + * This is the generic implementation that can be used directly, but most likely the developer wants to use + * the function currently named `getDapperForRequest`. + * + * @remarks + * This function uses `lodash/isEqual` for deep comparison of `inputs`. + * - **Functions**: Compared by reference. Different instances of the same function logic will cause cache misses. + * - **Circular References**: Supported by `isEqual` but can be computationally expensive. + * - **Performance**: Deep comparison of large or deeply nested objects can be slow. + * + * @param funcName - The name of the function (used as part of the cache key). + * @param func - The function to execute. + * @param inputs - The arguments to pass to the function. + * @param request - The Request object to scope the cache to. + */ +export function deduplicatePromiseForRequest( + funcName: string, + func: (...inputs: Args) => Promise, + inputs: Args, + request: Request +): Promise { + // Performance note: We use a linear search over the cache entries. + // Since the number of unique requests per page load is usually small, this is acceptable. + // We use lodash/isEqual for deep comparison of arguments. + // CAUTION: + // - Large objects can cause performance issues. + // - Functions are compared by reference. + // - Circular references are handled but expensive. + const cache = getCache(request); + for (const cacheEntry of cache) { + if ( + cacheEntry.funcName === funcName && + isEqual(cacheEntry.inputs, inputs) + ) { + return cacheEntry.promise as Promise; + } + } + + const promise = func(...inputs) as Promise; + + const removeFromCache = (req: Request) => { + const currentCache = getCache(req); + const index = currentCache.findIndex((entry) => entry.promise === promise); + if (index !== -1) { + currentCache.splice(index, 1); + } + }; + + // Remove from cache if it rejects to allow retrying + promise.catch(() => removeFromCache(request)); + + // Remove from cache after a timeout to prevent memory leaks from hanging promises + // or unbounded growth in long-lived requests. + const CACHE_TIMEOUT_MS = 60000; + if (typeof WeakRef !== "undefined") { + const requestRef = new WeakRef(request); + setTimeout(() => { + const req = requestRef.deref(); + if (req) { + removeFromCache(req); + } + }, CACHE_TIMEOUT_MS); + } else { + setTimeout(() => removeFromCache(request), CACHE_TIMEOUT_MS); + } + + cache.push({ + funcName, + inputs, + promise, + }); + + return promise; +}