From 3110a2a7044fbba7c8d804a4cd9d00fe56e50be1 Mon Sep 17 00:00:00 2001
From: Daniel Holmgren <dtholmgren@gmail.com>
Date: Tue, 5 Dec 2023 15:57:05 -0600
Subject: [PATCH] Cache tweaks (#1929)

* cache tweaks

* tweak

* another fix

* tweaks to expiry

* handle empty setMulti

* dont build
---
 packages/bsky/src/cache/read-through.ts   | 22 +++++++++++++++-------
 packages/bsky/src/redis.ts                | 13 +++++++++----
 packages/bsky/src/services/label/index.ts | 11 ++++++++---
 3 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/packages/bsky/src/cache/read-through.ts b/packages/bsky/src/cache/read-through.ts
index b414026e086..1d1849e8451 100644
--- a/packages/bsky/src/cache/read-through.ts
+++ b/packages/bsky/src/cache/read-through.ts
@@ -17,13 +17,18 @@ export class ReadThroughCache<T> {
   constructor(public redis: Redis, public opts: CacheOptions<T>) {}
 
   private async _fetchMany(keys: string[]): Promise<Record<string, T | null>> {
+    let result: Record<string, T | null> = {}
     if (this.opts.fetchManyMethod) {
-      return this.opts.fetchManyMethod(keys)
+      result = await this.opts.fetchManyMethod(keys)
+    } else {
+      const got = await Promise.all(keys.map((k) => this.opts.fetchMethod(k)))
+      for (let i = 0; i < keys.length; i++) {
+        result[keys[i]] = got[i] ?? null
+      }
     }
-    const got = await Promise.all(keys.map((k) => this.opts.fetchMethod(k)))
-    const result: Record<string, T | null> = {}
-    for (let i = 0; i < keys.length; i++) {
-      result[keys[i]] = got[i] ?? null
+    // ensure caching negatives
+    for (const key of keys) {
+      result[key] ??= null
     }
     return result
   }
@@ -89,9 +94,12 @@ export class ReadThroughCache<T> {
       const val = cached[key] ? (JSON.parse(cached[key]) as CacheItem<T>) : null
       if (!val || this.isExpired(val)) {
         toFetch.push(key)
-      } else if (this.isStale(val)) {
+        continue
+      }
+      if (this.isStale(val)) {
         stale.push(key)
-      } else if (val.val) {
+      }
+      if (val.val) {
         results[key] = val.val
       }
     }
diff --git a/packages/bsky/src/redis.ts b/packages/bsky/src/redis.ts
index ce9d2cecc62..3104f021e4a 100644
--- a/packages/bsky/src/redis.ts
+++ b/packages/bsky/src/redis.ts
@@ -107,9 +107,10 @@ export class Redis {
 
   async set(key: string, val: string | number, ttlMs?: number) {
     if (ttlMs !== undefined) {
-      return this.setMulti({ [key]: val })
+      await this.driver.set(this.ns(key), val, 'PX', ttlMs)
+    } else {
+      await this.driver.set(this.ns(key), val)
     }
-    await this.driver.set(this.ns(key), val)
   }
 
   async getMulti(keys: string[]) {
@@ -124,11 +125,15 @@ export class Redis {
   }
 
   async setMulti(vals: Record<string, string | number>, ttlMs?: number) {
+    if (Object.keys(vals).length === 0) {
+      return
+    }
     let builder = this.driver.multi({ pipeline: true })
     for (const key of Object.keys(vals)) {
-      builder = builder.set(this.ns(key), vals[key])
       if (ttlMs !== undefined) {
-        builder = builder.pexpire(key, ttlMs)
+        builder = builder.set(this.ns(key), vals[key], 'PX', ttlMs)
+      } else {
+        builder = builder.set(this.ns(key), vals[key])
       }
     }
     await builder.exec()
diff --git a/packages/bsky/src/services/label/index.ts b/packages/bsky/src/services/label/index.ts
index ed6691c09d0..f4c11295da7 100644
--- a/packages/bsky/src/services/label/index.ts
+++ b/packages/bsky/src/services/label/index.ts
@@ -23,7 +23,7 @@ export class LabelService {
         ...cacheOpts,
         fetchMethod: async (subject: string) => {
           const res = await fetchLabelsForSubjects(db, [subject])
-          return res[subject] ?? null
+          return res[subject] ?? []
         },
         fetchManyMethod: (subjects: string[]) =>
           fetchLabelsForSubjects(db, subjects),
@@ -192,7 +192,7 @@ const fetchLabelsForSubjects = async (
   db: Database,
   subjects: string[],
 ): Promise<Record<string, Label[]>> => {
-  if (subjects.length < 0) {
+  if (subjects.length === 0) {
     return {}
   }
   const res = await db.db
@@ -200,7 +200,7 @@ const fetchLabelsForSubjects = async (
     .where('label.uri', 'in', subjects)
     .selectAll()
     .execute()
-  return res.reduce((acc, cur) => {
+  const labelMap = res.reduce((acc, cur) => {
     acc[cur.uri] ??= []
     acc[cur.uri].push({
       ...cur,
@@ -209,4 +209,9 @@ const fetchLabelsForSubjects = async (
     })
     return acc
   }, {} as Record<string, Label[]>)
+  // ensure we cache negatives
+  for (const subject of subjects) {
+    labelMap[subject] ??= []
+  }
+  return labelMap
 }