Multiple fixes for SQLiteStore, enable them in CI (#3881)

* Multiple fixes for SQLiteStore, enable them in CI Signed-off-by: Matteo Collina <[email protected]> * Update test/cache-interceptor/sqlite-cache-store-tests.js Co-authored-by: flakey5 <[email protected]> * fixup Signed-off-by: Matteo Collina <[email protected]> --------- Signed-off-by: Matteo Collina <[email protected]> Co-authored-by: flakey5 <[email protected]>
nodejs · Nov 25, 2024 · 55ad16f · 55ad16f
1 parent ac30c29
commit 55ad16f
Show file tree

Hide file tree

Showing 4 changed files with 78 additions and 10 deletions.
diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml
@@ -37,13 +37,13 @@ jobs:
       - name: Checkout
         uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
         with:
-          persist-credentials: false          
+          persist-credentials: false
 
       - name: Setup Node.js
         uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
         with:
           node-version: lts/*
-      
+
       - name: Install dependencies
         run: npm install
 
@@ -170,19 +170,40 @@ jobs:
       - name: Checkout
         uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
         with:
-          persist-credentials: false          
+          persist-credentials: false
 
       - name: Setup Node.js
         uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
         with:
           node-version: lts/*
-      
+
       - name: Install dependencies
         run: npm install
 
       - name: Run typings tests
         run: npm run test:typescript
 
+  test-sqlite:
+    name: Test with SQLite enabled
+    timeout-minutes: 15
+    runs-on: ubuntu-latest
+    steps:
+      - name: Checkout
+        uses: actions/checkout@9bb56186c3b09b4f86b1c65136769dd318469633 # v4.1.2
+        with:
+          persist-credentials: false
+
+      - name: Setup Node.js
+        uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2
+        with:
+          node-version: 22
+
+      - name: Install dependencies
+        run: npm install
+
+      - name: Run typings tests
+        run: npm run test:sqlite
+
   automerge:
     if: >
       github.event_name == 'pull_request' && github.event.pull_request.user.login == 'dependabot[bot]'

diff --git a/lib/cache/sqlite-cache-store.js b/lib/cache/sqlite-cache-store.js
@@ -274,7 +274,7 @@ class SqliteCacheStore {
             value.statusCode,
             value.statusMessage,
             value.headers ? JSON.stringify(value.headers) : null,
-            value.etag,
+            value.etag ? value.etag : null,
             value.cachedAt,
             value.staleAt,
             value.deleteAt,
@@ -316,7 +316,7 @@ class SqliteCacheStore {
   }
 
   #prune () {
-    if (this.#size <= this.#maxCount) {
+    if (this.size <= this.#maxCount) {
       return 0
     }
 
@@ -341,7 +341,7 @@ class SqliteCacheStore {
    * Counts the number of rows in the cache
    * @returns {Number}
    */
-  get #size () {
+  get size () {
     const { total } = this.#countEntriesQuery.get()
     return total
   }
@@ -388,7 +388,7 @@ class SqliteCacheStore {
         const vary = JSON.parse(value.vary)
 
         for (const header in vary) {
-          if (headerValueEquals(headers[header], vary[header])) {
+          if (!headerValueEquals(headers[header], vary[header])) {
             matches = false
             break
           }

diff --git a/package.json b/package.json
@@ -73,6 +73,7 @@
     "test:javascript:without-intl": "npm run test:javascript:no-jest",
     "test:busboy": "borp -p \"test/busboy/*.js\"",
     "test:cache": "borp -p \"test/cache/*.js\"",
+    "test:sqlite": "NODE_OPTIONS=--experimental-sqlite borp -p \"test/cache-interceptor/*.js\"",
     "test:cache-interceptor": "borp -p \"test/cache-interceptor/*.js\"",
     "test:cookies": "borp -p \"test/cookie/*.js\"",
     "test:eventsource": "npm run build:node && borp --expose-gc -p \"test/eventsource/*.js\"",

diff --git a/test/cache-interceptor/sqlite-cache-store-tests.js b/test/cache-interceptor/sqlite-cache-store-tests.js
@@ -52,7 +52,7 @@ test('SqliteCacheStore works nicely with multiple stores', async (t) => {
   const requestValue = {
     statusCode: 200,
     statusMessage: '',
-    rawHeaders: [Buffer.from('1'), Buffer.from('2'), Buffer.from('3')],
+    headers: { foo: 'bar' },
     cachedAt: Date.now(),
     staleAt: Date.now() + 10000,
     deleteAt: Date.now() + 20000
@@ -111,7 +111,7 @@ test('SqliteCacheStore maxEntries', async (t) => {
     const requestValue = {
       statusCode: 200,
       statusMessage: '',
-      rawHeaders: [Buffer.from('1'), Buffer.from('2'), Buffer.from('3')],
+      headers: { foo: 'bar' },
       cachedAt: Date.now(),
       staleAt: Date.now() + 10000,
       deleteAt: Date.now() + 20000
@@ -125,3 +125,49 @@ test('SqliteCacheStore maxEntries', async (t) => {
 
   strictEqual(store.size <= 11, true)
 })
+
+test('two writes', async (t) => {
+  if (!hasSqlite) {
+    t.skip()
+    return
+  }
+
+  const SqliteCacheStore = require('../../lib/cache/sqlite-cache-store.js')
+  const sqliteLocation = 'cache-interceptor.sqlite'
+
+  const store = new SqliteCacheStore({
+    location: sqliteLocation,
+    maxCount: 10
+  })
+
+  t.after(async () => {
+    await rm(sqliteLocation)
+  })
+
+  const request = {
+    origin: 'localhost',
+    path: '/',
+    method: 'GET',
+    headers: {}
+  }
+
+  const requestValue = {
+    statusCode: 200,
+    statusMessage: '',
+    headers: { foo: 'bar' },
+    cachedAt: Date.now(),
+    staleAt: Date.now() + 10000,
+    deleteAt: Date.now() + 20000
+  }
+  const requestBody = ['asd', '123']
+
+  {
+    const writable = store.createWriteStream(request, requestValue)
+    await once(writeResponse(writable, requestBody), 'close')
+  }
+
+  {
+    const writable = store.createWriteStream(request, requestValue)
+    await once(writeResponse(writable, requestBody), 'close')
+  }
+})