From bb039d8e4ce5b7f70c4f3e86d1327e210ef24dc3 Mon Sep 17 00:00:00 2001 From: Daniel Holmgren Date: Wed, 25 Oct 2023 15:53:08 -0500 Subject: [PATCH] Dedupe did cache refreshes (#1773) * dedupe refreshes to did cache * handle expired cache entries as well * apply in pds as well * changeset --- .changeset/new-snails-hope.md | 5 +++ packages/bsky/src/did-cache.ts | 45 ++++++++++++++-------- packages/identity/src/did/base-resolver.ts | 29 +++++++++----- packages/identity/src/did/memory-cache.ts | 3 +- packages/identity/src/types.ts | 8 +++- packages/pds/src/did-cache.ts | 44 +++++++++++++-------- 6 files changed, 88 insertions(+), 46 deletions(-) create mode 100644 .changeset/new-snails-hope.md diff --git a/.changeset/new-snails-hope.md b/.changeset/new-snails-hope.md new file mode 100644 index 00000000000..2526d0836ed --- /dev/null +++ b/.changeset/new-snails-hope.md @@ -0,0 +1,5 @@ +--- +'@atproto/identity': minor +--- + +Pass stale did doc into refresh cache functions diff --git a/packages/bsky/src/did-cache.ts b/packages/bsky/src/did-cache.ts index b161ff73fe3..e08b09ca7e7 100644 --- a/packages/bsky/src/did-cache.ts +++ b/packages/bsky/src/did-cache.ts @@ -17,28 +17,42 @@ export class DidSqlCache implements DidCache { this.pQueue = new PQueue() } - async cacheDid(did: string, doc: DidDocument): Promise { - await this.db.db - .insertInto('did_cache') - .values({ did, doc, updatedAt: Date.now() }) - .onConflict((oc) => - oc.column('did').doUpdateSet({ - doc: excluded(this.db.db, 'doc'), - updatedAt: excluded(this.db.db, 'updatedAt'), - }), - ) - .executeTakeFirst() + async cacheDid( + did: string, + doc: DidDocument, + prevResult?: CacheResult, + ): Promise { + if (prevResult) { + await this.db.db + .updateTable('did_cache') + .set({ doc, updatedAt: Date.now() }) + .where('did', '=', did) + .where('updatedAt', '=', prevResult.updatedAt) + .execute() + } else { + await this.db.db + .insertInto('did_cache') + .values({ did, doc, updatedAt: Date.now() }) + .onConflict((oc) => + oc.column('did').doUpdateSet({ + doc: excluded(this.db.db, 'doc'), + updatedAt: excluded(this.db.db, 'updatedAt'), + }), + ) + .executeTakeFirst() + } } async refreshCache( did: string, getDoc: () => Promise, + prevResult?: CacheResult, ): Promise { this.pQueue?.add(async () => { try { const doc = await getDoc() if (doc) { - await this.cacheDid(did, doc) + await this.cacheDid(did, doc, prevResult) } else { await this.clearEntry(did) } @@ -55,20 +69,17 @@ export class DidSqlCache implements DidCache { .selectAll() .executeTakeFirst() if (!res) return null + const now = Date.now() const updatedAt = new Date(res.updatedAt).getTime() - const expired = now > updatedAt + this.maxTTL - if (expired) { - return null - } - const stale = now > updatedAt + this.staleTTL return { doc: res.doc, updatedAt, did, stale, + expired, } } diff --git a/packages/identity/src/did/base-resolver.ts b/packages/identity/src/did/base-resolver.ts index fb3d7bc57f8..765f354213c 100644 --- a/packages/identity/src/did/base-resolver.ts +++ b/packages/identity/src/did/base-resolver.ts @@ -1,6 +1,12 @@ import * as crypto from '@atproto/crypto' import { check } from '@atproto/common-web' -import { DidCache, AtprotoData, DidDocument, didDocument } from '../types' +import { + DidCache, + AtprotoData, + DidDocument, + didDocument, + CacheResult, +} from '../types' import * as atprotoData from './atproto-data' import { DidNotFoundError, PoorlyFormattedDidDocumentError } from '../errors' @@ -25,20 +31,25 @@ export abstract class BaseResolver { return this.validateDidDoc(did, got) } - async refreshCache(did: string): Promise { - await this.cache?.refreshCache(did, () => this.resolveNoCache(did)) + async refreshCache(did: string, prevResult?: CacheResult): Promise { + await this.cache?.refreshCache( + did, + () => this.resolveNoCache(did), + prevResult, + ) } async resolve( did: string, forceRefresh = false, ): Promise { + let fromCache: CacheResult | null = null if (this.cache && !forceRefresh) { - const fromCache = await this.cache.checkCache(did) - if (fromCache?.stale) { - await this.refreshCache(did) - } - if (fromCache) { + fromCache = await this.cache.checkCache(did) + if (fromCache && !fromCache.expired) { + if (fromCache?.stale) { + await this.refreshCache(did, fromCache) + } return fromCache.doc } } @@ -48,7 +59,7 @@ export abstract class BaseResolver { await this.cache?.clearEntry(did) return null } - await this.cache?.cacheDid(did, got) + await this.cache?.cacheDid(did, got, fromCache ?? undefined) return got } diff --git a/packages/identity/src/did/memory-cache.ts b/packages/identity/src/did/memory-cache.ts index c5ab8c4ec8d..42f01527529 100644 --- a/packages/identity/src/did/memory-cache.ts +++ b/packages/identity/src/did/memory-cache.ts @@ -35,13 +35,12 @@ export class MemoryCache implements DidCache { if (!val) return null const now = Date.now() const expired = now > val.updatedAt + this.maxTTL - if (expired) return null - const stale = now > val.updatedAt + this.staleTTL return { ...val, did, stale, + expired, } } diff --git a/packages/identity/src/types.ts b/packages/identity/src/types.ts index f1d983e6742..a3604f72f73 100644 --- a/packages/identity/src/types.ts +++ b/packages/identity/src/types.ts @@ -30,14 +30,20 @@ export type CacheResult = { doc: DidDocument updatedAt: number stale: boolean + expired: boolean } export interface DidCache { - cacheDid(did: string, doc: DidDocument): Promise + cacheDid( + did: string, + doc: DidDocument, + prevResult?: CacheResult, + ): Promise checkCache(did: string): Promise refreshCache( did: string, getDoc: () => Promise, + prevResult?: CacheResult, ): Promise clearEntry(did: string): Promise clear(): Promise diff --git a/packages/pds/src/did-cache.ts b/packages/pds/src/did-cache.ts index dce0252bb0f..ac719fe922c 100644 --- a/packages/pds/src/did-cache.ts +++ b/packages/pds/src/did-cache.ts @@ -15,28 +15,42 @@ export class DidSqlCache implements DidCache { this.pQueue = new PQueue() } - async cacheDid(did: string, doc: DidDocument): Promise { - await this.db.db - .insertInto('did_cache') - .values({ did, doc: JSON.stringify(doc), updatedAt: Date.now() }) - .onConflict((oc) => - oc.column('did').doUpdateSet({ - doc: excluded(this.db.db, 'doc'), - updatedAt: excluded(this.db.db, 'updatedAt'), - }), - ) - .executeTakeFirst() + async cacheDid( + did: string, + doc: DidDocument, + prevResult?: CacheResult, + ): Promise { + if (prevResult) { + await this.db.db + .updateTable('did_cache') + .set({ doc: JSON.stringify(doc), updatedAt: Date.now() }) + .where('did', '=', did) + .where('updatedAt', '=', prevResult.updatedAt) + .execute() + } else { + await this.db.db + .insertInto('did_cache') + .values({ did, doc: JSON.stringify(doc), updatedAt: Date.now() }) + .onConflict((oc) => + oc.column('did').doUpdateSet({ + doc: excluded(this.db.db, 'doc'), + updatedAt: excluded(this.db.db, 'updatedAt'), + }), + ) + .executeTakeFirst() + } } async refreshCache( did: string, getDoc: () => Promise, + prevResult?: CacheResult, ): Promise { this.pQueue?.add(async () => { try { const doc = await getDoc() if (doc) { - await this.cacheDid(did, doc) + await this.cacheDid(did, doc, prevResult) } else { await this.clearEntry(did) } @@ -55,18 +69,14 @@ export class DidSqlCache implements DidCache { if (!res) return null const now = Date.now() const updatedAt = new Date(res.updatedAt).getTime() - const expired = now > updatedAt + this.maxTTL - if (expired) { - return null - } - const stale = now > updatedAt + this.staleTTL return { doc: JSON.parse(res.doc) as DidDocument, updatedAt, did, stale, + expired, } }