Merge pull request #574 from psteinroe/fix/only-store-nonempty-results

fix: only store nonempty results

Author: psteinroe

.changeset/fifty-oranges-hope.md

@@ -0,0 +1,5 @@
+---
+"@supabase-cache-helpers/postgrest-server": patch
+---
+
+fix: only store non-empty results

packages/postgrest-server/src/query-cache.ts

@@ -11,6 +11,7 @@ import { Value } from './stores/entry';
 import { Store } from './stores/interface';
 import { SwrCache } from './swr-cache';
 import { TieredStore } from './tiered-store';
+import { isEmpty } from './utils';
 
 export type QueryCacheOpts = {
   stores: Store[];
@@ -84,7 +85,7 @@ export class QueryCache {
 
     const result = await this.dedupeQuery(query);
 
-    if (!opts?.store || opts.store(result)) {
+    if (!isEmpty(result) && (!opts?.store || opts.store(result))) {
       await this.inner.set(key, result, opts);
     }
 

packages/postgrest-server/src/swr-cache.ts

@@ -1,6 +1,8 @@
+import { PostgrestResponse } from '@supabase/postgrest-js';
 import type { Context } from './context';
 import { Value } from './stores/entry';
 import { Store } from './stores/interface';
+import { isEmpty } from './utils';
 
 export type SwrCacheOpts = {
   ctx: Context;
@@ -97,15 +99,21 @@ export class SwrCache {
     if (typeof value !== 'undefined') {
       if (revalidate) {
         this.ctx.waitUntil(
-          loadFromOrigin(key).then((res) => this.set(key, res, opts)),
+          loadFromOrigin(key).then((res) => {
+            if (!isEmpty(res)) {
+              this.set(key, res, opts);
+            }
+          }),
         );
       }
 
       return value;
     }
 
     const loadedValue = await loadFromOrigin(key);
-    this.ctx.waitUntil(this.set(key, loadedValue));
+    if (!isEmpty(loadedValue)) {
+      this.ctx.waitUntil(this.set(key, loadedValue));
+    }
     return loadedValue;
   }
 }

packages/postgrest-server/src/utils.ts

@@ -0,0 +1,27 @@
+import { Value } from './stores/entry';
+
+/**
+ * A result is empty if
+ * - it does not contain a truhty data field
+ * - it does not contain a count field
+ * - data is an empty array
+ *
+ * @template Result - The Result of the query
+ * @param result - The value to check
+ * @returns true if the result is empty
+ */
+export function isEmpty<Result>(result: Value<Result>) {
+  if (typeof result.count === 'number') {
+    return false;
+  }
+
+  if (!result.data) {
+    return true;
+  }
+
+  if (Array.isArray(result.data)) {
+    return result.data.length === 0;
+  }
+
+  return false;
+}

packages/postgrest-server/tests/query-cache.test.ts

@@ -119,6 +119,56 @@ describe('QueryCache', () => {
       expect(spy).toHaveBeenCalledTimes(1);
     });
 
+    it('should store count-only queries', async () => {
+      const map = new Map();
+
+      const cache = new QueryCache(ctx, {
+        stores: [new MemoryStore({ persistentMap: map })],
+        fresh: 1000,
+        stale: 2000,
+      });
+
+      const query = client
+        .from('contact')
+        .select('id,username', { count: 'exact', head: true })
+        .ilike('username', `${testRunPrefix}%`);
+
+      const spy = vi.spyOn(query, 'then');
+
+      const res = await cache.query(query);
+
+      const res2 = await cache.query(query);
+
+      expect(res.count).toEqual(4);
+      expect(res2.count).toEqual(4);
+      expect(spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not store empty results', async () => {
+      const map = new Map();
+
+      const cache = new QueryCache(ctx, {
+        stores: [new MemoryStore({ persistentMap: map })],
+        fresh: 1000,
+        stale: 2000,
+      });
+
+      const query = client
+        .from('contact')
+        .select('id,username')
+        .eq('username', 'unknown')
+        .maybeSingle();
+
+      const spy = vi.spyOn(query, 'then');
+
+      const res = await cache.query(query);
+      const res2 = await cache.query(query);
+
+      expect(res.data).toBeFalsy();
+      expect(res2.data).toBeFalsy();
+      expect(spy).toHaveBeenCalledTimes(2);
+    });
+
     it('should not store result if store() returns false', async () => {
       const map = new Map();
 
@@ -249,6 +299,55 @@ describe('QueryCache', () => {
       expect(res2.count).toEqual(4);
       expect(spy).toHaveBeenCalledTimes(1);
     });
+
+    it('should store count-only queries', async () => {
+      const map = new Map();
+
+      const cache = new QueryCache(ctx, {
+        stores: [new MemoryStore({ persistentMap: map })],
+        fresh: 1000,
+        stale: 2000,
+      });
+
+      const query = client
+        .from('contact')
+        .select('id,username', { count: 'exact', head: true })
+        .ilike('username', `${testRunPrefix}%`);
+
+      const spy = vi.spyOn(query, 'then');
+
+      const res = await cache.swr(query);
+
+      const res2 = await cache.swr(query);
+
+      expect(res.count).toEqual(4);
+      expect(res2.count).toEqual(4);
+      expect(spy).toHaveBeenCalledTimes(1);
+    });
+
+    it('should not store empty results', async () => {
+      const map = new Map();
+
+      const cache = new QueryCache(ctx, {
+        stores: [new MemoryStore({ persistentMap: map })],
+        fresh: 1000,
+        stale: 2000,
+      });
+
+      const query = client
+        .from('contact')
+        .select('id,username')
+        .ilike('username', 'unknown');
+
+      const spy = vi.spyOn(query, 'then');
+
+      const res = await cache.swr(query);
+      const res2 = await cache.swr(query);
+
+      expect(res.data).toEqual([]);
+      expect(res2.data).toEqual([]);
+      expect(spy).toHaveBeenCalledTimes(2);
+    });
   });
 
   it('should dedupe', async () => {