diff --git a/src/server/api/__tests__/organization/getOrgById.test.ts b/src/server/api/__tests__/organization/getOrgById.test.ts new file mode 100644 index 00000000..ebf28a85 --- /dev/null +++ b/src/server/api/__tests__/organization/getOrgById.test.ts @@ -0,0 +1,192 @@ +import { describe, it, expect, beforeEach } from "@jest/globals"; +import { PrismaClient } from "@prisma/client"; +import { type PartialDeep } from "type-fest"; +import type { Session } from "~/lib/authTypes"; +import * as ztController from "~/utils/ztApi"; +import { appRouter } from "../../root"; + +// Mock the ZeroTier controller. The member count is now computed from the +// database, so member_details should only be hit by the background +// reconciliation, never by the counting path. +jest.mock("~/utils/ztApi", () => ({ + member_details: jest.fn(), +})); + +const mockSession: PartialDeep = { + expires: new Date().toISOString(), + update: { name: "test" }, + user: { + id: "userid", + name: "Bernt Christian", + email: "mail@gmail.com", + }, +}; + +// Flush pending microtasks/macrotasks so the fire-and-forget background +// reconciliation has a chance to run before we assert on its side effects. +const flushPromises = async (times = 20): Promise => { + for (let i = 0; i < times; i++) { + await new Promise((resolve) => setImmediate(resolve)); + } +}; + +// Build a Prisma mock that satisfies both the role check and the org lookup. +// biome-ignore lint/suspicious/noExplicitAny: building a minimal Prisma stub +const createPrismaMock = (organization: any): PrismaClient => { + const prismaMock = new PrismaClient(); + prismaMock.userOrganizationRole.findFirst = jest + .fn() + .mockResolvedValue({ role: "ADMIN" }); + prismaMock.organization.findUnique = jest.fn().mockResolvedValue(organization); + prismaMock.network_members.delete = jest.fn().mockResolvedValue({}); + return prismaMock; +}; + +const createCaller = (prismaMock: PrismaClient) => + appRouter.createCaller({ + session: mockSession as Session, + wss: null, + prisma: prismaMock, + res: null, + }); + +describe("organization getOrgById member counts", () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it("counts authorized and total members from the database, ignoring deleted members", async () => { + const organizationId = "org-counts"; + const organization = { + id: organizationId, + orgName: "Test Org", + ownerId: "userid", + userRoles: [], + users: [], + webhooks: [], + invitations: [], + networks: [ + { + nwid: "nw1", + name: "network-one", + networkMembers: [ + { id: "m1", nwid: "nw1", authorized: true, deleted: false, permanentlyDeleted: false }, + { id: "m2", nwid: "nw1", authorized: false, deleted: false, permanentlyDeleted: false }, + // Deleted and permanently deleted members must be excluded from the counts. + { id: "m3", nwid: "nw1", authorized: true, deleted: true, permanentlyDeleted: false }, + { id: "m4", nwid: "nw1", authorized: true, deleted: false, permanentlyDeleted: true }, + ], + }, + { + nwid: "nw2", + name: "network-two", + networkMembers: [ + { id: "m5", nwid: "nw2", authorized: true, deleted: false, permanentlyDeleted: false }, + ], + }, + ], + }; + + // The controller reports everyone as unauthorized. If counting used the + // controller, the authorized counts would be 0 - asserting they reflect the + // database `authorized` flag proves the count comes from the database. + (ztController.member_details as jest.Mock).mockResolvedValue({ + authorized: false, + }); + + const prismaMock = createPrismaMock(organization); + const result = await createCaller(prismaMock).org.getOrgById({ organizationId }); + + expect(result.networks[0].memberCounts).toEqual({ + authorized: 1, + total: 2, + display: "1 (2)", + }); + expect(result.networks[1].memberCounts).toEqual({ + authorized: 1, + total: 1, + display: "1 (1)", + }); + expect(result.memberCounts).toEqual({ + authorized: 2, + total: 3, + display: "2 (3)", + }); + + await flushPromises(); + }); + + it("returns counts without blocking on the controller", async () => { + const organizationId = "org-nonblocking"; + const organization = { + id: organizationId, + orgName: "Test Org", + ownerId: "userid", + userRoles: [], + users: [], + webhooks: [], + invitations: [], + networks: [ + { + nwid: "nw1", + name: "network-one", + networkMembers: [ + { id: "m1", nwid: "nw1", authorized: true, deleted: false, permanentlyDeleted: false }, + { id: "m2", nwid: "nw1", authorized: true, deleted: false, permanentlyDeleted: false }, + ], + }, + ], + }; + + // A controller call that never resolves would hang the request if the + // counting path awaited it. The query must still resolve from the database. + (ztController.member_details as jest.Mock).mockImplementation( + () => new Promise(() => {}), + ); + + const prismaMock = createPrismaMock(organization); + const result = await createCaller(prismaMock).org.getOrgById({ organizationId }); + + expect(result.memberCounts.display).toBe("2 (2)"); + }); + + it("removes orphaned members in the background when the controller returns 404", async () => { + const organizationId = "org-reconcile"; + const organization = { + id: organizationId, + orgName: "Test Org", + ownerId: "userid", + userRoles: [], + users: [], + webhooks: [], + invitations: [], + networks: [ + { + nwid: "nw1", + name: "network-one", + networkMembers: [ + { id: "m1", nwid: "nw1", authorized: true, deleted: false, permanentlyDeleted: false }, + ], + }, + ], + }; + + // The controller no longer knows about this member - it is an orphan. + const notFound = new Error("Member not found"); + // biome-ignore lint/suspicious/noExplicitAny: APIError exposes a status field + (notFound as any).status = 404; + (ztController.member_details as jest.Mock).mockRejectedValue(notFound); + + const prismaMock = createPrismaMock(organization); + const result = await createCaller(prismaMock).org.getOrgById({ organizationId }); + + // Counts still come back immediately from the database. + expect(result.networks[0].memberCounts.display).toBe("1 (1)"); + + // The background reconciliation deletes the orphaned member row. + await flushPromises(); + expect(prismaMock.network_members.delete).toHaveBeenCalledWith({ + where: { id_nwid: { id: "m1", nwid: "nw1" } }, + }); + }); +}); diff --git a/src/server/api/routers/organizationRouter.ts b/src/server/api/routers/organizationRouter.ts index ee343bf4..077f813b 100644 --- a/src/server/api/routers/organizationRouter.ts +++ b/src/server/api/routers/organizationRouter.ts @@ -36,6 +36,7 @@ import { RoutesEntity } from "~/types/local/network"; import { MailTemplateKey } from "~/utils/enums"; import { TRPCError } from "@trpc/server"; import { MemberCounts } from "~/types/local/member"; +import { UserContext } from "~/types/ctx"; // Create a Zod schema for the HookType enum const HookTypeEnum = z.enum(Object.values(HookType) as [HookType, ...HookType[]]); @@ -49,6 +50,74 @@ const invitationRateLimit = rateLimit({ uniqueTokenPerInterval: 1000, }); +// Tracks in-flight background reconciliations per organization so that opening +// the org page repeatedly does not pile up overlapping controller syncs. +const orgReconcileInFlight = new Set(); + +/** + * Background reconciliation of an organization's network members against the + * ZeroTier controller. + * + * `getOrgById` counts members directly from the database for speed. This runs + * fire-and-forget (not awaited) to remove members that no longer exist on the + * controller (404 responses), which keeps the database from accumulating + * orphaned rows without slowing down page load. + */ +const reconcileOrgNetworkMembers = async ( + ctx: UserContext, + organizationId: string, + networks: { nwid: string; networkMembers: network_members[] }[], +) => { + // Skip if a reconciliation for this org is already running. + if (orgReconcileInFlight.has(organizationId)) return; + orgReconcileInFlight.add(organizationId); + + try { + for (const network of networks) { + for (const member of network.networkMembers) { + // Skip deleted or permanently deleted members + if (member.deleted || member.permanentlyDeleted) { + continue; + } + + try { + await ztController.member_details(ctx, network.nwid, member.id); + } catch (error) { + // Get status code directly from APIError + // biome-ignore lint/suspicious/noExplicitAny: APIError exposes status + const statusCode = (error as any).status; + if (statusCode === 404) { + // Safe to delete - member or network doesn't exist in ZeroTier + try { + await ctx.prisma.network_members.delete({ + where: { + id_nwid: { + id: member.id, + nwid: member.nwid, + }, + }, + }); + console.error(`Cleaned up orphaned member ${member.id} from database`); + } catch (cleanupError) { + console.error( + `Failed to cleanup orphaned member ${member.id}:`, + cleanupError, + ); + } + } else { + // For any other error, just log and skip + console.error( + `Skipping member ${member.id} due to error: ${(error as Error).message}`, + ); + } + } + } + } + } finally { + orgReconcileInFlight.delete(organizationId); + } +}; + export const organizationRouter = createTRPCRouter({ createOrg: adminRoleProtectedRoute .input( @@ -388,7 +457,12 @@ export const organizationRouter = createTRPCRouter({ })), }; - // Get authorized member and total member counts for each network + // Count authorized and total members directly from the database. + // This avoids one controller request per member on page load. The previous + // implementation called the ZeroTier controller sequentially for every + // member, which made this query extremely slow for large organizations. + // The `authorized`/`deleted` flags are kept in sync with the controller by + // the per-network page (see syncMemberPeersAndStatus). for (const network of organization.networks) { for (const member of network.networkMembers) { // Skip deleted or permanently deleted members @@ -396,51 +470,28 @@ export const organizationRouter = createTRPCRouter({ continue; } - try { - const memberDetails = await ztController.member_details( - ctx, - network.nwid, - member.id, - ); - if (memberDetails.authorized) { - network.memberCounts.authorized += 1; - organization.memberCounts.authorized += 1; - } - network.memberCounts.total += 1; - organization.memberCounts.total += 1; - } catch (error) { - // Get status code directly from APIError - // biome-ignore lint/suspicious/noExplicitAny: - const statusCode = (error as any).status; - if (statusCode === 404) { - // Safe to delete - member or network doesn't exist in ZeroTier - try { - await ctx.prisma.network_members.delete({ - where: { - id_nwid: { - id: member.id, - nwid: member.nwid, - }, - }, - }); - console.error(`Cleaned up orphaned member ${member.id} from database`); - } catch (cleanupError) { - console.error( - `Failed to cleanup orphaned member ${member.id}:`, - cleanupError, - ); - } - } else { - // For any other error, just log and skip - console.error( - `Skipping member ${member.id} due to error: ${error.message}`, - ); - } + if (member.authorized) { + network.memberCounts.authorized += 1; + organization.memberCounts.authorized += 1; } + network.memberCounts.total += 1; + organization.memberCounts.total += 1; } network.memberCounts.display = `${network.memberCounts.authorized} (${network.memberCounts.total})`; } organization.memberCounts.display = `${organization.memberCounts.authorized} (${organization.memberCounts.total})`; + + // Fire-and-forget reconciliation against the controller to clean up + // members that no longer exist on ZeroTier (404). This intentionally does + // not block the response so the page loads from the database immediately. + void reconcileOrgNetworkMembers( + ctx, + input.organizationId, + organization.networks, + ).catch((error) => { + console.error("Error reconciling organization network members:", error); + }); + return organization; }), createOrgNetwork: protectedProcedure