From ffa3880b809ece408a3a2cc3941bb28e07bbcb30 Mon Sep 17 00:00:00 2001 From: ryan-crabbe-berri Date: Sat, 6 Jun 2026 19:47:06 -0700 Subject: [PATCH] feat(ui): adopt openapi-react-query as the typed data-fetching client Stands up the typed transport on top of the generated OpenAPI types and migrates the first real hook to prove the pattern. src/lib/http/api.ts creates an openapi-fetch client wrapped by openapi-react-query ($api); base URL stays call-time via getProxyBaseUrl in an onRequest middleware (not a frozen createClient baseUrl, which would break server-root-path and worker switching), and the access token is passed per call via authHeader. useInfiniteUsers now calls $api.useInfiniteQuery("get", "/user/list", ...) instead of the hand-rolled userListCall: query params, the response body, and the user rows are all typed from the spec, and the 11-arg positional call collapses into a typed query object. Its infinite-query return shape is unchanged, so UsagePageView is untouched. Global query errors route through QueryCache.onError into the existing handleError, preserving the session-expiry logout behavior. The test mocks fetch (installed via vi.hoisted because openapi-fetch captures globalThis.fetch at client creation) and asserts the real request URLs, the bearer header, pagination, and the error path. --- ui/litellm-dashboard/package-lock.json | 30 ++ ui/litellm-dashboard/package.json | 2 + .../(dashboard)/hooks/users/useUsers.test.ts | 309 ++++++------------ .../app/(dashboard)/hooks/users/useUsers.ts | 44 +-- .../src/contexts/ReactQueryProvider.tsx | 10 +- ui/litellm-dashboard/src/lib/http/api.ts | 24 ++ 6 files changed, 190 insertions(+), 229 deletions(-) create mode 100644 ui/litellm-dashboard/src/lib/http/api.ts diff --git a/ui/litellm-dashboard/package-lock.json b/ui/litellm-dashboard/package-lock.json index 2afc145d15b..c552fb4e716 100644 --- a/ui/litellm-dashboard/package-lock.json +++ b/ui/litellm-dashboard/package-lock.json @@ -24,6 +24,8 @@ "moment": "2.30.1", "next": "16.2.6", "openai": "4.104.0", + "openapi-fetch": "0.17.0", + "openapi-react-query": "0.5.4", "papaparse": "5.5.3", "react": "18.3.1", "react-copy-to-clipboard": "5.1.1", @@ -10005,6 +10007,28 @@ "integrity": "sha512-JlCMO+ehdEIKqlFxk6IfVoAUVmgz7cU7zD/h9XZ0qzeosSHmUJVOzSQvvYSYWXkFXC+IfLKSIffhv0sVZup6pA==", "license": "MIT" }, + "node_modules/openapi-fetch": { + "version": "0.17.0", + "resolved": "https://registry.npmjs.org/openapi-fetch/-/openapi-fetch-0.17.0.tgz", + "integrity": "sha512-PsbZR1wAPcG91eEthKhN+Zn92FMHxv+/faECIwjXdxfTODGSGegYv0sc1Olz+HYPvKOuoXfp+0pA2XVt2cI0Ig==", + "license": "MIT", + "dependencies": { + "openapi-typescript-helpers": "^0.1.0" + } + }, + "node_modules/openapi-react-query": { + "version": "0.5.4", + "resolved": "https://registry.npmjs.org/openapi-react-query/-/openapi-react-query-0.5.4.tgz", + "integrity": "sha512-V9lRiozjHot19/BYSgXYoyznDxDJQhEBSdi26+SJ0UqjMANLQhkni4XG+Z7e3Ag7X46ZLMrL9VxYkghU3QvbWg==", + "license": "MIT", + "dependencies": { + "openapi-typescript-helpers": "^0.1.0" + }, + "peerDependencies": { + "@tanstack/react-query": "^5.80.0", + "openapi-fetch": "^0.17.0" + } + }, "node_modules/openapi-typescript": { "version": "7.13.0", "resolved": "https://registry.npmjs.org/openapi-typescript/-/openapi-typescript-7.13.0.tgz", @@ -10026,6 +10050,12 @@ "typescript": "^5.x" } }, + "node_modules/openapi-typescript-helpers": { + "version": "0.1.0", + "resolved": "https://registry.npmjs.org/openapi-typescript-helpers/-/openapi-typescript-helpers-0.1.0.tgz", + "integrity": "sha512-OKTGPthhivLw/fHz6c3OPtg72vi86qaMlqbJuVJ23qOvQ+53uw1n7HdmkJFibloF7QEjDrDkzJiOJuockM/ljw==", + "license": "MIT" + }, "node_modules/openapi-typescript/node_modules/supports-color": { "version": "10.2.2", "resolved": "https://registry.npmjs.org/supports-color/-/supports-color-10.2.2.tgz", diff --git a/ui/litellm-dashboard/package.json b/ui/litellm-dashboard/package.json index a8948f4be34..f2041e55cb9 100644 --- a/ui/litellm-dashboard/package.json +++ b/ui/litellm-dashboard/package.json @@ -40,6 +40,8 @@ "moment": "2.30.1", "next": "16.2.6", "openai": "4.104.0", + "openapi-fetch": "0.17.0", + "openapi-react-query": "0.5.4", "papaparse": "5.5.3", "react": "18.3.1", "react-copy-to-clipboard": "5.1.1", diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts index 537e2c5378a..eab2476809e 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.test.ts @@ -3,21 +3,24 @@ import { renderHook, waitFor } from "@testing-library/react"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import React, { ReactNode } from "react"; import { useInfiniteUsers } from "./useUsers"; -import { userListCall } from "@/components/networking"; -import type { UserListResponse } from "@/components/networking"; +import type { paths } from "@/lib/http/schema"; +type UserListResponse = paths["/user/list"]["get"]["responses"][200]["content"]["application/json"]; + +// openapi-fetch captures globalThis.fetch when the client is created (at import time), so the +// mock must be installed before imports run — hence vi.hoisted. Per test we reconfigure its +// implementation; the captured reference stays the same. +const { fetchMock } = vi.hoisted(() => { + const fetchMock = vi.fn(); + globalThis.fetch = fetchMock as unknown as typeof fetch; + return { fetchMock }; +}); + +// api.ts only needs the base URL + auth header name from networking; mock those so the +// test doesn't pull in the whole networking module. vi.mock("@/components/networking", () => ({ - userListCall: vi.fn(), -})); - -vi.mock("../common/queryKeysFactory", () => ({ - createQueryKeys: vi.fn((resource: string) => ({ - all: [resource], - lists: () => [resource, "list"], - list: (params?: any) => [resource, "list", { params }], - details: () => [resource, "detail"], - detail: (uid: string) => [resource, "detail", uid], - })), + getProxyBaseUrl: () => "http://localhost:4000", + getGlobalLitellmHeaderName: () => "Authorization", })); const mockUseAuthorized = vi.fn(); @@ -36,134 +39,102 @@ const DEFAULT_AUTH = { showSSOBanner: false, }; -const buildUserListResponse = (page: number, totalPages: number, userCount = 2): UserListResponse => ({ - page, - page_size: 50, - total: totalPages * userCount, - total_pages: totalPages, - users: Array.from({ length: userCount }, (_, i) => ({ - user_id: `user-${page}-${i}`, - user_email: `user-${page}-${i}@example.com`, - user_alias: null, - user_role: "Internal User", - spend: 0, - max_budget: null, - key_count: 0, - created_at: "2024-01-01T00:00:00Z", - updated_at: "2024-01-01T00:00:00Z", - sso_user_id: null, - budget_duration: null, - })), -}); +const buildUserListResponse = (page: number, totalPages: number, userCount = 2): UserListResponse => + ({ + page, + page_size: 50, + total: totalPages * userCount, + total_pages: totalPages, + users: Array.from({ length: userCount }, (_, i) => ({ + user_id: `user-${page}-${i}`, + user_email: `user-${page}-${i}@example.com`, + user_role: "internal_user", + key_count: 0, + })), + }) as UserListResponse; + +const requestOf = (arg: unknown): Request => arg as Request; + +const jsonResponse = (body: unknown, status = 200): Response => + new Response(JSON.stringify(body), { status, headers: { "content-type": "application/json" } }); + +/** Serve buildUserListResponse for whatever `page` the request asks for. */ +const stubPagedFetch = (totalPages: number, userCount = 2) => { + fetchMock.mockImplementation(async (arg: unknown) => { + const url = new URL(requestOf(arg).url); + const page = Number(url.searchParams.get("page") ?? "1"); + return jsonResponse(buildUserListResponse(page, totalPages, userCount)); + }); + return fetchMock; +}; + +const lastRequestUrl = (mock: typeof fetchMock): URL => new URL(requestOf(mock.mock.calls.at(-1)![0]).url); describe("useInfiniteUsers", () => { let queryClient: QueryClient; beforeEach(() => { - queryClient = new QueryClient({ - defaultOptions: { - queries: { - retry: false, - }, - }, - }); - vi.clearAllMocks(); + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + fetchMock.mockReset(); + mockUseAuthorized.mockReset(); mockUseAuthorized.mockReturnValue(DEFAULT_AUTH); }); const wrapper = ({ children }: { children: ReactNode }) => React.createElement(QueryClientProvider, { client: queryClient }, children); - it("should return paginated user data when query is successful", async () => { - const mockResponse = buildUserListResponse(1, 2); - (userListCall as any).mockResolvedValue(mockResponse); - + it("returns the first page of typed user data on success", async () => { + stubPagedFetch(2); const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); expect(result.current.data?.pages).toHaveLength(1); - expect(result.current.data?.pages[0]).toEqual(mockResponse); - expect(userListCall).toHaveBeenCalledWith("test-access-token", null, 1, 50, null); + expect(result.current.data?.pages[0].users[0].user_id).toBe("user-1-0"); }); - it("should use the default page size of 50", async () => { - const mockResponse = buildUserListResponse(1, 1); - (userListCall as any).mockResolvedValue(mockResponse); - + it("requests page 1 with the default page size and the bearer auth header", async () => { + const mock = stubPagedFetch(1); const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); - expect(userListCall).toHaveBeenCalledWith("test-access-token", null, 1, 50, null); + const url = lastRequestUrl(mock); + expect(url.pathname).toBe("/user/list"); + expect(url.searchParams.get("page")).toBe("1"); + expect(url.searchParams.get("page_size")).toBe("50"); + expect(requestOf(mock.mock.calls[0][0]).headers.get("Authorization")).toBe("Bearer test-access-token"); }); - it("should use a custom page size when provided", async () => { - const customPageSize = 25; - const mockResponse = buildUserListResponse(1, 1, 5); - (userListCall as any).mockResolvedValue(mockResponse); + it("sends a custom page size when provided", async () => { + const mock = stubPagedFetch(1, 5); + const { result } = renderHook(() => useInfiniteUsers(25), { wrapper }); - const { result } = renderHook(() => useInfiniteUsers(customPageSize), { - wrapper, - }); - - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - - expect(userListCall).toHaveBeenCalledWith("test-access-token", null, 1, customPageSize, null); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(lastRequestUrl(mock).searchParams.get("page_size")).toBe("25"); }); - it("should pass searchEmail to userListCall when provided", async () => { - const searchEmail = "search@example.com"; - const mockResponse = buildUserListResponse(1, 1, 1); - (userListCall as any).mockResolvedValue(mockResponse); + it("maps searchEmail to the user_email query param", async () => { + const mock = stubPagedFetch(1, 1); + const { result } = renderHook(() => useInfiniteUsers(50, "search@example.com"), { wrapper }); - const { result } = renderHook(() => useInfiniteUsers(50, searchEmail), { - wrapper, - }); - - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - - expect(userListCall).toHaveBeenCalledWith("test-access-token", null, 1, 50, searchEmail); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(lastRequestUrl(mock).searchParams.get("user_email")).toBe("search@example.com"); }); - it("should pass null for searchEmail when not provided", async () => { - const mockResponse = buildUserListResponse(1, 1); - (userListCall as any).mockResolvedValue(mockResponse); + it("omits user_email when no searchEmail is given", async () => { + const mock = stubPagedFetch(1); + const { result } = renderHook(() => useInfiniteUsers(50), { wrapper }); - const { result } = renderHook(() => useInfiniteUsers(50, undefined), { - wrapper, - }); - - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - - expect(userListCall).toHaveBeenCalledWith("test-access-token", null, 1, 50, null); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(lastRequestUrl(mock).searchParams.has("user_email")).toBe(false); }); - it("should fetch the next page when more pages are available", async () => { - const page1 = buildUserListResponse(1, 3); - const page2 = buildUserListResponse(2, 3); - let callCount = 0; - (userListCall as any).mockImplementation(async () => { - callCount++; - return callCount === 1 ? page1 : page2; - }); - + it("fetches the next page with an incremented page param", async () => { + const mock = stubPagedFetch(3); const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - + await waitFor(() => expect(result.current.isSuccess).toBe(true)); expect(result.current.hasNextPage).toBe(true); result.current.fetchNextPage(); @@ -173,115 +144,53 @@ describe("useInfiniteUsers", () => { expect(result.current.data?.pages).toHaveLength(2); }); - expect(result.current.data?.pages[1]).toEqual(page2); - expect(userListCall).toHaveBeenCalledTimes(2); - expect(userListCall).toHaveBeenLastCalledWith("test-access-token", null, 2, 50, null); + expect(result.current.data?.pages[1].page).toBe(2); + expect(mock).toHaveBeenCalledTimes(2); + expect(lastRequestUrl(mock).searchParams.get("page")).toBe("2"); }); - it("should not have a next page when on the last page", async () => { - const lastPage = buildUserListResponse(2, 2); - (userListCall as any).mockResolvedValue(lastPage); - + it("has no next page on the last page", async () => { + stubPagedFetch(1); const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - + await waitFor(() => expect(result.current.isSuccess).toBe(true)); expect(result.current.hasNextPage).toBe(false); }); - it("should not execute query when accessToken is missing", async () => { - mockUseAuthorized.mockReturnValue({ - ...DEFAULT_AUTH, - accessToken: null, - }); + it("surfaces an error when the request fails", async () => { + fetchMock.mockImplementation(async () => jsonResponse({ detail: "boom" }, 500)); + + const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); + + await waitFor(() => expect(result.current.isError).toBe(true)); + expect(result.current.data).toBeUndefined(); + }); + + it.each([ + ["null token", { accessToken: null }], + ["non-admin role", { userRole: "Internal User" }], + ])("does not fire a request when gated by %s", async (_label, override) => { + const mock = stubPagedFetch(1); + mockUseAuthorized.mockReturnValue({ ...DEFAULT_AUTH, ...override }); const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); expect(result.current.isLoading).toBe(false); - expect(result.current.data).toBeUndefined(); expect(result.current.isFetched).toBe(false); - expect(userListCall).not.toHaveBeenCalled(); + expect(mock).not.toHaveBeenCalled(); }); - it("should not execute query when userRole is not an admin role", async () => { - mockUseAuthorized.mockReturnValue({ - ...DEFAULT_AUTH, - userRole: "Internal User", - }); - - const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - - expect(result.current.isLoading).toBe(false); - expect(result.current.data).toBeUndefined(); - expect(result.current.isFetched).toBe(false); - expect(userListCall).not.toHaveBeenCalled(); - }); - - it("should not execute query when both accessToken and userRole are invalid", async () => { - mockUseAuthorized.mockReturnValue({ - ...DEFAULT_AUTH, - accessToken: null, - userRole: "App User", - }); - - const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - - expect(result.current.isLoading).toBe(false); - expect(result.current.data).toBeUndefined(); - expect(result.current.isFetched).toBe(false); - expect(userListCall).not.toHaveBeenCalled(); - }); - - it("should execute query for each admin role", async () => { - const adminRoles = ["Admin", "Admin Viewer", "proxy_admin", "proxy_admin_viewer", "org_admin"]; - - for (const role of adminRoles) { - vi.clearAllMocks(); - queryClient = new QueryClient({ - defaultOptions: { queries: { retry: false } }, - }); - const mockResponse = buildUserListResponse(1, 1); - (userListCall as any).mockResolvedValue(mockResponse); - mockUseAuthorized.mockReturnValue({ ...DEFAULT_AUTH, userRole: role }); + it("runs for each admin role", async () => { + for (const userRole of ["Admin", "Admin Viewer", "proxy_admin", "proxy_admin_viewer", "org_admin"]) { + fetchMock.mockReset(); + queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const mock = stubPagedFetch(1); + mockUseAuthorized.mockReturnValue({ ...DEFAULT_AUTH, userRole }); const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - - expect(userListCall).toHaveBeenCalledTimes(1); + await waitFor(() => expect(result.current.isSuccess).toBe(true)); + expect(mock).toHaveBeenCalledTimes(1); } }); - - it("should handle error when userListCall fails", async () => { - const testError = new Error("Failed to fetch users"); - (userListCall as any).mockRejectedValue(testError); - - const { result } = renderHook(() => useInfiniteUsers(), { wrapper }); - - await waitFor(() => { - expect(result.current.isError).toBe(true); - }); - - expect(result.current.error).toEqual(testError); - expect(result.current.data).toBeUndefined(); - }); - - it("should pass empty string searchEmail as null", async () => { - const mockResponse = buildUserListResponse(1, 1); - (userListCall as any).mockResolvedValue(mockResponse); - - const { result } = renderHook(() => useInfiniteUsers(50, ""), { - wrapper, - }); - - await waitFor(() => { - expect(result.current.isSuccess).toBe(true); - }); - - expect(userListCall).toHaveBeenCalledWith("test-access-token", null, 1, 50, null); - }); }); diff --git a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts index 9031de3cb1a..761b39b2416 100644 --- a/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts +++ b/ui/litellm-dashboard/src/app/(dashboard)/hooks/users/useUsers.ts @@ -1,38 +1,28 @@ -import { userListCall, UserListResponse } from "@/components/networking"; -import { useInfiniteQuery } from "@tanstack/react-query"; -import { createQueryKeys } from "../common/queryKeysFactory"; +import { $api, authHeader } from "@/lib/http/api"; import { all_admin_roles } from "@/utils/roles"; import useAuthorized from "@/app/(dashboard)/hooks/useAuthorized"; -const infiniteUsersKeys = createQueryKeys("infiniteUsers"); - const DEFAULT_PAGE_SIZE = 50; export const useInfiniteUsers = (pageSize: number = DEFAULT_PAGE_SIZE, searchEmail?: string) => { const { accessToken, userRole } = useAuthorized(); - return useInfiniteQuery({ - queryKey: infiniteUsersKeys.list({ - filters: { - pageSize, - ...(searchEmail && { searchEmail }), + return $api.useInfiniteQuery( + "get", + "/user/list", + { + params: { + query: { + page_size: pageSize, + ...(searchEmail ? { user_email: searchEmail } : {}), + }, }, - }), - queryFn: async ({ pageParam }) => { - return await userListCall( - accessToken!, - null, // userIDs - pageParam as number, // page - pageSize, // page_size - searchEmail || null, // userEmail - ); + headers: authHeader(accessToken!), }, - initialPageParam: 1, - getNextPageParam: (lastPage) => { - if (lastPage.page < lastPage.total_pages) { - return lastPage.page + 1; - } - return undefined; + { + pageParamName: "page", + initialPageParam: 1, + getNextPageParam: (lastPage) => (lastPage.page < lastPage.total_pages ? lastPage.page + 1 : undefined), + enabled: Boolean(accessToken) && all_admin_roles.includes(userRole!), }, - enabled: Boolean(accessToken) && all_admin_roles.includes(userRole!), - }); + ); }; diff --git a/ui/litellm-dashboard/src/contexts/ReactQueryProvider.tsx b/ui/litellm-dashboard/src/contexts/ReactQueryProvider.tsx index cf2d203a824..23e4568e854 100644 --- a/ui/litellm-dashboard/src/contexts/ReactQueryProvider.tsx +++ b/ui/litellm-dashboard/src/contexts/ReactQueryProvider.tsx @@ -1,8 +1,14 @@ "use client"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { QueryClient, QueryClientProvider, QueryCache } from "@tanstack/react-query"; +import { handleError } from "@/components/networking"; +import { deriveErrorMessage } from "@/lib/http/client"; -const queryClient = new QueryClient(); +const queryClient = new QueryClient({ + queryCache: new QueryCache({ + onError: (error) => handleError(deriveErrorMessage(error)), + }), +}); export default function ReactQueryProvider({ children }: { children: React.ReactNode }) { return {children}; diff --git a/ui/litellm-dashboard/src/lib/http/api.ts b/ui/litellm-dashboard/src/lib/http/api.ts new file mode 100644 index 00000000000..8aa105a2b50 --- /dev/null +++ b/ui/litellm-dashboard/src/lib/http/api.ts @@ -0,0 +1,24 @@ +import createFetchClient from "openapi-fetch"; +import createClient from "openapi-react-query"; +import type { paths } from "./schema"; +import { getProxyBaseUrl, getGlobalLitellmHeaderName } from "@/components/networking"; + +// Placeholder origin so openapi-fetch always builds a parseable absolute URL (relative +// Request construction throws under Node/SSR). The onRequest middleware replaces it with +// the real, call-time base from getProxyBaseUrl on every request, so it never hits the network. +const PLACEHOLDER_ORIGIN = "http://litellm.local"; + +const fetchClient = createFetchClient({ baseUrl: PLACEHOLDER_ORIGIN }); + +fetchClient.use({ + onRequest({ request }) { + const tail = new URL(request.url); + return new Request(getProxyBaseUrl() + tail.pathname + tail.search, request); + }, +}); + +export const $api = createClient(fetchClient); + +export const authHeader = (accessToken: string): Record => ({ + [getGlobalLitellmHeaderName()]: `Bearer ${accessToken}`, +});