feat: sqlite add set and minor cleanup (#4018)

* feat: sqlite add set and minor cleanup

* fixup

Author: ronag

lib/cache/sqlite-cache-store.js

@@ -15,11 +15,18 @@ const MAX_ENTRY_SIZE = 2 * 1000 * 1000 * 1000
  * @implements {CacheStore}
  *
  * @typedef {{
- *  id: Readonly<number>
- *  headers?: Record<string, string | string[]>
- *  vary?: string | object
- *  body: string
- * } & import('../../types/cache-interceptor.d.ts').default.CacheValue} SqliteStoreValue
+ *  id: Readonly<number>,
+ *  body?: Uint8Array
+ *  statusCode: number
+ *  statusMessage: string
+ *  headers?: string
+ *  vary?: string
+ *  etag?: string
+ *  cacheControlDirectives?: string
+ *  cachedAt: number
+ *  staleAt: number
+ *  deleteAt: number
+ * }} SqliteStoreValue
  */
 module.exports = class SqliteCacheStore {
   #maxEntrySize = MAX_ENTRY_SIZE
@@ -217,36 +224,78 @@ module.exports = class SqliteCacheStore {
 
   /**
    * @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} key
-   * @returns {import('../../types/cache-interceptor.d.ts').default.GetResult | undefined}
+   * @returns {(import('../../types/cache-interceptor.d.ts').default.GetResult & { body?: Buffer }) | undefined}
    */
   get (key) {
     assertCacheKey(key)
 
     const value = this.#findValue(key)
+    return value
+      ? {
+          body: value.body ? Buffer.from(value.body.buffer) : undefined,
+          statusCode: value.statusCode,
+          statusMessage: value.statusMessage,
+          headers: value.headers ? JSON.parse(value.headers) : undefined,
+          etag: value.etag ? value.etag : undefined,
+          vary: value.vary ? JSON.parse(value.vary) : undefined,
+          cacheControlDirectives: value.cacheControlDirectives
+            ? JSON.parse(value.cacheControlDirectives)
+            : undefined,
+          cachedAt: value.cachedAt,
+          staleAt: value.staleAt,
+          deleteAt: value.deleteAt
+        }
+      : undefined
+  }
 
-    if (!value) {
-      return undefined
-    }
+  /**
+   * @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} key
+   * @param {import('../../types/cache-interceptor.d.ts').default.CacheValue & { body: null | Buffer | Array<Buffer>}} value
+   */
+  set (key, value) {
+    assertCacheKey(key)
 
-    /**
-     * @type {import('../../types/cache-interceptor.d.ts').default.GetResult}
-     */
-    const result = {
-      body: Buffer.from(value.body),
-      statusCode: value.statusCode,
-      statusMessage: value.statusMessage,
-      headers: value.headers ? JSON.parse(value.headers) : undefined,
-      etag: value.etag ? value.etag : undefined,
-      vary: value.vary ?? undefined,
-      cacheControlDirectives: value.cacheControlDirectives
-        ? JSON.parse(value.cacheControlDirectives)
-        : undefined,
-      cachedAt: value.cachedAt,
-      staleAt: value.staleAt,
-      deleteAt: value.deleteAt
+    const url = this.#makeValueUrl(key)
+    const body = Array.isArray(value.body) ? Buffer.concat(value.body) : value.body
+    const size = body?.byteLength
+
+    if (size && size > this.#maxEntrySize) {
+      return
     }
 
-    return result
+    const existingValue = this.#findValue(key, true)
+    if (existingValue) {
+      // Updating an existing response, let's overwrite it
+      this.#updateValueQuery.run(
+        body,
+        value.deleteAt,
+        value.statusCode,
+        value.statusMessage,
+        value.headers ? JSON.stringify(value.headers) : null,
+        value.etag ? value.etag : null,
+        value.cacheControlDirectives ? JSON.stringify(value.cacheControlDirectives) : null,
+        value.cachedAt,
+        value.staleAt,
+        existingValue.id
+      )
+    } else {
+      this.#prune()
+      // New response, let's insert it
+      this.#insertValueQuery.run(
+        url,
+        key.method,
+        body,
+        value.deleteAt,
+        value.statusCode,
+        value.statusMessage,
+        value.headers ? JSON.stringify(value.headers) : null,
+        value.etag ? value.etag : null,
+        value.cacheControlDirectives ? JSON.stringify(value.cacheControlDirectives) : null,
+        value.vary ? JSON.stringify(value.vary) : null,
+        value.cachedAt,
+        value.staleAt
+      )
+    }
   }
 
   /**
@@ -258,7 +307,6 @@ module.exports = class SqliteCacheStore {
     assertCacheKey(key)
     assertCacheValue(value)
 
-    const url = this.#makeValueUrl(key)
     let size = 0
     /**
      * @type {Buffer[] | null}
@@ -267,11 +315,8 @@ module.exports = class SqliteCacheStore {
     const store = this
 
     return new Writable({
+      decodeStrings: true,
       write (chunk, encoding, callback) {
-        if (typeof chunk === 'string') {
-          chunk = Buffer.from(chunk, encoding)
-        }
-
         size += chunk.byteLength
 
         if (size < store.#maxEntrySize) {
@@ -283,40 +328,7 @@ module.exports = class SqliteCacheStore {
         callback()
       },
       final (callback) {
-        const existingValue = store.#findValue(key, true)
-        if (existingValue) {
-          // Updating an existing response, let's overwrite it
-          store.#updateValueQuery.run(
-            Buffer.concat(body),
-            value.deleteAt,
-            value.statusCode,
-            value.statusMessage,
-            value.headers ? JSON.stringify(value.headers) : null,
-            value.etag ? value.etag : null,
-            value.cacheControlDirectives ? JSON.stringify(value.cacheControlDirectives) : null,
-            value.cachedAt,
-            value.staleAt,
-            existingValue.id
-          )
-        } else {
-          store.#prune()
-          // New response, let's insert it
-          store.#insertValueQuery.run(
-            url,
-            key.method,
-            Buffer.concat(body),
-            value.deleteAt,
-            value.statusCode,
-            value.statusMessage,
-            value.headers ? JSON.stringify(value.headers) : null,
-            value.etag ? value.etag : null,
-            value.cacheControlDirectives ? JSON.stringify(value.cacheControlDirectives) : null,
-            value.vary ? JSON.stringify(value.vary) : null,
-            value.cachedAt,
-            value.staleAt
-          )
-        }
-
+        store.set(key, { ...value, body })
         callback()
       }
     })
@@ -375,7 +387,7 @@ module.exports = class SqliteCacheStore {
   /**
    * @param {import('../../types/cache-interceptor.d.ts').default.CacheKey} key
    * @param {boolean} [canBeExpired=false]
-   * @returns {(SqliteStoreValue & { vary?: Record<string, string[]> }) | undefined}
+   * @returns {SqliteStoreValue | undefined}
    */
   #findValue (key, canBeExpired = false) {
     const url = this.#makeValueUrl(key)
@@ -403,10 +415,10 @@ module.exports = class SqliteCacheStore {
           return undefined
         }
 
-        value.vary = JSON.parse(value.vary)
+        const vary = JSON.parse(value.vary)
 
-        for (const header in value.vary) {
-          if (!headerValueEquals(headers[header], value.vary[header])) {
+        for (const header in vary) {
+          if (!headerValueEquals(headers[header], vary[header])) {
             matches = false
             break
           }

test/cache-interceptor/sqlite-cache-store-tests.js

@@ -1,7 +1,7 @@
 'use strict'
 
 const { test, skip } = require('node:test')
-const { notEqual, strictEqual } = require('node:assert')
+const { notEqual, strictEqual, deepStrictEqual } = require('node:assert')
 const { rm } = require('node:fs/promises')
 const { cacheStoreTests, writeBody, compareGetResults } = require('./cache-store-test-utils.js')
 
@@ -179,3 +179,46 @@ test('SqliteCacheStore two writes', async (t) => {
     writeBody(writable, body)
   }
 })
+
+test('SqliteCacheStore write & read', async (t) => {
+  if (!hasSqlite) {
+    t.skip()
+    return
+  }
+
+  const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
+
+  const store = new SqliteCacheStore({
+    maxCount: 10
+  })
+
+  /**
+   * @type {import('../../types/cache-interceptor.d.ts').default.CacheKey}
+   */
+  const key = {
+    origin: 'localhost',
+    path: '/',
+    method: 'GET',
+    headers: {}
+  }
+
+  /**
+   * @type {import('../../types/cache-interceptor.d.ts').default.CacheValue & { body: Buffer }}
+   */
+  const value = {
+    statusCode: 200,
+    statusMessage: '',
+    headers: { foo: 'bar' },
+    cacheControlDirectives: { 'max-stale': 0 },
+    cachedAt: Date.now(),
+    staleAt: Date.now() + 10000,
+    deleteAt: Date.now() + 20000,
+    body: Buffer.from('asd'),
+    etag: undefined,
+    vary: undefined
+  }
+
+  store.set(key, value)
+
+  deepStrictEqual(store.get(key), value)
+})

types/cache-interceptor.d.ts

@@ -90,7 +90,7 @@ declare namespace CacheHandler {
     headers: Record<string, string | string[]>
     vary?: Record<string, string | string[]>
     etag?: string
-    body: null | Readable | Iterable<Buffer> | AsyncIterable<Buffer> | Buffer | Iterable<string> | AsyncIterable<string> | string
+    body?: Readable | Iterable<Buffer> | AsyncIterable<Buffer> | Buffer | Iterable<string> | AsyncIterable<string> | string
     cacheControlDirectives: CacheControlDirectives,
     cachedAt: number
     staleAt: number