fix(finalize-manifest): use digest to uniquify cached manifests

Fixes: #63

lib/finalize-manifest.js

@@ -18,10 +18,10 @@ const through = require('mississippi').through
 
 module.exports = finalizeManifest
 function finalizeManifest (pkg, spec, opts) {
-  const key = cache.key(`${spec.type}-manifest`, pkg ? pkg._resolved : spec.spec)
+  const key = finalKey(pkg, spec)
   opts = optCheck(opts)
   opts.memoize = true
-  return (opts.cache ? cache.get.info(opts.cache, key, opts).then(res => {
+  return ((opts.cache && key) ? cache.get.info(opts.cache, key, opts).then(res => {
     if (!res) { throw new Error('cache miss') }
     return new Manifest(res.metadata)
   }) : BB.reject()).catch(() => {
@@ -35,7 +35,7 @@ function finalizeManifest (pkg, spec, opts) {
           opts.metadata = result
           cache.put(
             opts.cache,
-            key,
+            finalKey(result, spec),
             '.',
             opts
           ).then(() => cb(null, result), cb)
@@ -205,3 +205,13 @@ function tarballedProps (pkg, spec, opts, cb) {
     })
   }
 }
+
+function finalKey (pkg, spec) {
+  return (
+    pkg && (pkg._sha512sum || pkg._shasum) &&
+    cache.key(
+      `${spec.type}-manifest`,
+      `${pkg._sha512sum || pkg._shasum}-${pkg._resolved}`
+    )
+  )
+}

test/finalize-manifest.js

@@ -2,21 +2,24 @@
 
 const BB = require('bluebird')
 
+const cache = require('../lib/cache')
 const crypto = require('crypto')
 const npmlog = require('npmlog')
 const path = require('path')
 const tar = require('tar-stream')
 const test = require('tap').test
+const testDir = require('./util/test-dir')
 const tnock = require('./util/tnock')
 
-require('./util/test-dir')(__filename)
+const CACHE = testDir(__filename)
 
 const finalizeManifest = require('../lib/finalize-manifest')
 
 npmlog.level = process.env.LOGLEVEL || 'silent'
 const OPTS = {
   registry: 'https://mock.reg/',
   log: npmlog,
+  hashAlgorithm: 'sha1',
   retry: {
     retries: 1,
     factor: 1,
@@ -94,7 +97,6 @@ test('fills in shrinkwrap if missing', t => {
     name: 'testing',
     version: '1.2.3',
     _resolved: OPTS.registry + tarballPath,
-    _shasum: 'deadbeefc0ffeebad1dea',
     _hasShrinkwrap: true
   }
   const sr = {
@@ -151,7 +153,6 @@ test('fills in `bin` if `directories.bin` string', t => {
       bin: 'foo/my/bin'
     },
     _resolved: OPTS.registry + tarballPath,
-    _shasum: 'deadbeefc0ffeebad1dea',
     _hasShrinkwrap: false
   }
   const sr = {
@@ -189,8 +190,8 @@ test('fills in `bin` if original was an array', t => {
     directories: {
       bin: 'foo'
     },
-    _resolved: OPTS.registry + tarballPath,
     _shasum: 'deadbeefc0ffeebad1dea',
+    _resolved: OPTS.registry + tarballPath,
     _hasShrinkwrap: false
   }
   return finalizeManifest(base, {
@@ -244,6 +245,55 @@ test('uses package.json as base if passed null', t => {
     })
   })
 })
+
+test('caches finalized manifests', t => {
+  cache.clearMemoized()
+  const tarballPath = 'testing/tarball-1.2.3.tgz'
+  const base = {
+    name: 'testing',
+    version: '1.2.3',
+    _resolved: OPTS.registry + tarballPath,
+    _hasShrinkwrap: true
+  }
+  const sr = {
+    name: base.name,
+    version: base.version
+  }
+  const opts = Object.create(OPTS)
+  opts.cache = CACHE
+  return makeTarball({
+    'package.json': base,
+    'npm-shrinkwrap.json': sr
+  }).then(tarData => {
+    tnock(t, OPTS.registry).get('/' + tarballPath).reply(200, tarData)
+    return finalizeManifest(base, {
+      name: base.name,
+      type: 'range'
+    }, opts).then(manifest1 => {
+      base._shasum = manifest1._shasum
+      base._sha512sum = manifest1._sha512sum
+      return cache.ls(CACHE, opts).then(entries => {
+        const promises = []
+        Object.keys(entries).forEach(k => {
+          if (!k.match(/^pacote:range-manifest/)) {
+            promises.push(cache.put(CACHE, k, '', opts))
+          } else {
+            t.ok(true, 'manifest entry exists in cache: ' + k)
+          }
+        })
+        return Promise.all(promises)
+      }).then(() => {
+        return finalizeManifest(base, {
+          name: base.name,
+          type: 'range'
+        }, opts)
+      }).then(manifest2 => {
+        t.deepEqual(manifest2, manifest1, 'got cached manifest')
+      })
+    })
+  })
+})
+
 // TODO - this is pending major changes in npm, so not implemented for now.
 test('manifest returned is immutable + inextensible')