fix: isolate full and corgi packuments in packumentCache (#369)

lib/registry.js

@@ -20,6 +20,7 @@ const fullDoc = 'application/json'
 const MISSING_TIME_CUTOFF = '2015-01-01T00:00:00.000Z'
 
 class RegistryFetcher extends Fetcher {
+  #cacheKey
   constructor (spec, opts) {
     super(spec, opts)
 
@@ -32,8 +33,8 @@ class RegistryFetcher extends Fetcher {
     this.packumentCache = this.opts.packumentCache || null
 
     this.registry = fetch.pickRegistry(spec, opts)
-    this.packumentUrl = removeTrailingSlashes(this.registry) + '/' +
-      this.spec.escapedName
+    this.packumentUrl = `${removeTrailingSlashes(this.registry)}/${this.spec.escapedName}`
+    this.#cacheKey = `${this.fullMetadata ? 'full' : 'corgi'}:${this.packumentUrl}`
 
     const parsed = new URL(this.registry)
     const regKey = `//${parsed.host}${parsed.pathname}`
@@ -78,8 +79,8 @@ class RegistryFetcher extends Fetcher {
     // note this might be either an in-flight promise for a request,
     // or the actual packument, but we never want to make more than
     // one request at a time for the same thing regardless.
-    if (this.packumentCache?.has(this.packumentUrl)) {
-      return this.packumentCache.get(this.packumentUrl)
+    if (this.packumentCache?.has(this.#cacheKey)) {
+      return this.packumentCache.get(this.#cacheKey)
     }
 
     // npm-registry-fetch the packument
@@ -99,10 +100,10 @@ class RegistryFetcher extends Fetcher {
       if (contentLength) {
         packument._contentLength = Number(contentLength)
       }
-      this.packumentCache?.set(this.packumentUrl, packument)
+      this.packumentCache?.set(this.#cacheKey, packument)
       return packument
     } catch (err) {
-      this.packumentCache?.delete(this.packumentUrl)
+      this.packumentCache?.delete(this.#cacheKey)
       if (err.code !== 'E404' || this.fullMetadata) {
         throw err
       }

test/registry.js

@@ -22,57 +22,62 @@ const MockedRegistryFetcher = t.mock('../lib/registry.js', {
 
 const port = 18000 + (+process.env.TAP_CHILD_ID || 0)
 const me = t.testdir()
+const registry = `http://localhost:${port}/`
+const cache = me + '/cache'
 
-t.test('start mock registry', { bail: true }, t => {
-  mr({
-    port,
-    mocks: {
-      get: {
-        '/no-integrity/-/no-integrity-1.2.3.tgz': [
-          200,
-          `${__dirname}/fixtures/abbrev-1.1.1.tgz`,
-        ],
+let mockServer
+t.before(() => {
+  return new Promise((resolve, reject) => {
+    mr({
+      port,
+      mocks: {
+        get: {
+          '/no-integrity/-/no-integrity-1.2.3.tgz': [
+            200,
+            `${__dirname}/fixtures/abbrev-1.1.1.tgz`,
+          ],
+        },
       },
-    },
 
-    plugin (server) {
-      server.get('/thing-is-not-here').many().reply(404, { error: 'not found' })
-      server.get('/no-tarball').many().reply(200, {
-        name: 'no-tarball',
-        'dist-tags': { latest: '1.2.3' },
-        versions: {
-          '1.2.3': {
-            name: 'no-tarball',
-            version: '1.2.3',
+      plugin (server) {
+        server.get('/thing-is-not-here').many().reply(404, { error: 'not found' })
+        server.get('/no-tarball').many().reply(200, {
+          name: 'no-tarball',
+          'dist-tags': { latest: '1.2.3' },
+          versions: {
+            '1.2.3': {
+              name: 'no-tarball',
+              version: '1.2.3',
+            },
           },
-        },
-      })
-      server.get('/no-integrity').many().reply(200, {
-        name: 'no-integrity',
-        'dist-tags': { latest: '1.2.3' },
-        versions: {
-          '1.2.3': {
-            name: 'no-integrity',
-            version: '1.2.3',
-            dist: {
-              tarball: `${registry}no-integrity/-/no-integrity-1.2.3.tgz`,
+        })
+        server.get('/no-integrity').many().reply(200, {
+          name: 'no-integrity',
+          'dist-tags': { latest: '1.2.3' },
+          versions: {
+            '1.2.3': {
+              name: 'no-integrity',
+              version: '1.2.3',
+              dist: {
+                tarball: `${registry}no-integrity/-/no-integrity-1.2.3.tgz`,
+              },
             },
           },
-        },
-      })
-    },
-  }, (er, s) => {
-    if (er) {
-      throw er
-    }
-
-    t.parent.teardown(() => s.close())
-    t.end()
+        })
+      },
+    }, (err, s) => {
+      if (err) {
+        return reject(err)
+      }
+      mockServer = s
+      resolve()
+    })
   })
 })
 
-const registry = `http://localhost:${port}/`
-const cache = me + '/cache'
+t.teardown(() => {
+  mockServer?.close()
+})
 
 t.test('underscore, no tag or version', t => {
   const f = new RegistryFetcher('underscore', { registry, cache, fullReadJson: true })
@@ -1207,11 +1212,34 @@ t.test('packument that has been cached', async t => {
       },
     },
   }
-  const packumentCache = new Map([[packumentUrl, packument]])
+  const packumentCache = new Map([[`corgi:${packumentUrl}`, packument]])
   const f = new RegistryFetcher('asdf@1.2', { registry, cache, packumentCache })
   t.equal(await f.packument(), packument, 'got cached packument')
 })
 
+t.test('corgi packument is not cached as full packument', async t => {
+  const packumentUrl = `${registry}underscore`
+  const packument = {
+    name: 'underscore',
+    versions: {
+      '1.5.1': {
+        cached: true,
+        name: 'underscore',
+        version: '1.5.1',
+      },
+    },
+  }
+  const packumentCache = new Map([[`corgi:${packumentUrl}`, packument]])
+  const f = new RegistryFetcher('underscore', {
+    packumentCache,
+    registry,
+    cache,
+    fullMetadata: true,
+  })
+  t.notEqual(await f.packument(), packument, 'did not get cached packument')
+  t.ok(packumentCache.has(`full:${packumentUrl}`), 'full packument is also now cached')
+})
+
 t.test('packument that falls back to fullMetadata', t => {
   const http = require('http')
   const packument = {