mirror of
https://github.com/MBombeck/HealthLog.git
synced 2026-11-08 20:42:43 +00:00
feat(sharing): key the query cache by the record it holds
Every cached read in this app is about one health record, and nothing in the key factory said which one — there was only ever the signed-in account's. Once a browser can act on somebody else's, `["dashboard","snapshot"]` names two different answers and the cache cannot tell them apart. That is this repo's known cache-poison class, on health data, across two people. One line at the root rather than a dimension threaded through twenty-three key modules: a `queryKeyHashFn` that folds the record id into the cache identity of every entry. Key arrays are untouched, which is the part that matters — prefix invalidation and the import-time dependent-key bundles both match on the array and keep working. Prefixing the arrays would have broken every bundle silently. The hash is defence in depth. Switching is a full page reload, so the in-memory cache is empty on the other side of it; the layers that survive a reload are the IndexedDB snapshot and the service worker's data cache, and those are handled by refusing to write either while a switch is on, and by skipping the restore. Nothing of the owner's record ever reaches the disk of a delegate's device, so a stale scope marker can cost an empty restore and never another person's data. The persister's per-account binding read the account payload's id, which under a switch is the delegate in both states — it could not tell the two apart. It now asks the record scope, which can.
This commit is contained in:
1 parent
562123d4d1
commit
80175f3bfc
5 files changed
+256
No files matched your search
@@ -1,5 +1,7 @@
|
||||
import type { DefaultOptions, QueryClient } from "@tanstack/react-query";
|
||||
|
||||
import { recordScopedQueryKeyHashFn } from "@/lib/query-keys/record-scope";
|
||||
|
||||
export const MEANINGFUL_HIDDEN_INTERVAL_MS = 60_000;
|
||||
|
||||
interface VisibilityTarget {
|
||||
@@ -57,6 +59,13 @@ export function subscribeToMeaningfulVisibilityRefresh(
|
||||
* regardless, the offline `fetch` rejects immediately, and `onError` fires — an
|
||||
* honest failure the form surfaces (and the global `OfflineMutationToaster`
|
||||
* backstops), instead of a spinner-of-death plus silent write loss.
|
||||
*
|
||||
* v1.36.0 — `queryKeyHashFn` folds the record being read into the cache
|
||||
* identity of every entry in the app. Without it `["dashboard","snapshot"]`
|
||||
* names two different people's data and the cache cannot tell them apart. See
|
||||
* `src/lib/query-keys/record-scope.ts` for why this sits on the hash rather
|
||||
* than on the key arrays (the arrays feed prefix invalidation and the
|
||||
* import-time dependent-key bundles, both of which would break).
|
||||
*/
|
||||
export const QUERY_CLIENT_DEFAULT_OPTIONS: DefaultOptions = {
|
||||
queries: {
|
||||
@@ -64,6 +73,7 @@ export const QUERY_CLIENT_DEFAULT_OPTIONS: DefaultOptions = {
|
||||
gcTime: 10 * 60 * 1000,
|
||||
refetchOnWindowFocus: false,
|
||||
retry: 1,
|
||||
queryKeyHashFn: recordScopedQueryKeyHashFn,
|
||||
},
|
||||
mutations: {
|
||||
networkMode: "always",
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
} from "@tanstack/react-query";
|
||||
|
||||
import { queryKeys } from "@/lib/query-keys";
|
||||
import { isReadingSharedRecord } from "@/lib/query-keys/record-scope";
|
||||
|
||||
const DB_NAME = "healthlog-query-cache";
|
||||
const STORE = "kv";
|
||||
@@ -173,6 +174,12 @@ export async function restorePersistedQueryCache(
|
||||
buildVersion: string,
|
||||
): Promise<void> {
|
||||
if (typeof indexedDB === "undefined") return;
|
||||
// v1.36.0 — a browser inside somebody else's record restores nothing. The
|
||||
// store holds the caller's OWN dashboard (it is the only thing that was ever
|
||||
// written to it, see the flush below), and painting that under a banner
|
||||
// saying "viewing her record" is the mis-context failure this feature is
|
||||
// built to prevent, in the one place a full reload cannot clear.
|
||||
if (isReadingSharedRecord()) return;
|
||||
try {
|
||||
const payload = await idbGet<PersistedPayload>();
|
||||
if (!payload) return;
|
||||
@@ -245,6 +252,13 @@ export function startPersistingQueryCache(
|
||||
let timer: ReturnType<typeof setTimeout> | undefined;
|
||||
|
||||
const flush = () => {
|
||||
// v1.36.0 — never write another person's record to this device. The
|
||||
// allowlist below decides WHICH families are low-sensitivity enough to
|
||||
// persist; that judgement was made about the account's own data and does
|
||||
// not carry over to a record they were merely given to read. Refusing at
|
||||
// the flush (rather than filtering keys) also means the last good snapshot
|
||||
// of the caller's own dashboard survives a switch untouched.
|
||||
if (isReadingSharedRecord()) return;
|
||||
const state = dehydrate(queryClient, {
|
||||
shouldDehydrateQuery: (query) =>
|
||||
query.state.status === "success" && isPersistableKey(query.queryKey),
|
||||
@@ -338,3 +352,26 @@ export async function clearOfflineCachesForSessionEnd(): Promise<void> {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* v1.36.0 — the same wipe, before a record switch in either direction.
|
||||
*
|
||||
* Deliberately the same bytes as the session-end wipe rather than a narrower
|
||||
* one, and deliberately named for the act that calls it.
|
||||
*
|
||||
* The full page reload the switch performs clears the in-memory cache; these
|
||||
* two layers are the ones that survive it. The IndexedDB snapshot holds the
|
||||
* dashboard the browser was last looking at, and the service worker's
|
||||
* `healthlog-data-*` cache answers `/api/…` reads from disk on a
|
||||
* stale-while-revalidate branch — so without this call, the first paint after
|
||||
* switching into a record can be the previous record's numbers, served by the
|
||||
* worker, under a banner naming the new one. Nothing about the page's own code
|
||||
* would be wrong; the answer would just come from somewhere the page does not
|
||||
* control.
|
||||
*
|
||||
* Run it BEFORE the reload and await it. A wipe racing the navigation is a
|
||||
* wipe that sometimes does not happen, and "sometimes" is the whole failure.
|
||||
*/
|
||||
export async function clearOfflineCachesForRecordSwitch(): Promise<void> {
|
||||
await clearOfflineCachesForSessionEnd();
|
||||
}
|
||||
@@ -36,6 +36,7 @@ import { mentalHealthKeys } from "./mental-health";
|
||||
import { moodKeys } from "./mood";
|
||||
import { nutrientKeys } from "./nutrients";
|
||||
import { settingsKeys } from "./settings";
|
||||
import { sharingKeys } from "./sharing";
|
||||
import { workoutKeys } from "./workouts";
|
||||
|
||||
export const queryKeys = {
|
||||
@@ -62,8 +63,25 @@ export const queryKeys = {
|
||||
...documentKeys,
|
||||
...customMetrics,
|
||||
...nutrientKeys,
|
||||
...sharingKeys,
|
||||
};
|
||||
|
||||
/**
|
||||
* v1.36.0 — every read a grant transition can make stale.
|
||||
*
|
||||
* Inviting, accepting, revoking and renouncing all change the same three
|
||||
* answers: the grant list, the owner's record-activity feed, and
|
||||
* `accountAccess` on the account payload — which is what drives the switcher
|
||||
* and the banner. A revoke that refreshed the panel but not the payload would
|
||||
* leave a switcher entry pointing at a record the server had just closed: the
|
||||
* affordance still on screen, the click already refused.
|
||||
*/
|
||||
export const grantDependentKeys = [
|
||||
queryKeys.accountGrants(),
|
||||
queryKeys.accountActivity(),
|
||||
queryKeys.authMe(),
|
||||
];
|
||||
|
||||
/**
|
||||
* Keys that should be invalidated when a measurement is created, updated or
|
||||
* deleted. Kept here so dashboards, insights, and targets always stay in sync.
|
||||
|
||||
@@ -0,0 +1,154 @@
|
||||
/**
|
||||
* v1.36.0 — the account dimension of the query cache.
|
||||
*
|
||||
* Every cached read in this app is about one health record. Nothing in the key
|
||||
* factory says which one, because until now there was only ever one: the
|
||||
* signed-in account's. Once a browser can act on somebody else's record, a key
|
||||
* like `["dashboard","snapshot"]` names two different answers, and a cache that
|
||||
* cannot tell them apart will hand the second reader the first one's data.
|
||||
* That is this repository's known cache-poison class, on health data, across
|
||||
* two people.
|
||||
*
|
||||
* The fix is one line at the root rather than a dimension threaded through
|
||||
* twenty-three key modules: a `queryKeyHashFn` on the QueryClient. The hash is
|
||||
* what the cache stores entries under and what `getQueryData` looks them up by,
|
||||
* so folding the record id into it partitions every entry in the app at once.
|
||||
* Key ARRAYS are untouched, which matters more than it looks: prefix
|
||||
* invalidation (`invalidateQueries({ queryKey: ["measurements"] })`) and the
|
||||
* module-scope dependent-key bundles in `./index.ts` both match on the array,
|
||||
* were computed at import time before any record was known, and keep working
|
||||
* unchanged. Prefixing the arrays instead would have silently broken every one
|
||||
* of those bundles.
|
||||
*
|
||||
* ## Why this is defence in depth and not the primary control
|
||||
*
|
||||
* Switching records is a full page reload (design §2), so the in-memory cache
|
||||
* is empty on the other side of it and this hash can only matter if that ever
|
||||
* stops being true. The primary control for the layers that DO survive a
|
||||
* reload — the IndexedDB snapshot and the service worker's data cache — is the
|
||||
* wipe the switch performs before reloading, in `switchRecord()`. Both are
|
||||
* cheap; only one of them is the reason the feature is safe.
|
||||
*
|
||||
* ## Why the mirror is in localStorage
|
||||
*
|
||||
* The scope has to be known synchronously at boot, before `/api/auth/me`
|
||||
* answers, because the persister decides whether to restore from disk in the
|
||||
* first effect after mount. A switch is a property of the SESSION — it is
|
||||
* carried on the session row, not per tab — so every tab of the browser is
|
||||
* switched at once, which is what localStorage models and sessionStorage does
|
||||
* not.
|
||||
*
|
||||
* A mirror can go stale, and the honest question is what a stale one costs.
|
||||
* Nothing, because the disk layers are wiped on every switch in both
|
||||
* directions: a mirror claiming a record the session has left points the
|
||||
* persister at a store that was emptied when it left, and a mirror claiming
|
||||
* "own" while the session is inside a record points it at a store that was
|
||||
* emptied on the way in and never written to since (the persister refuses to
|
||||
* write while switched — see `isPersistableKey`'s caller). Either way the
|
||||
* worst case is an empty restore and a skeleton, never another person's data.
|
||||
* `fetchMe` re-stamps the mirror from the server's resolved answer on every
|
||||
* boot, so the staleness window is one round trip.
|
||||
*/
|
||||
import type { QueryKey } from "@tanstack/react-query";
|
||||
|
||||
const STORAGE_KEY = "healthlog-record-scope";
|
||||
|
||||
/**
|
||||
* The in-process copy. Seeded from storage on first read so the hash function
|
||||
* — which runs on every single query — never touches localStorage.
|
||||
*/
|
||||
let scope: string | null = null;
|
||||
let seeded = false;
|
||||
|
||||
function readMirror(): string | null {
|
||||
try {
|
||||
const raw = localStorage.getItem(STORAGE_KEY);
|
||||
return raw === null || raw.length === 0 ? null : raw;
|
||||
} catch {
|
||||
// Private mode, disabled storage, or SSR. "Own record" is the safe
|
||||
// reading: it is what every browser reported before this feature existed.
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The record this browser is currently reading, or null when it is reading its
|
||||
* own.
|
||||
*
|
||||
* Server-side this is always null. `Providers` renders on the server during
|
||||
* SSR, where there is no storage to read and no cache to partition.
|
||||
*/
|
||||
export function getRecordScope(): string | null {
|
||||
if (typeof window === "undefined") return null;
|
||||
if (!seeded) {
|
||||
scope = readMirror();
|
||||
seeded = true;
|
||||
}
|
||||
return scope;
|
||||
}
|
||||
|
||||
/**
|
||||
* Point the cache at a record, or (with `null`) back at the caller's own.
|
||||
*
|
||||
* Called from exactly two places: the switch flow, before it reloads the page,
|
||||
* and `fetchMe`, which corrects the mirror from the server's resolved
|
||||
* `accountAccess.active` on every account read. The second is what keeps a
|
||||
* mirror written by a switch that the server has since ended from surviving
|
||||
* past one round trip.
|
||||
*/
|
||||
export function setRecordScope(accountId: string | null): void {
|
||||
scope = accountId;
|
||||
seeded = true;
|
||||
if (typeof window === "undefined") return;
|
||||
try {
|
||||
if (accountId === null) {
|
||||
localStorage.removeItem(STORAGE_KEY);
|
||||
} else {
|
||||
localStorage.setItem(STORAGE_KEY, accountId);
|
||||
}
|
||||
} catch {
|
||||
/* storage unavailable — the in-process copy still partitions this tab */
|
||||
}
|
||||
}
|
||||
|
||||
/** True while this browser is reading somebody else's record. */
|
||||
export function isReadingSharedRecord(): boolean {
|
||||
return getRecordScope() !== null;
|
||||
}
|
||||
|
||||
/**
|
||||
* The QueryClient's key hash function.
|
||||
*
|
||||
* Identical to TanStack's own `hashKey` when the browser is in its own record
|
||||
* — a `JSON.stringify` with sorted object keys — so nothing about the app's
|
||||
* cache changes for the overwhelming majority of sessions. Inside a shared
|
||||
* record every entry lands under a distinct hash, which is what makes an
|
||||
* inherited entry unreachable rather than merely stale.
|
||||
*/
|
||||
export function recordScopedQueryKeyHashFn(queryKey: QueryKey): string {
|
||||
const key = JSON.stringify(queryKey, (_, val) =>
|
||||
isPlainObject(val)
|
||||
? Object.keys(val)
|
||||
.sort()
|
||||
.reduce<Record<string, unknown>>((result, k) => {
|
||||
result[k] = (val as Record<string, unknown>)[k];
|
||||
return result;
|
||||
}, {})
|
||||
: val,
|
||||
);
|
||||
const record = getRecordScope();
|
||||
return record === null ? key : `record:${record}|${key}`;
|
||||
}
|
||||
|
||||
/** TanStack's own predicate, copied rather than imported — it is not exported. */
|
||||
function isPlainObject(value: unknown): value is Record<string, unknown> {
|
||||
if (Object.prototype.toString.call(value) !== "[object Object]") return false;
|
||||
const prototype = Object.getPrototypeOf(value);
|
||||
return prototype === null || prototype === Object.prototype;
|
||||
}
|
||||
|
||||
/** @internal — lets a test start from a known state. */
|
||||
export const __resetRecordScopeForTests = (): void => {
|
||||
scope = null;
|
||||
seeded = false;
|
||||
};
|
||||
@@ -0,0 +1,37 @@
|
||||
/**
|
||||
* Query keys — account sharing (v1.36.0).
|
||||
* Part of the centralized factory; aggregated in `./index.ts`.
|
||||
*
|
||||
* Note what these keys deliberately do NOT carry: the acting account. That
|
||||
* dimension lives on the QueryClient's `queryKeyHashFn`
|
||||
* (`./record-scope.ts`), which folds it into every entry in the app at once
|
||||
* — including these. Adding it here as well would partition the same cache
|
||||
* twice under two rules, which is the two-deciders shape in miniature.
|
||||
*
|
||||
* The reads below are all non-delegable by construction: `/api/account/grants`
|
||||
* and the record-activity feed both resolve through a bare `requireAuth()` and
|
||||
* refuse while a switch is on. They are still record-scoped like everything
|
||||
* else, so a session that switches out and back never reads a grant list
|
||||
* cached under the wrong scope.
|
||||
*/
|
||||
export const sharingKeys = {
|
||||
/**
|
||||
* Grants in both directions (`GET /api/account/grants`). One key for one
|
||||
* endpoint: the route answers "who can see my record, and whose can I see"
|
||||
* in a single payload precisely so no client renders half of it.
|
||||
*/
|
||||
accountGrants: () => ["account", "grants"] as const,
|
||||
|
||||
/**
|
||||
* The owner's record-activity feed (`GET /api/account/activity`) — who
|
||||
* opened their record, and when.
|
||||
*/
|
||||
accountActivity: () => ["account", "activity"] as const,
|
||||
|
||||
/** Mutation keys — kept in the factory so no bare array reaches a call site. */
|
||||
accountGrantInvite: () => ["account", "grants", "invite"] as const,
|
||||
accountGrantAccept: () => ["account", "grants", "accept"] as const,
|
||||
accountGrantRevoke: () => ["account", "grants", "revoke"] as const,
|
||||
accountGrantRenounce: () => ["account", "grants", "renounce"] as const,
|
||||
accountSwitchMutation: () => ["account", "switch"] as const,
|
||||
};
|
||||
Reference in new issue
Block a user