From 7f8ae90978836d88942c91cb5aa79e2f36de4737 Mon Sep 17 00:00:00 2001 From: dev_agent Date: Mon, 5 Oct 2026 09:10:53 -0400 Subject: [PATCH] Open saved pages offline, cache recipe reads, flag cache fallbacks Address the Feature 06 integration review: - Offline bootstrap: the API client keeps a device-local verified-session marker (localStorage "1" only, never the /me payload or CSRF token), set when /me answers with a member and cleared on a 401 or a not-in-household 403. When /me fails with a network error and the marker is set, AuthBootstrap opens the saved pages read-only (writes are refused for want of a CSRF token) and loads /me again when the browser comes back online. /me runs with networkMode 'always' so an offline load fails fast instead of pausing on the skeleton. - Recipe reads (/api/v1/recipes) join the network-first API cache; /auth, settings and jobs stay network-only, including encoded and dot-segment forms. - The network-first strategy marks a response answered from its cache with X-MealQuest-Cache-Fallback; the client counts it as offline, so the indicator shows while navigator.onLine is still true. Tests: offline reload of /plan and /dinner/:id after an online visit, no saved pages without a verified session or after a 401, reconnect reloads /me; the real Workbox NetworkFirst strategy against an in-memory cache opens a previously viewed recipe offline, marked; the pill shows for a cache fallback while online. Co-Authored-By: Claude Opus 5.5 --- web/src/App.test.tsx | 110 ++++++++++++++++++++++++++++--- web/src/AuthBootstrap.tsx | 22 ++++++- web/src/api/client.test.ts | 95 ++++++++++++++++++++++---- web/src/api/client.ts | 21 ++++-- web/src/api/index.ts | 1 + web/src/api/useJob.test.tsx | 4 +- web/src/api/verifiedSession.ts | 24 +++++++ web/src/main.tsx | 4 +- web/src/pwa/OfflinePill.test.tsx | 22 ++++++- web/src/pwa/networkStatus.ts | 7 +- web/src/sw.ts | 6 +- web/src/sw/apiStrategy.test.ts | 96 +++++++++++++++++++++++++++ web/src/sw/apiStrategy.ts | 16 +++++ web/src/sw/routes.test.ts | 29 +++++++- web/src/sw/routes.ts | 14 +++- web/src/sw/worker.test.ts | 9 ++- web/src/test/setup.ts | 3 + 17 files changed, 436 insertions(+), 47 deletions(-) create mode 100644 web/src/api/verifiedSession.ts create mode 100644 web/src/sw/apiStrategy.test.ts create mode 100644 web/src/sw/apiStrategy.ts diff --git a/web/src/App.test.tsx b/web/src/App.test.tsx index ea0f33a..8626068 100644 --- a/web/src/App.test.tsx +++ b/web/src/App.test.tsx @@ -1,7 +1,7 @@ -import { render, screen } from '@testing-library/react' +import { act, render, screen } from '@testing-library/react' import { MemoryRouter } from 'react-router' -import { afterEach, describe, expect, it, vi } from 'vitest' -import { createApiClient } from './api/index.ts' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createApiClient, createVerifiedSession } from './api/index.ts' import { createNetworkStatus } from './pwa/networkStatus.ts' import { mockViewport } from './test/matchMedia.ts' import { errorResponse, fakeLocation, jsonResponse, ME, request, stubFetch } from './test/fetchStub.ts' @@ -9,16 +9,20 @@ import App, { routes } from './App.tsx' afterEach(() => { Reflect.deleteProperty(window, 'matchMedia') + vi.restoreAllMocks() vi.unstubAllGlobals() }) function renderAt(path: string, location = fakeLocation(path)) { - render( + return { location, ...renderAppAt(path, location) } +} + +function renderAppAt(path: string, location = fakeLocation(path)) { + return render( - + , ) - return location } describe('App routes', () => { @@ -71,7 +75,7 @@ describe('App bootstrap', () => { it('sends a signed-out visitor to sign-in and never renders the routes', async () => { const fetchMock = stubFetch(errorResponse(401, 'unauthenticated')) - const location = renderAt('/discover', fakeLocation('/discover', '?swipe=1')) + const { location } = renderAt('/discover', fakeLocation('/discover', '?swipe=1')) await vi.waitFor(() => expect(location.assign).toHaveBeenCalledWith('/auth/login?return=%2Fdiscover%3Fswipe%3D1')) expect(fetchMock).toHaveBeenCalledTimes(1) @@ -81,7 +85,7 @@ describe('App bootstrap', () => { it('shows the not-in-this-household screen on a 403', async () => { const fetchMock = stubFetch(errorResponse(403, 'forbidden')) - const location = renderAt('/') + const { location } = renderAt('/') expect(await screen.findByText(/not in this household/)).toBeInTheDocument() expect(screen.getByRole('heading', { name: 'Not in this household' })).toBeInTheDocument() @@ -100,3 +104,93 @@ describe('App bootstrap', () => { expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() }) }) + +describe('App offline bootstrap', () => { + let onLine: boolean + + function goOffline() { + onLine = false + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) + } + + // visitOnline is an earlier online visit on this device: /me answers with a + // member, then the app is closed. + async function visitOnline() { + stubFetch(jsonResponse(200, ME)) + const { unmount } = renderAt('/') + expect(await screen.findByRole('heading', { level: 1, name: 'Home' })).toBeInTheDocument() + unmount() + } + + beforeEach(() => { + mockViewport(390) + onLine = true + vi.spyOn(navigator, 'onLine', 'get').mockImplementation(() => onLine) + }) + + it.each([ + ['/plan', 'Plan'], + ['/dinner/01J9ZK', 'Dinner'], + ])('an offline reload of %s after an online visit opens the saved page', async (path, title) => { + await visitOnline() + goOffline() + renderAt(path) + + expect(await screen.findByRole('heading', { level: 1, name: title })).toBeInTheDocument() + expect(screen.getByRole('navigation', { name: 'Primary' })).toBeInTheDocument() + expect(screen.getByRole('status')).toHaveTextContent('Offline — showing saved data') + expect(screen.queryByRole('alert')).not.toBeInTheDocument() + }) + + it('opens the saved pages when the server is unreachable but the browser reports online', async () => { + await visitOnline() + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) + renderAt('/plan') + + expect(await screen.findByRole('heading', { level: 1, name: 'Plan' })).toBeInTheDocument() + expect(screen.getByRole('status')).toHaveTextContent('Offline — showing saved data') + }) + + it('reports the failure offline when this device never verified a session', async () => { + goOffline() + renderAt('/plan') + + expect(await screen.findByRole('alert')).toHaveTextContent('MealQuest could not load: Failed to fetch') + expect(screen.queryByRole('navigation', { name: 'Primary' })).not.toBeInTheDocument() + }) + + it('reports the failure offline after the session was refused', async () => { + await visitOnline() + stubFetch(errorResponse(401, 'unauthenticated')) + const { location, unmount } = renderAt('/plan') + await vi.waitFor(() => expect(location.assign).toHaveBeenCalled()) + unmount() + + goOffline() + renderAt('/plan') + expect(await screen.findByRole('alert')).toHaveTextContent('MealQuest could not load: Failed to fetch') + expect(screen.queryByRole('navigation', { name: 'Primary' })).not.toBeInTheDocument() + }) + + it('loads /me again on reconnect and refuses writes until it has', async () => { + await visitOnline() + goOffline() + const api = createApiClient(fakeLocation('/plan'), createNetworkStatus(window), createVerifiedSession(localStorage)) + render( + + + , + ) + expect(await screen.findByRole('heading', { level: 1, name: 'Plan' })).toBeInTheDocument() + await expect(api.apiFetch('/api/v1/plans', { method: 'POST', body: {} })).rejects.toThrow(/no CSRF token/) + + const fetchMock = stubFetch(jsonResponse(200, ME)) + onLine = true + act(() => { + window.dispatchEvent(new Event('online')) + }) + await vi.waitFor(() => expect(request(fetchMock, 0)).toMatchObject({ path: '/api/v1/me', method: 'GET' })) + await vi.waitFor(() => expect(screen.queryByRole('status')).toBeNull()) + expect(screen.getByRole('heading', { level: 1, name: 'Plan' })).toBeInTheDocument() + }) +}) diff --git a/web/src/AuthBootstrap.tsx b/web/src/AuthBootstrap.tsx index 53f8661..d9c1aee 100644 --- a/web/src/AuthBootstrap.tsx +++ b/web/src/AuthBootstrap.tsx @@ -1,4 +1,4 @@ -import { useSyncExternalStore, type ReactNode } from 'react' +import { useEffect, useSyncExternalStore, type ReactNode } from 'react' import { useQuery } from '@tanstack/react-query' import { ApiError, ME_PATH, useApi } from './api/index.ts' import { NotInHouseholdPage } from './pages/NotInHouseholdPage.tsx' @@ -11,13 +11,29 @@ export const meQueryKey = [ME_PATH] as const // AuthBootstrap loads GET /api/v1/me before rendering children. A 401 is // already navigating to sign-in, so the skeleton stays up; an account outside // the household sees only the "not in this household" screen. +// +// When /me cannot reach the server and it last answered with a member on this +// device, the saved pages open read-only: the service worker answers their +// reads from its cache, and every write is refused for want of a CSRF token. +// /me runs even while the browser reports offline, so that case fails fast +// instead of waiting, and it is fetched again when the browser is back online. export function AuthBootstrap({ children }: { children: ReactNode }) { const api = useApi() const forbidden = useSyncExternalStore(api.subscribe, api.forbidden) - const me = useQuery({ queryKey: meQueryKey, queryFn: api.loadMe, staleTime: Infinity }) + const me = useQuery({ queryKey: meQueryKey, queryFn: api.loadMe, staleTime: Infinity, networkMode: 'always' }) + // fetch rejects with a TypeError only when the network fails. + const savedPages = me.isError && me.error instanceof TypeError && api.session.verified() + const { refetch } = me + + useEffect(() => { + if (!savedPages) return + return api.network.subscribe(() => { + if (!api.network.offline()) void refetch() + }) + }, [savedPages, api, refetch]) if (forbidden) return - if (me.isSuccess) return children + if (me.isSuccess || savedPages) return children if (me.isError && !(me.error instanceof ApiError && me.error.status === 401)) { return (
diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 3e788b7..c9bdb84 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -1,16 +1,18 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { errorResponse, fakeLocation, jsonResponse, ME, request, stubFetch } from '../test/fetchStub.ts' -import { ApiError, createApiClient, CSRF_HEADER } from './index.ts' +import { ApiError, createApiClient, createVerifiedSession, CSRF_HEADER, VERIFIED_SESSION_KEY } from './index.ts' +import { cacheFallbackHeader } from '../sw/routes.ts' import { createNetworkStatus } from '../pwa/networkStatus.ts' afterEach(() => { + vi.restoreAllMocks() vi.unstubAllGlobals() }) describe('apiFetch', () => { it('sends same-origin JSON and the cached CSRF token on non-GET requests only', async () => { const fetchMock = stubFetch(jsonResponse(200, ME), jsonResponse(200, { ok: 1 }), jsonResponse(200, { ok: 2 })) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() await expect(api.apiFetch('/api/v1/settings/household')).resolves.toEqual({ ok: 1 }) @@ -30,7 +32,7 @@ describe('apiFetch', () => { it('refuses a non-GET request before /me has loaded', async () => { const fetchMock = stubFetch() - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await expect(api.apiFetch('/api/v1/jobs', { method: 'POST', body: {} })).rejects.toThrow(/no CSRF token/) expect(fetchMock).not.toHaveBeenCalled() }) @@ -48,7 +50,7 @@ describe('apiFetch', () => { 'api/v1/me', ])('refuses %s without sending it or the CSRF token', async (path) => { const fetchMock = stubFetch(jsonResponse(200, ME)) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() await expect(api.apiFetch(path, { method: 'POST', body: {} })).rejects.toThrow( @@ -61,7 +63,7 @@ describe('apiFetch', () => { it('rejects with a typed ApiError from the error body', async () => { stubFetch(errorResponse(503, 'unavailable', true)) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) const error = await api.apiFetch('/api/v1/jobs/j1').catch((e: unknown) => e) expect(error).toBeInstanceOf(ApiError) expect(error).toMatchObject({ status: 503, code: 'unavailable', message: 'unavailable message', retryable: true }) @@ -69,14 +71,14 @@ describe('apiFetch', () => { it('rejects a response without the API error body', async () => { stubFetch(new Response('bad gateway', { status: 502 })) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await expect(api.apiFetch('/api/v1/jobs/j1')).rejects.toThrow('GET /api/v1/jobs/j1: 502 response is not JSON') }) it('on 401 navigates to sign-in with the current path and search to return to', async () => { stubFetch(errorResponse(401, 'unauthenticated')) const location = fakeLocation('/plan/shopping', '?week=2026-10-05') - const api = createApiClient(location, createNetworkStatus(window)) + const api = createApiClient(location, createNetworkStatus(window), createVerifiedSession(localStorage)) await expect(api.apiFetch('/api/v1/settings/household')).rejects.toMatchObject({ status: 401 }) expect(location.assign).toHaveBeenCalledExactlyOnceWith( @@ -91,7 +93,7 @@ describe('apiFetch', () => { jsonResponse(200, { ...ME, csrfToken: 'token-2' }), jsonResponse(201, { id: 'j1' }), ) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() await expect(api.apiFetch('/api/v1/jobs', { method: 'POST', body: { kind: 'noop' } })).resolves.toEqual({ @@ -112,7 +114,7 @@ describe('apiFetch', () => { jsonResponse(200, { ...ME, csrfToken: 'token-2' }), errorResponse(403, 'csrf'), ) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await api.loadMe() await expect(api.apiFetch('/api/v1/jobs', { method: 'POST', body: {} })).rejects.toMatchObject({ @@ -125,7 +127,7 @@ describe('apiFetch', () => { it('marks any other 403 as not in this household and notifies subscribers', async () => { const fetchMock = stubFetch(errorResponse(403, 'forbidden')) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) const listener = vi.fn() api.subscribe(listener) @@ -137,14 +139,14 @@ describe('apiFetch', () => { it('does not take a 403 without the API error body as not in this household', async () => { stubFetch(new Response('denied by proxy', { status: 403 })) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await expect(api.apiFetch('/api/v1/me')).rejects.toThrow('GET /api/v1/me: 403 response is not JSON') expect(api.forbidden()).toBe(false) }) it('does not refresh /me for a csrf 403 on a GET, which carries no token', async () => { const fetchMock = stubFetch(errorResponse(403, 'csrf')) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await expect(api.loadMe()).rejects.toMatchObject({ status: 403, code: 'csrf' }) expect(fetchMock).toHaveBeenCalledTimes(1) }) @@ -153,7 +155,74 @@ describe('apiFetch', () => { describe('loadMe', () => { it('rejects a /me response without a CSRF token', async () => { stubFetch(jsonResponse(200, { ...ME, csrfToken: '' })) - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) await expect(api.loadMe()).rejects.toThrow('response is not a member with a CSRF token') }) }) + +describe('verified session', () => { + it('is remembered when /me answers with a member', async () => { + stubFetch(jsonResponse(200, ME)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + expect(api.session.verified()).toBe(false) + await api.loadMe() + expect(api.session.verified()).toBe(true) + expect(localStorage.getItem(VERIFIED_SESSION_KEY)).toBe('1') + }) + + it('stores nothing from the /me response', async () => { + stubFetch(jsonResponse(200, ME)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + expect(Object.keys(localStorage)).toEqual([VERIFIED_SESSION_KEY]) + expect(JSON.stringify({ ...localStorage })).not.toContain(ME.csrfToken) + }) + + it('is not remembered for a /me response that is not a member', async () => { + stubFetch(jsonResponse(200, { ...ME, csrfToken: '' })) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await expect(api.loadMe()).rejects.toThrow() + expect(api.session.verified()).toBe(false) + }) + + it('is forgotten on a 401', async () => { + stubFetch(jsonResponse(200, ME), errorResponse(401, 'unauthenticated')) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + await expect(api.apiFetch('/api/v1/plans')).rejects.toMatchObject({ status: 401 }) + expect(api.session.verified()).toBe(false) + }) + + it('is forgotten when the account is not in this household', async () => { + stubFetch(jsonResponse(200, ME), errorResponse(403, 'forbidden')) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + await expect(api.apiFetch('/api/v1/plans')).rejects.toMatchObject({ status: 403 }) + expect(api.session.verified()).toBe(false) + }) + + it('is kept when a request fails with a network error', async () => { + stubFetch(jsonResponse(200, ME)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + await api.loadMe() + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) + await expect(api.loadMe()).rejects.toThrow(TypeError) + expect(api.session.verified()).toBe(true) + }) +}) + +describe('network status', () => { + it('counts a response the service worker answered from its cache as offline', async () => { + vi.spyOn(navigator, 'onLine', 'get').mockReturnValue(true) + const cached = jsonResponse(200, { id: '01J9ZR' }) + cached.headers.set(cacheFallbackHeader, '1') + stubFetch(cached, jsonResponse(200, { id: '01J9ZR' })) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) + + await expect(api.apiFetch('/api/v1/recipes/01J9ZR')).resolves.toEqual({ id: '01J9ZR' }) + expect(api.network.offline()).toBe(true) + + await expect(api.apiFetch('/api/v1/recipes/01J9ZR')).resolves.toEqual({ id: '01J9ZR' }) + expect(api.network.offline()).toBe(false) + }) +}) diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 22d6c10..cbfc0f0 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -1,5 +1,7 @@ import type { NetworkStatus } from '../pwa/networkStatus.ts' +import { cacheFallbackHeader } from '../sw/routes.ts' import { ApiError, isRecord, parseApiError } from './errors.ts' +import type { VerifiedSession } from './verifiedSession.ts' // CSRF_HEADER carries the session's CSRF token on every non-GET request. export const CSRF_HEADER = 'X-MealQuest-CSRF' @@ -52,14 +54,18 @@ export type ApiClient = { // account is not in this household; subscribe is notified when it changes. forbidden(): boolean subscribe(listener: () => void): () => void - // network learns of every request that failed with a network error and of - // every request that got a response. + // network learns of every request that failed with a network error or was + // answered from the service worker's cache, and of every request the + // server answered. network: NetworkStatus + // session remembers, on this device, whether /me last answered with a + // member; offline, AuthBootstrap opens the saved pages only if it did. + session: VerifiedSession } // createApiClient builds the app's API client. location is window.location in // the app: a 401 sends it to /auth/login with the current path to return to. -export function createApiClient(location: ApiLocation, network: NetworkStatus): ApiClient { +export function createApiClient(location: ApiLocation, network: NetworkStatus, session: VerifiedSession): ApiClient { let csrfToken: string | null = null let refreshing: Promise | null = null let forbidden = false @@ -91,7 +97,10 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus): if (error instanceof TypeError) network.requestFailed() throw error } - network.requestSucceeded() + // The service worker marks a response it answered from its cache because + // the server could not be reached in time. + if (response.headers.has(cacheFallbackHeader)) network.requestFailed() + else network.requestSucceeded() return response } @@ -135,12 +144,14 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus): function markForbidden() { if (forbidden) return + session.forget() forbidden = true listeners.forEach((listener) => listener()) } async function decode(method: string, path: string, response: Response): Promise { if (response.status === 401) { + session.forget() location.assign(`/auth/login?return=${encodeURIComponent(location.pathname + location.search)}`) } if (!response.ok) throw await parseApiError(method, path, response) @@ -162,6 +173,7 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus): throw new Error(`GET ${ME_PATH}: response is not a member with a CSRF token`) } csrfToken = me.csrfToken + session.remember() return me as Me } @@ -174,5 +186,6 @@ export function createApiClient(location: ApiLocation, network: NetworkStatus): return () => listeners.delete(listener) }, network, + session, } } diff --git a/web/src/api/index.ts b/web/src/api/index.ts index ea7a989..cae3e1f 100644 --- a/web/src/api/index.ts +++ b/web/src/api/index.ts @@ -2,4 +2,5 @@ export { ApiProvider, useApi } from './ApiContext.tsx' export { createApiClient, CSRF_HEADER, ME_PATH, type ApiClient, type ApiLocation, type ApiRequest, type Me } from './client.ts' export { ApiError } from './errors.ts' export { createQueryClient, shouldRetry } from './queryClient.ts' +export { createVerifiedSession, VERIFIED_SESSION_KEY, type VerifiedSession } from './verifiedSession.ts' export { useJob, type Job, type JobState, type JobStatus } from './useJob.ts' diff --git a/web/src/api/useJob.test.tsx b/web/src/api/useJob.test.tsx index e249ec5..21d1359 100644 --- a/web/src/api/useJob.test.tsx +++ b/web/src/api/useJob.test.tsx @@ -2,7 +2,7 @@ import { act, renderHook } from '@testing-library/react' import type { ReactNode } from 'react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { errorResponse, fakeLocation, jsonResponse, request, stubFetch } from '../test/fetchStub.ts' -import { ApiProvider, createApiClient, useJob } from './index.ts' +import { ApiProvider, createApiClient, createVerifiedSession, useJob } from './index.ts' import { createNetworkStatus } from '../pwa/networkStatus.ts' type Draft = { dinners: string[] } @@ -12,7 +12,7 @@ function job(status: string, result: Draft | null, error: string | null = null) } function renderJob(jobId: string | null) { - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) const wrapper = ({ children }: { children: ReactNode }) => {children} return renderHook(({ id }) => useJob(id), { wrapper, initialProps: { id: jobId } }) } diff --git a/web/src/api/verifiedSession.ts b/web/src/api/verifiedSession.ts new file mode 100644 index 0000000..c3fa7c3 --- /dev/null +++ b/web/src/api/verifiedSession.ts @@ -0,0 +1,24 @@ +// VERIFIED_SESSION_KEY is the localStorage key marking that GET /api/v1/me +// last answered with a household member on this device. It holds only "1" — +// never the /me response, its CSRF token or anything from /auth. +export const VERIFIED_SESSION_KEY = 'mealquest.verified-session' + +export type VerifiedSession = { + // verified reports whether /me last answered with a member on this device. + verified(): boolean + // remember records a /me that answered with a member. + remember(): void + // forget records that the server refused the session (401) or the account + // (not in this household). + forget(): void +} + +// createVerifiedSession keeps the marker in storage (window.localStorage in +// the app). A storage failure is thrown to the caller. +export function createVerifiedSession(storage: Storage): VerifiedSession { + return { + verified: () => storage.getItem(VERIFIED_SESSION_KEY) === '1', + remember: () => storage.setItem(VERIFIED_SESSION_KEY, '1'), + forget: () => storage.removeItem(VERIFIED_SESSION_KEY), + } +} diff --git a/web/src/main.tsx b/web/src/main.tsx index fbd16cf..63cd1b1 100644 --- a/web/src/main.tsx +++ b/web/src/main.tsx @@ -4,7 +4,7 @@ import { BrowserRouter } from 'react-router' import './styles/tokens.css' import './styles/base.css' import App from './App.tsx' -import { createApiClient } from './api/index.ts' +import { createApiClient, createVerifiedSession } from './api/index.ts' import { createNetworkStatus } from './pwa/networkStatus.ts' const root = document.getElementById('root') @@ -12,7 +12,7 @@ if (!root) { throw new Error('MealQuest: #root element missing from index.html') } -const api = createApiClient(window.location, createNetworkStatus(window)) +const api = createApiClient(window.location, createNetworkStatus(window), createVerifiedSession(window.localStorage)) createRoot(root).render( diff --git a/web/src/pwa/OfflinePill.test.tsx b/web/src/pwa/OfflinePill.test.tsx index 9a06880..5413a8a 100644 --- a/web/src/pwa/OfflinePill.test.tsx +++ b/web/src/pwa/OfflinePill.test.tsx @@ -1,7 +1,8 @@ import { act, render, screen } from '@testing-library/react' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' -import { ApiProvider, createApiClient, type ApiClient } from '../api/index.ts' +import { ApiProvider, createApiClient, createVerifiedSession, type ApiClient } from '../api/index.ts' import { fakeLocation, jsonResponse, ME, stubFetch } from '../test/fetchStub.ts' +import { cacheFallbackHeader } from '../sw/routes.ts' import { createNetworkStatus } from './networkStatus.ts' import { OfflinePill } from './OfflinePill.tsx' @@ -30,7 +31,7 @@ function goOnline() { } function renderPill(): ApiClient { - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) render( @@ -80,6 +81,21 @@ describe('OfflinePill', () => { expect(screen.queryByRole('status')).toBeNull() }) + it('shows while online when the service worker answers from its cache', async () => { + const api = renderPill() + const cached = jsonResponse(200, { id: '01J9ZR' }) + cached.headers.set(cacheFallbackHeader, '1') + stubFetch(cached, jsonResponse(200, { id: '01J9ZR' })) + + await act(() => api.apiFetch('/api/v1/recipes/01J9ZR')) + expect(navigator.onLine).toBe(true) + expect(screen.getByRole('status')).toHaveTextContent(copy) + + // The server answers again. + await act(() => api.apiFetch('/api/v1/recipes/01J9ZR')) + expect(screen.queryByRole('status')).toBeNull() + }) + it('ignores a request body that cannot be serialized', async () => { const api = renderPill() const fetchMock = stubFetch(jsonResponse(200, ME)) @@ -119,7 +135,7 @@ describe('OfflinePill', () => { it('removes its online and offline listeners on unmount', () => { const add = vi.spyOn(window, 'addEventListener') const remove = vi.spyOn(window, 'removeEventListener') - const api = createApiClient(fakeLocation(), createNetworkStatus(window)) + const api = createApiClient(fakeLocation(), createNetworkStatus(window), createVerifiedSession(localStorage)) const { unmount } = render( diff --git a/web/src/pwa/networkStatus.ts b/web/src/pwa/networkStatus.ts index 16955fc..8f124c1 100644 --- a/web/src/pwa/networkStatus.ts +++ b/web/src/pwa/networkStatus.ts @@ -4,14 +4,15 @@ export type NetworkWindow = Pick void): () => void - // requestFailed records a fetch that rejected with a network error. + // requestFailed records a fetch that rejected with a network error or that + // the service worker answered from its cache. requestFailed(): void - // requestSucceeded records a fetch that got a response. + // requestSucceeded records a fetch the server answered. requestSucceeded(): void } diff --git a/web/src/sw.ts b/web/src/sw.ts index 284f08e..81ebbb9 100644 --- a/web/src/sw.ts +++ b/web/src/sw.ts @@ -10,11 +10,11 @@ import type { RouteHandlerCallbackOptions } from 'workbox-core/types' import { ExpirationPlugin } from 'workbox-expiration' import { PrecacheFallbackPlugin, matchPrecache, precacheAndRoute } from 'workbox-precaching' import { registerRoute } from 'workbox-routing' -import { CacheFirst, NetworkFirst, NetworkOnly, type Strategy } from 'workbox-strategies' +import { CacheFirst, NetworkOnly, type Strategy } from 'workbox-strategies' +import { createApiStrategy } from './sw/apiStrategy' import { cacheNames, cacheVersion, - networkTimeoutSeconds, routeFor, staleCaches, type Route, @@ -30,7 +30,7 @@ setCacheNameDetails({ prefix: 'mealquest', precache: 'precache', suffix: cacheVe const day = 24 * 60 * 60 const strategies = { - [cacheNames.api]: new NetworkFirst({ cacheName: cacheNames.api, networkTimeoutSeconds }), + [cacheNames.api]: createApiStrategy(), [cacheNames.media]: new CacheFirst({ cacheName: cacheNames.media, plugins: [ diff --git a/web/src/sw/apiStrategy.test.ts b/web/src/sw/apiStrategy.test.ts new file mode 100644 index 0000000..c7e60b4 --- /dev/null +++ b/web/src/sw/apiStrategy.test.ts @@ -0,0 +1,96 @@ +// Runs the real Workbox network-first strategy against an in-memory cache +// storage: jsdom has none, and the worker itself needs service worker globals. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RouteHandlerCallbackOptions } from 'workbox-core/types' +import { createApiStrategy } from './apiStrategy' +import { cacheFallbackHeader, cacheNames, routeFor } from './routes' + +const origin = 'https://mealquest.test' + +// FakeCache keys entries by URL, which is all the API cache needs. +class FakeCache { + readonly entries = new Map() + async match(request: RequestInfo | URL): Promise { + return this.entries.get(new Request(request).url)?.clone() + } + async put(request: RequestInfo | URL, response: Response): Promise { + this.entries.set(new Request(request).url, response) + } +} + +// FakeExtendableEvent stands in for the worker-only ExtendableEvent, +// collecting the promises the strategy waits on. +class FakeExtendableEvent extends Event { + readonly pending: Promise[] = [] + waitUntil(promise: Promise) { + this.pending.push(promise) + } +} + +let cache: FakeCache + +beforeEach(() => { + cache = new FakeCache() + // Workbox checks its arguments against these worker-only globals. + vi.stubGlobal('FetchEvent', class FetchEvent {}) + vi.stubGlobal('ExtendableEvent', FakeExtendableEvent) + const only = (name: string | undefined) => { + if (name !== cacheNames.api) throw new Error(`unexpected cache ${name}`) + return cache + } + vi.stubGlobal('caches', { + open: async (name: string) => only(name), + match: async (request: RequestInfo | URL, options?: MultiCacheQueryOptions) => + only(options?.cacheName).match(request), + }) +}) + +afterEach(() => { + vi.unstubAllGlobals() +}) + +// handle sends a GET through the API strategy, as src/sw.ts does for a +// network-first route. +async function handle(path: string): Promise { + const request = new Request(new URL(path, origin)) + expect(routeFor({ method: 'GET', url: new URL(request.url), mode: 'cors' }, origin)).toEqual({ + strategy: 'network-first', + cacheName: cacheNames.api, + }) + const event = new FakeExtendableEvent('fetch') + const response = await createApiStrategy().handle({ request, event: event as unknown as RouteHandlerCallbackOptions['event'] }) + await Promise.all(event.pending) + return response +} + +function recipe(): Response { + return new Response(JSON.stringify({ id: '01J9ZR', name: 'Chicken tikka masala' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) +} + +describe('API strategy', () => { + it('answers from the network while the server is reachable, unmarked', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(recipe())) + const response = await handle('/api/v1/recipes/01J9ZR') + expect(response.headers.has(cacheFallbackHeader)).toBe(false) + expect(await response.json()).toMatchObject({ id: '01J9ZR' }) + expect(cache.entries.has(`${origin}/api/v1/recipes/01J9ZR`)).toBe(true) + }) + + it('opens a previously viewed recipe offline, marked as a cache fallback', async () => { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(recipe())) + await handle('/api/v1/recipes/01J9ZR') + + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) + const response = await handle('/api/v1/recipes/01J9ZR') + expect(response.headers.get(cacheFallbackHeader)).toBe('1') + expect(await response.json()).toEqual({ id: '01J9ZR', name: 'Chicken tikka masala' }) + }) + + it('fails offline for a recipe never viewed', async () => { + vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new TypeError('Failed to fetch'))) + await expect(handle('/api/v1/recipes/01J9ZS')).rejects.toThrow() + }) +}) diff --git a/web/src/sw/apiStrategy.ts b/web/src/sw/apiStrategy.ts new file mode 100644 index 0000000..0000b43 --- /dev/null +++ b/web/src/sw/apiStrategy.ts @@ -0,0 +1,16 @@ +// The network-first strategy for API reads, kept apart from src/sw.ts so the +// tests can run it without a service worker. +import type { WorkboxPlugin } from 'workbox-core/types' +import { NetworkFirst } from 'workbox-strategies' +import { cacheNames, markCacheFallback, networkTimeoutSeconds } from './routes' + +// NetworkFirst reads its cache only when the network fails or times out, so +// every cached response it uses is a fallback. +const markFallback: WorkboxPlugin = { + cachedResponseWillBeUsed: async ({ cachedResponse }) => + cachedResponse === undefined ? undefined : markCacheFallback(cachedResponse), +} + +export function createApiStrategy(): NetworkFirst { + return new NetworkFirst({ cacheName: cacheNames.api, networkTimeoutSeconds, plugins: [markFallback] }) +} diff --git a/web/src/sw/routes.test.ts b/web/src/sw/routes.test.ts index 784fa68..9b57d20 100644 --- a/web/src/sw/routes.test.ts +++ b/web/src/sw/routes.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest' -import { cacheNames, routeFor, staleCaches, type Route } from './routes' +import { cacheFallbackHeader, cacheNames, markCacheFallback, routeFor, staleCaches, type Route } from './routes' const origin = 'https://mealquest.test' @@ -17,6 +17,8 @@ describe('routeFor', () => { '/api/v1/shopping/current', '/api/v1/dinners', '/api/v1/dinners/01J9ZK?include=recipe', + '/api/v1/recipes', + '/api/v1/recipes/01J9ZR', '/api/v1/home', ])('GET %s is network-first in the API cache', (path) => { expect(route(path)).toEqual(apiCache) @@ -65,6 +67,8 @@ describe('routeFor', () => { '/%61pi/v1/jobs/01J9ZK', '/api/v1/j%6Fbs', '/api/v1/plans%2F..%2Fsettings', + '/api/v1/recipes%2F..%2Fsettings%2Fai', + '/api/v1/recipes/..%2F..%2F..%2Fauth%2Flogin', '/plan%2F..%2Fauth%2Flogin', ])('an encoded reserved path %s is network-only, navigated or not', (path) => { expect(route(path)).toEqual(networkOnly) @@ -81,7 +85,8 @@ describe('routeFor', () => { it('classifies other API paths by their decoded form', () => { expect(route('/%61pi/v1/plans/2026-W40')).toEqual(apiCache) - expect(route('/%61pi/v1/recipes', 'GET', 'navigate')).toEqual(networkOnly) + expect(route('/%61pi/v1/recipes/01J9ZR')).toEqual(apiCache) + expect(route('/%61pi/v1/ideas', 'GET', 'navigate')).toEqual(networkOnly) }) it.each(['POST', 'PUT', 'PATCH', 'DELETE', 'HEAD'])( @@ -91,6 +96,7 @@ describe('routeFor', () => { '/api/v1/plans/2026-W40', '/api/v1/shopping/current', '/api/v1/dinners/01J9ZK', + '/api/v1/recipes/01J9ZR', '/api/v1/home', '/media/photos/abc.jpg', '/assets/Figtree-Bold-a1b2c3.woff2', @@ -106,13 +112,14 @@ describe('routeFor', () => { it('matches whole path segments only', () => { expect(route('/api/v1/plansx')).toEqual(networkOnly) expect(route('/api/v1/homepage')).toEqual(networkOnly) + expect(route('/api/v1/recipesx')).toEqual(networkOnly) expect(route('/mediax/a.jpg')).toEqual(networkOnly) expect(route('/authors')).toEqual(networkOnly) expect(route('/api/v1/settingsx')).toEqual(networkOnly) }) it('leaves other API calls network-only', () => { - expect(route('/api/v1/recipes')).toEqual(networkOnly) + expect(route('/api/v1/me')).toEqual(networkOnly) expect(route('/api/v1/ideas', 'GET', 'navigate')).toEqual(networkOnly) }) @@ -155,3 +162,19 @@ describe('staleCaches', () => { expect(staleCaches(names, current)).toEqual(['mealquest-precache', 'mealquest-api-v0']) }) }) + +describe('markCacheFallback', () => { + it('copies the cached response with the cache fallback header set', async () => { + const cached = new Response('{"id":"01J9ZR"}', { + status: 200, + statusText: 'OK', + headers: { 'Content-Type': 'application/json' }, + }) + const marked = markCacheFallback(cached) + expect(marked.status).toBe(200) + expect(marked.statusText).toBe('OK') + expect(marked.headers.get('Content-Type')).toBe('application/json') + expect(marked.headers.get(cacheFallbackHeader)).toBe('1') + expect(await marked.json()).toEqual({ id: '01J9ZR' }) + }) +}) diff --git a/web/src/sw/routes.ts b/web/src/sw/routes.ts index e9ff348..46daa72 100644 --- a/web/src/sw/routes.ts +++ b/web/src/sw/routes.ts @@ -36,8 +36,20 @@ export const cacheNames = { export const networkTimeoutSeconds = 3 +// cacheFallbackHeader marks a network-first response answered from the cache +// because the server could not be reached in time, so the app can show the +// offline indicator even while the browser reports that it is online. +export const cacheFallbackHeader = 'X-MealQuest-Cache-Fallback' + +// markCacheFallback copies a cached response with cacheFallbackHeader set. +export function markCacheFallback(cached: Response): Response { + const headers = new Headers(cached.headers) + headers.set(cacheFallbackHeader, '1') + return new Response(cached.body, { status: cached.status, statusText: cached.statusText, headers }) +} + const neverCached = ['/auth', '/api/v1/settings', '/api/v1/jobs'] -const networkFirst = ['/api/v1/plans', '/api/v1/shopping', '/api/v1/dinners', '/api/v1/home'] +const networkFirst = ['/api/v1/plans', '/api/v1/shopping', '/api/v1/dinners', '/api/v1/recipes', '/api/v1/home'] // True when path is prefix itself or a path below it: /api/v1/plans matches // /api/v1/plans and /api/v1/plans/2026-W40, never /api/v1/plansx. diff --git a/web/src/sw/worker.test.ts b/web/src/sw/worker.test.ts index f4d3572..4b207b6 100644 --- a/web/src/sw/worker.test.ts +++ b/web/src/sw/worker.test.ts @@ -1,10 +1,15 @@ // The service worker itself needs worker globals, so these tests read its -// source: they pin the parts routes.ts cannot express — the font strategy's -// expiration and the font route's place ahead of the precache route. +// source: they pin the parts routes.ts cannot express — the API cache's +// strategy, the font strategy's expiration and the font route's place ahead of +// the precache route. import { describe, expect, it } from 'vitest' import source from '../sw.ts?raw' describe('service worker source', () => { + it('serves the API cache with the strategy that marks cache fallbacks', () => { + expect(source).toContain('[cacheNames.api]: createApiStrategy(),') + }) + it('gives the fonts cache a CacheFirst strategy with an expiration plugin', () => { expect(source).toMatch( /\[cacheNames\.fonts\]: new CacheFirst\(\{\s*cacheName: cacheNames\.fonts,\s*plugins: \[\s*new ExpirationPlugin\(\{ maxEntries: \d+, maxAgeSeconds: [^}]+\}\)/, diff --git a/web/src/test/setup.ts b/web/src/test/setup.ts index e87ca3f..6d0998a 100644 --- a/web/src/test/setup.ts +++ b/web/src/test/setup.ts @@ -4,4 +4,7 @@ import { afterEach } from 'vitest' afterEach(() => { cleanup() + // The verified-session marker and other device state must not leak between + // tests. Tests in the node environment (src/pwa.test.ts) have no storage. + if (typeof localStorage !== 'undefined') localStorage.clear() })