diff --git a/src/lib/cache.test.ts b/src/lib/cache.test.ts index 993608d..84e3335 100644 --- a/src/lib/cache.test.ts +++ b/src/lib/cache.test.ts @@ -4,10 +4,12 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { + ACCOUNT_SCOPES, POLICIES, type Scope, cached, invalidate, + invalidateAccount, invalidateSubject, peek, put, @@ -15,25 +17,28 @@ import { } from './cache' let store = new Map() +let local = new Map() + +/** One storage area over its own Map, so a test can tell them apart. */ +function area(backing: Map) { + return { + get: async (keys: string | null) => { + if (keys === null) return Object.fromEntries(backing) + return backing.has(keys) ? { [keys]: backing.get(keys) } : {} + }, + set: async (items: Record) => { + for (const [k, v] of Object.entries(items)) backing.set(k, v) + }, + remove: async (keys: string | string[]) => { + for (const k of [keys].flat()) backing.delete(k) + }, + } +} beforeEach(() => { store = new Map() - vi.stubGlobal('chrome', { - storage: { - session: { - get: async (keys: string | null) => { - if (keys === null) return Object.fromEntries(store) - return store.has(keys) ? { [keys]: store.get(keys) } : {} - }, - set: async (items: Record) => { - for (const [k, v] of Object.entries(items)) store.set(k, v) - }, - remove: async (keys: string | string[]) => { - for (const k of [keys].flat()) store.delete(k) - }, - }, - }, - }) + local = new Map() + vi.stubGlobal('chrome', { storage: { session: area(store), local: area(local) } }) }) afterEach(() => { @@ -97,6 +102,48 @@ describe('cached', () => { }) }) +describe('the graph scope', () => { + it('writes to disk, where a browser restart cannot take it', async () => { + await cached(key('graph', 'did:plc:me', 'follows'), async () => ['did:plc:you']) + expect(local.get('graph:did:plc:me/follows')).toBeDefined() + // Everything else stays in the area the browser clears on close. + expect(store.size).toBe(0) + }) + + it('keeps a follow set for a day, not for the hour a write-backed scope gets', async () => { + vi.useFakeTimers() + const load = vi.fn(async () => ['did:plc:you']) + await cached(key('graph', 'did:plc:me', 'follows'), load) + vi.advanceTimersByTime(POLICIES.own.ttlMs + 1) + await cached(key('graph', 'did:plc:me', 'follows'), load) + expect(load).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(POLICIES.graph.ttlMs) + await cached(key('graph', 'did:plc:me', 'follows'), load) + expect(load).toHaveBeenCalledTimes(2) + }) +}) + +describe('invalidateAccount', () => { + it('drops every scope keyed by the account, not just the one', async () => { + await put(key('own', 'did:plc:me', 'subs'), 1) + await put(key('graph', 'did:plc:me', 'follows'), ['did:plc:you']) + await put(key('world', 'at://pub', 'labels'), 2) + + await invalidateAccount('did:plc:me') + + expect(await peek(key('own', 'did:plc:me', 'subs'))).toBeUndefined() + // The one a sign-out would otherwise leave on disk for the next account. + expect(await peek(key('graph', 'did:plc:me', 'follows'))).toBeUndefined() + expect(await peek(key('world', 'at://pub', 'labels'))).toBe(2) + }) + + it('covers every account scope the policies declare', () => { + const keyed = (Object.keys(POLICIES) as Scope[]).filter((s) => s !== 'world') + expect([...ACCOUNT_SCOPES].sort()).toEqual(keyed.sort()) + }) +}) + describe('peek', () => { it('answers from the store or not at all', async () => { expect(await peek(key('world', 'at://pub', 'labels'))).toBeUndefined() diff --git a/src/lib/cache.ts b/src/lib/cache.ts index f9879e3..290a9ec 100644 --- a/src/lib/cache.ts +++ b/src/lib/cache.ts @@ -9,16 +9,33 @@ // The answer does not actually depend on which feature is asking. It depends // on whose data it is: // -// - `own` — the signed-in account's own repo, read from its PDS: its -// subscriptions, follows, blocks, labeler subscriptions. It changes when -// the person changes it, and when they change it *here* we know, so this -// can be cached for an hour and dropped on the write. What it must never -// do is show somebody their own action not having happened. +// - `own` — the signed-in account's own repo, where this extension also +// writes: its subscriptions, blocks, labeler subscriptions. It changes +// when the person changes it, and when they change it *here* we know, so +// this can be cached for an hour and dropped on the write. What it must +// never do is show somebody their own action not having happened. +// - `graph` — the signed-in account's own repo, which this extension only +// ever reads: the follow list. See below; it is the one scope on disk. // - `world` — somebody else's publication, profile or reputation: the owner // card, subscriber counts, labels. Nothing the reader does changes it, and // it changes slowly at the source, so a day is fine and a stale day costs // a slightly out-of-date number. // +// `graph` is `own` data with the write taken away, and both halves of that +// matter. Nothing in this extension creates or deletes an `app.bsky.graph.follow` +// record, so a stale follow set cannot show somebody their own action not +// having happened — the rule the hour exists to keep. It can only lag what +// they did in another client, and it costs a face missing from a decorative +// row until tomorrow. That buys the day. +// +// The day is also why it is the one thing written to disk. The walk is one +// request per hundred follows against the account's own PDS, and an account +// following fifty thousand people is five hundred of them; paying that again +// every time the browser restarts is not a cache. It is allowed on disk +// because of what it is not: a follow list is the account's own public social +// graph, and unlike everything under `world` it records nothing about which +// sites were visited to produce it. +// // Detection's well-known probe cache is deliberately not here. Its TTL // depends on the answer rather than on the scope — five minutes for a hit, // an hour for the miss nearly every origin gives — and it also dedups probes @@ -26,29 +43,31 @@ // its own policy in src/lib/detection.ts, where that reasoning is written // down. // -// Everything lives in `chrome.storage.session`, which the browser clears when -// it closes. So a day-long TTL means "up to a day, within this browsing -// session" — nothing about which publications were visited is written to -// disk. Moving `world` to `storage.local` would make the day literal at that -// price; it is one word here, and deliberately not taken. +// `own` and `world` live in `chrome.storage.session`, which the browser clears +// when it closes. So a day-long TTL there means "up to a day, within this +// browsing session" — nothing about which publications were visited is written +// to disk. Moving `world` to `storage.local` would make its day literal at +// that price; it is one word here, and deliberately not taken. /** Whose data it is, which is what decides how long it keeps. */ -export type Scope = 'own' | 'world' +export type Scope = 'own' | 'graph' | 'world' export interface Policy { ttlMs: number - /** Session only, for now: see the note above before changing one. */ - area: 'session' + /** `local` only where the value carries no record of where the user has been. */ + area: 'session' | 'local' } export const POLICIES: Record = { own: { ttlMs: 60 * 60 * 1000, area: 'session' }, + graph: { ttlMs: 24 * 60 * 60 * 1000, area: 'local' }, world: { ttlMs: 24 * 60 * 60 * 1000, area: 'session' }, } /** * What a cached value is about. `subject` is the thing it describes — an - * account DID for `own`, a publication at-uri or origin for `world` — and it + * account DID for `own` and `graph`, a publication at-uri or origin for + * `world` — and it * is a separate field because dropping everything cached about one subject * (signing out, switching account) has to be one call, not a list of names * kept in sync by hand. @@ -132,6 +151,21 @@ export async function invalidate(key: CacheKey): Promise { * out and for switching account, where the next answer must not be the * previous account's. */ +/** + * Everything cached from one account's own repo, across every scope keyed by + * a DID. For signing out and for switching account. + * + * A list of scopes rather than one call, and in here rather than at the two + * call sites, because the cost of getting it wrong is the new account being + * shown the old one's follows — and the way to get it wrong is to add an + * account scope and not find every place that had to be told about it. + */ +export const ACCOUNT_SCOPES: Scope[] = ['own', 'graph'] + +export async function invalidateAccount(did: string): Promise { + await Promise.all(ACCOUNT_SCOPES.map((scope) => invalidateSubject(scope, did))) +} + export async function invalidateSubject(scope: Scope, subject: string): Promise { const storage = area(scope) const prefix = `${scope}:${subject}/` diff --git a/src/popup/popup.ts b/src/popup/popup.ts index 4657c09..77b63bf 100644 --- a/src/popup/popup.ts +++ b/src/popup/popup.ts @@ -1,7 +1,7 @@ import 'actor-typeahead' import { parseAtUri } from '../lib/atproto' import { AUTH_ERROR_KEY } from '../lib/authflow' -import { invalidate, invalidateSubject } from '../lib/cache' +import { invalidate, invalidateAccount } from '../lib/cache' import { BOARD_URL, type BoardSpace, @@ -79,7 +79,7 @@ async function invalidateSubsCache() { * the next account's answers must not be the previous one's. */ async function invalidateAccountCaches() { - if (session) await invalidateSubject('own', session.did) + if (session) await invalidateAccount(session.did) } async function init() { diff --git a/src/signin.ts b/src/signin.ts index 74ec5e1..820a0f5 100644 --- a/src/signin.ts +++ b/src/signin.ts @@ -9,7 +9,7 @@ // level so their events revive the worker. import { AUTH_ERROR_KEY, callbackFromUpdate, oauthRedirectUri, replacedSession } from './lib/authflow' -import { invalidateSubject } from './lib/cache' +import { invalidateAccount } from './lib/cache' import { ensureOffscreenDocument } from './lib/offscreen' import { getStoredSession, saveSessionMirror } from './lib/session-store' import type { OffscreenMsg, SessionInfo } from './lib/types' @@ -120,7 +120,7 @@ async function onConsentTabUpdated( const replaced = replacedSession(await getStoredSession(), info) if (replaced) { await toOffscreen({ target: 'offscreen', type: 'oauth-revoke', sub: replaced }) - await invalidateSubject('own', replaced) + await invalidateAccount(replaced) console.debug('[substandard] revoked replaced session', replaced) } // The offscreen document cannot persist this itself (no chrome.storage