diff --git a/web/app/(app)/(auth)/verify-email/page.tsx b/web/app/(app)/(auth)/verify-email/page.tsx index 63339fd..1dd5be1 100644 --- a/web/app/(app)/(auth)/verify-email/page.tsx +++ b/web/app/(app)/(auth)/verify-email/page.tsx @@ -17,6 +17,7 @@ import { Loader2, CheckCircle, XCircle, Mail, ArrowRight } from 'lucide-react' import { useMutation, useQuery } from '@tanstack/react-query' import httpBrowserClient from '@/lib/httpBrowserClient' import { ApiEndpoints } from '@/config/api' +import { queryKeys } from '@/lib/api/query-keys' import { Routes } from '@/config/routes' // Reusable components @@ -82,17 +83,22 @@ export default function VerifyEmailPage() { const [errorMessage, setErrorMessage] = useState('') // Check user authentication and email verification status - const { - data: whoAmIData, + // Unwrapped to the user here rather than cached as the raw axios response. + // Every consumer of this endpoint now shares one key, so they must also + // share one cached shape. + const { + data: user, isPending: isCheckingAuth, - isError: isAuthError + isError: isAuthError } = useQuery({ - queryKey: ['whoAmI'], - queryFn: () => httpBrowserClient.get(ApiEndpoints.auth.whoAmI()), + queryKey: queryKeys.currentUser, + queryFn: () => + httpBrowserClient + .get(ApiEndpoints.auth.whoAmI()) + .then((res) => res.data?.data), retry: 1, }) - const user = whoAmIData?.data?.data const isEmailVerified = !!user?.emailVerifiedAt const isLoggedIn = !isAuthError && !!user diff --git a/web/app/(app)/dashboard/(components)/account-deletion-alert.tsx b/web/app/(app)/dashboard/(components)/account-deletion-alert.tsx index eceb3b0..91ab61a 100644 --- a/web/app/(app)/dashboard/(components)/account-deletion-alert.tsx +++ b/web/app/(app)/dashboard/(components)/account-deletion-alert.tsx @@ -2,6 +2,7 @@ import { Alert, AlertDescription } from '@/components/ui/alert' import { ApiEndpoints } from '@/config/api' import httpBrowserClient from '@/lib/httpBrowserClient' import { useQuery } from '@tanstack/react-query' +import { queryKeys } from '@/lib/api/query-keys' import { AlertTriangle } from 'lucide-react' export default function AccountDeletionAlert() { @@ -10,7 +11,7 @@ export default function AccountDeletionAlert() { isLoading: isLoadingUserData, error: userDataError, } = useQuery({ - queryKey: ['whoAmI'], + queryKey: queryKeys.currentUser, queryFn: () => httpBrowserClient .get(ApiEndpoints.auth.whoAmI()) diff --git a/web/app/(app)/dashboard/(components)/generate-api-key.tsx b/web/app/(app)/dashboard/(components)/generate-api-key.tsx index 6c5de78..ed6ae47 100644 --- a/web/app/(app)/dashboard/(components)/generate-api-key.tsx +++ b/web/app/(app)/dashboard/(components)/generate-api-key.tsx @@ -12,6 +12,7 @@ import { Routes } from '@/config/routes' import { useToast } from '@/hooks/use-toast' import httpBrowserClient from '@/lib/httpBrowserClient' import { useMutation, useQueryClient } from '@tanstack/react-query' +import { queryKeys } from '@/lib/api/query-keys' import { QrCode, Copy, Smartphone, Download, AlertTriangle } from 'lucide-react' import React, { forwardRef, useImperativeHandle, useState } from 'react' import QRCode from 'react-qr-code' @@ -59,9 +60,14 @@ const GenerateApiKey = forwardRef( onSuccess: () => { setIsConfirmGenerateKeyModalOpen(false) setIsGenerateKeyModalOpen(true) - queryClient.invalidateQueries({ queryKey: ['apiKeys', 'stats'] }) - queryClient.refetchQueries({ queryKey: ['apiKeys', 'stats'] }) - queryClient.invalidateQueries({ queryKey: ['devices'] }) + // Three separate caches, so three separate invalidations. This was + // written as a single ['apiKeys', 'stats'] key, which react-query + // matches by prefix and which no query uses, so it matched neither + // the key list (['apiKeys', status]) nor the stats (['stats']): + // generating a key refreshed nothing. + queryClient.invalidateQueries({ queryKey: queryKeys.apiKeysAll }) + queryClient.invalidateQueries({ queryKey: queryKeys.stats }) + queryClient.invalidateQueries({ queryKey: queryKeys.devices }) }, mutationFn: () => httpBrowserClient diff --git a/web/app/(app)/dashboard/(components)/get-started/use-onboarding.ts b/web/app/(app)/dashboard/(components)/get-started/use-onboarding.ts index 1c5e702..99dd1a1 100644 --- a/web/app/(app)/dashboard/(components)/get-started/use-onboarding.ts +++ b/web/app/(app)/dashboard/(components)/get-started/use-onboarding.ts @@ -4,6 +4,7 @@ import { useCallback, useEffect, useMemo, useRef, useState } from 'react' import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query' import httpBrowserClient from '@/lib/httpBrowserClient' import { ApiEndpoints } from '@/config/api' +import { queryKeys } from '@/lib/api/query-keys' import { STEPS, computeStepStates, @@ -23,9 +24,12 @@ export type OnboardingStatus = | 'hidden' | 'celebrate' -// Data + derived state for the onboarding card. Query keys intentionally match -// the app-wide keys (['whoAmI'], ['stats'], ['currentSubscription']) so this -// hook shares caches and invalidations with the other dashboard components. +// Data + derived state for the onboarding card. Keys come from the shared +// queryKeys factory so this hook shares caches and invalidations with the +// other dashboard components. They were previously written out by hand, and +// the user key was spelled ['whoAmI'] here while the typed useCurrentUser +// hook used ['currentUser'], so the same endpoint was cached twice and +// invalidating either one left the other stale. export function useOnboarding() { const queryClient = useQueryClient() const autoCompletedRef = useRef(false) @@ -38,7 +42,7 @@ export function useOnboarding() { const prevCompletedAtRef = useRef(undefined) const userQuery = useQuery({ - queryKey: ['whoAmI'], + queryKey: queryKeys.currentUser, queryFn: () => httpBrowserClient .get(ApiEndpoints.auth.whoAmI()) @@ -51,7 +55,7 @@ export function useOnboarding() { const userData = userQuery.data const statsQuery = useQuery({ - queryKey: ['stats'], + queryKey: queryKeys.stats, queryFn: () => httpBrowserClient .get(ApiEndpoints.gateway.getStats()) @@ -61,7 +65,7 @@ export function useOnboarding() { const stats = statsQuery.data const subQuery = useQuery({ - queryKey: ['currentSubscription'], + queryKey: queryKeys.subscription, queryFn: () => httpBrowserClient .get(ApiEndpoints.billing.currentSubscription()) @@ -81,9 +85,9 @@ export function useOnboarding() { .patch(ApiEndpoints.auth.updateOnboarding(), body) .then((r) => r.data), onSuccess: () => { - queryClient.invalidateQueries({ queryKey: ['whoAmI'] }) - queryClient.invalidateQueries({ queryKey: ['stats'] }) - queryClient.invalidateQueries({ queryKey: ['currentSubscription'] }) + queryClient.invalidateQueries({ queryKey: queryKeys.currentUser }) + queryClient.invalidateQueries({ queryKey: queryKeys.stats }) + queryClient.invalidateQueries({ queryKey: queryKeys.subscription }) }, }) diff --git a/web/app/(app)/dashboard/(components)/verify-email-alert.tsx b/web/app/(app)/dashboard/(components)/verify-email-alert.tsx index 0fc8581..c2913f4 100644 --- a/web/app/(app)/dashboard/(components)/verify-email-alert.tsx +++ b/web/app/(app)/dashboard/(components)/verify-email-alert.tsx @@ -3,6 +3,7 @@ import { Button } from '@/components/ui/button' import { ApiEndpoints } from '@/config/api' import httpBrowserClient from '@/lib/httpBrowserClient' import { useQuery } from '@tanstack/react-query' +import { queryKeys } from '@/lib/api/query-keys' import Link from 'next/link' import { useMemo } from 'react' import { Mail, ShieldAlert } from 'lucide-react' @@ -13,7 +14,7 @@ export default function VerifyEmailAlert() { isLoading: isLoadingUserData, error: userDataError, } = useQuery({ - queryKey: ['whoAmI'], + queryKey: queryKeys.currentUser, queryFn: () => httpBrowserClient .get(ApiEndpoints.auth.whoAmI()) diff --git a/web/lib/api/cache-invalidation.test.tsx b/web/lib/api/cache-invalidation.test.tsx new file mode 100644 index 0000000..7bf75ae --- /dev/null +++ b/web/lib/api/cache-invalidation.test.tsx @@ -0,0 +1,113 @@ +import { describe, expect, it, beforeEach } from 'vitest' +import { render, screen, waitFor, within } from '@testing-library/react' +import userEvent from '@testing-library/user-event' +import { http, HttpResponse } from 'msw' +import { QueryClient } from '@tanstack/react-query' +import { server } from '@/test/msw/server' +import { TestProviders } from '@/test/render' +import { API_BASE_URL, mockUser } from '@/test/fixtures' +import { ApiEndpoints } from '@/config/api' +import { queryKeys } from './query-keys' +import VerifyEmailAlert from '@/app/(app)/dashboard/(components)/verify-email-alert' +import GenerateApiKey from '@/app/(app)/dashboard/(components)/generate-api-key' + +const url = (path: string) => `${API_BASE_URL}${path.split('?')[0]}` + +/** + * These cover cache bugs that are invisible on inspection: the code looks + * correct at every individual call site, and only the relationship between + * sites is wrong. Both shipped to production. + */ +describe('cache invalidation', () => { + let queryClient: QueryClient + + beforeEach(() => { + // Not makeTestQueryClient: it sets gcTime 0, which collects any cache + // entry the moment it has no observer, so priming a cache to assert it + // gets invalidated would never survive to be asserted on. + queryClient = new QueryClient({ + defaultOptions: { + queries: { retry: false }, + mutations: { retry: false }, + }, + }) + }) + + const renderWith = (ui: React.ReactElement) => + render({ui}) + + // /auth/who-am-i used to be cached under two keys, ['whoAmI'] and + // ['currentUser'], so invalidating one left the other serving stale data. + // A user who verified their email still saw the "verify your email" banner, + // because the code that refreshed the user used the other key. + it('one invalidation refreshes every consumer of the current user', async () => { + let verified = false + server.use( + http.get(url(ApiEndpoints.auth.whoAmI()), () => + HttpResponse.json({ + data: { + ...mockUser, + emailVerifiedAt: verified ? new Date().toISOString() : null, + }, + }) + ) + ) + + const { container } = renderWith() + // Selected by href, not by label: the banner picks its CTA text at random + // from five variants, so a text matcher here would be flaky. + const banner = () => container.querySelector('a[href="/verify-email"]') + + // Unverified: the banner is up. + await waitFor(() => expect(banner()).toBeInTheDocument()) + + // The user verifies elsewhere, and something invalidates the user query + // using the canonical key. + verified = true + await queryClient.invalidateQueries({ queryKey: queryKeys.currentUser }) + + // The banner must notice. Under the two-key split it never did. + await waitFor(() => expect(banner()).toBeNull()) + }) + + // The generate handler invalidated ['apiKeys', 'stats'], a key no query uses. + // react-query matches by prefix, so it matched neither the key list + // (['apiKeys', 'active']) nor the dashboard stats (['stats']): generating a + // key refreshed nothing at all. + it('generating an API key refreshes both the key list and the stats', async () => { + server.use( + http.post(url(ApiEndpoints.auth.generateApiKey()), () => + HttpResponse.json({ data: 'new-api-key' }) + ) + ) + + // Prime the two caches a new key must invalidate. + queryClient.setQueryData(queryKeys.apiKeys('active'), { data: [] }) + queryClient.setQueryData(queryKeys.stats, { totalApiKeyCount: 0 }) + + renderWith() + + // The trigger and the dialog's confirm button carry the same label, so the + // confirm has to be scoped to the dialog. + await userEvent.click( + screen.getByRole('button', { name: /Generate API Key/i }) + ) + const confirmDialog = await screen.findByRole('dialog', { + name: /Create new API Key/i, + }) + await userEvent.click( + within(confirmDialog).getByRole('button', { name: /Generate API Key/i }) + ) + + await waitFor(() => { + expect( + queryClient.getQueryState(queryKeys.apiKeys('active'))?.isInvalidated, + 'the API key list must be invalidated' + ).toBe(true) + expect( + queryClient.getQueryState(queryKeys.stats)?.isInvalidated, + 'the dashboard stats must be invalidated' + ).toBe(true) + }) + }) +}) diff --git a/web/lib/api/query-keys.ts b/web/lib/api/query-keys.ts index e69f075..06ef38b 100644 --- a/web/lib/api/query-keys.ts +++ b/web/lib/api/query-keys.ts @@ -13,6 +13,11 @@ export const queryKeys = { billingPlans: ['billingPlans'] as const, apiKeys: (status: ApiKeyStatusFilter = 'active') => ['apiKeys', status] as const, + // Prefix covering every apiKeys list regardless of status filter. Use this + // to invalidate: a new or revoked key changes the active, revoked and all + // lists at once, and invalidating only apiKeys('active') leaves the other + // two serving stale data. + apiKeysAll: ['apiKeys'] as const, deviceMessages: (deviceId: string, filters?: Record) => filters ? (['messages', deviceId, filters] as const) diff --git a/web/lib/api/types.ts b/web/lib/api/types.ts index d06eee3..97b2e43 100644 --- a/web/lib/api/types.ts +++ b/web/lib/api/types.ts @@ -12,6 +12,10 @@ export interface User { avatar?: string | null emailVerifiedAt?: string | null onboardingCompletedAt?: string | null + // Set when the user asks for deletion, which starts a 7-day window. Real + // field on the user schema, and read by the account-deletion banner, but it + // was missing from this type. + accountDeletionRequestedAt?: string | null } export interface GatewayStats {