break inflight management out into a module

5 files changed, 56 insertions, 79 deletions

diff --git a/lib/cache.js b/lib/cache.js
index 2b1dc217d..c269ffe86 100644
--- a/lib/cache.js
+++ b/lib/cache.js
@@ -9,15 +9,14 @@
 //
 
 /*
-fetching a url:
-1. Check for url in inFlightUrls.  If present, add cb, and return.
-2. create inFlightURL list
-3. Acquire lock at {cache}/{sha(url)}.lock
+fetching a URL:
+1. Check for URL in inflight URLs.  If present, add cb, and return.
+2. Acquire lock at {cache}/{sha(url)}.lock
    retries = {cache-lock-retries, def=3}
    stale = {cache-lock-stale, def=30000}
    wait = {cache-lock-wait, def=100}
-4. if lock can't be acquired, then fail
-5. fetch url, clear lock, call cbs
+3. if lock can't be acquired, then fail
+4. fetch url, clear lock, call cbs
 
 cache folders:
 1. urls: http!/server.com/path/to/thing
@@ -115,10 +114,6 @@ function cache (args, cb) {
   }
 }
 
-// Only have a single download action at once for a given url
-// additional calls stack the callbacks.
-var inFlightURLs = {}
-
 // if the pkg and ver are in the cache, then
 // just do a readJson and return.
 // if they're not, then fetch them from the registry.
@@ -132,14 +127,14 @@ function read (name, ver, forceBypass, cb) {
 
   if (forceBypass && npm.config.get("force")) {
     log.verbose("using force", "skipping cache")
-    return addNamed(name, ver, null, inFlightURLs, c)
+    return addNamed(name, ver, null, c)
   }
 
   readJson(jsonFile, function (er, data) {
     er = needName(er, data)
     er = needVersion(er, data)
     if (er && er.code !== "ENOENT" && er.code !== "ENOTDIR") return cb(er)
-    if (er) return addNamed(name, ver, null, inFlightURLs, c)
+    if (er) return addNamed(name, ver, null, c)
     deprCheck(data)
     c(er, data)
   })
@@ -280,13 +275,13 @@ function maybeFile (spec, p, cb) {
   fs.stat(spec, function (er) {
     if (!er) {
       // definitely a local thing
-      addLocal(spec, "", inFlightURLs, cb)
+      addLocal(spec, "", cb)
     } else if (er && spec.indexOf("@") !== -1) {
       // bar@baz/loofa
       maybeAt(spec, cb)
     } else {
       // Already know it's not a url, so must be local
-      addLocal(spec, "", inFlightURLs, cb)
+      addLocal(spec, "", cb)
     }
   })
 }
@@ -305,18 +300,18 @@ function add_ (name, spec, p, cb) {
   switch (p.protocol) {
     case "http:":
     case "https:":
-      return addRemoteTarball(spec, null, name, "", inFlightURLs, cb)
+      return addRemoteTarball(spec, null, name, "", cb)
 
     default:
       if (isGitUrl(p))
-        return addRemoteGit(spec, p, false, inFlightURLs, cb)
+        return addRemoteGit(spec, p, false, cb)
 
       // if we have a name and a spec, then try name@spec
       // if not, then try just spec (which may try name@"" if not found)
       if (name) {
-        addNamed(name, spec, null, inFlightURLs, cb)
+        addNamed(name, spec, null, cb)
       } else {
-        addLocal(spec, "", inFlightURLs, cb)
+        addLocal(spec, "", cb)
       }
   }
 }
diff --git a/lib/cache/add-local.js b/lib/cache/add-local.js
index 3d48bfb69..b6f9c75f3 100644
--- a/lib/cache/add-local.js
+++ b/lib/cache/add-local.js
@@ -21,9 +21,8 @@ var fs = require("graceful-fs")
 
 module.exports = addLocal
 
-function addLocal (p, name, inFlightURLs, cb_) {
+function addLocal (p, name, cb_) {
   assert(typeof p === "string", "must have path")
-  assert(typeof inFlightURLs === "object", "must have inflight URL cache")
   assert(typeof cb === "function", "must have callback")
 
   if (name === undefined || name === null) name = ""
@@ -35,7 +34,7 @@ function addLocal (p, name, inFlightURLs, cb_) {
         // remote thing.
         if (p.indexOf("/") === -1 && p.charAt(0) !== "."
            && (process.platform !== "win32" || p.indexOf("\\") === -1)) {
-          return addNamed(p, "", null, inFlightURLs, cb_)
+          return addNamed(p, "", null, cb_)
         }
         log.error("addLocal", "Could not install %s", p)
         return cb_(er)
diff --git a/lib/cache/add-named.js b/lib/cache/add-named.js
index 8c2db1626..1c68dc29b 100644
--- a/lib/cache/add-named.js
+++ b/lib/cache/add-named.js
@@ -11,6 +11,7 @@ var path = require("path")
   , npm = require("../npm.js")
   , registry = npm.registry
   , deprCheck = require("../utils/depr-check.js")
+  , inflight = require("inflight")
   , locker = require("../utils/locker.js")
   , lock = locker.lock
   , unlock = locker.unlock
@@ -20,44 +21,36 @@ var path = require("path")
 
 module.exports = addNamed
 
-// only have one request in flight for a given
-// name@blah thing.
-var inFlightNames = {}
-function addNamed (name, version, data, inFlightURLs, cb_) {
+var NAME_PREFIX = "addName:"
+function addNamed (name, version, data, cb_) {
   assert(typeof name === "string", "must have module name")
-  assert(typeof inFlightURLs === "object", "must have inflight URL cache")
   assert(typeof cb_ === "function", "must have callback")
 
   log.verbose("addNamed", [name, version])
 
-  var k = name + "@" + version
-  if (!inFlightNames[k]) inFlightNames[k] = []
-  var iF = inFlightNames[k]
-  iF.push(cb_)
-  if (iF.length > 1) return
-
+  var key = name + "@" + version
   function cb (er, data) {
-    if (data && !data._fromGithub) data._from = k
-    unlock(k, function () {
-      var c
-      while (c = iF.shift()) c(er, data)
-      delete inFlightNames[k]
-    })
+    if (data && !data._fromGithub) data._from = key
+    unlock(key, function () { cb_(er, data) })
   }
 
+  cb_ = inflight(NAME_PREFIX + key, cb_)
+
+  if (!cb_) return
+
   log.verbose("addNamed", [semver.valid(version), semver.validRange(version)])
-  lock(k, function (er) {
+  lock(key, function (er) {
     if (er) return cb(er)
 
     var fn = ( semver.valid(version, true) ? addNameVersion
              : semver.validRange(version, true) ? addNameRange
              : addNameTag
              )
-    fn(name, version, data, inFlightURLs, cb)
+    fn(name, version, data, cb)
   })
 }
 
-function addNameTag (name, tag, data, inFlightURLs, cb_) {
+function addNameTag (name, tag, data, cb_) {
   if (typeof cb_ !== "function") cb_ = data, data = null
   log.info("addNameTag", [name, tag])
   var explicit = true
@@ -84,10 +77,10 @@ function addNameTag (name, tag, data, inFlightURLs, cb_) {
     if (data["dist-tags"] && data["dist-tags"][tag]
         && data.versions[data["dist-tags"][tag]]) {
       var ver = data["dist-tags"][tag]
-      return addNamed(name, ver, data.versions[ver], inFlightURLs, cb)
+      return addNamed(name, ver, data.versions[ver], cb)
     }
     if (!explicit && Object.keys(data.versions).length) {
-      return addNamed(name, "*", data, inFlightURLs, cb)
+      return addNamed(name, "*", data, cb)
     }
 
     er = installTargetsError(tag, data)
@@ -113,7 +106,7 @@ function engineFilter (data) {
   })
 }
 
-function addNameVersion (name, v, data, inFlightURLs, cb) {
+function addNameVersion (name, v, data, cb) {
   if (typeof cb !== "function") cb = data, data = null
 
   var ver = semver.valid(v, true)
@@ -198,13 +191,12 @@ function addNameVersion (name, v, data, inFlightURLs, cb) {
                              , dist.shasum
                              , name
                              , ver
-                             , inFlightURLs
                              , cb )
     }
   }
 }
 
-function addNameRange (name, range, data, inFlightURLs, cb) {
+function addNameRange (name, range, data, cb) {
   if (typeof cb !== "function") cb = data, data = null
 
   range = semver.validRange(range, true)
@@ -236,7 +228,7 @@ function addNameRange (name, range, data, inFlightURLs, cb) {
     if (tagged
         && data.versions[tagged]
         && semver.satisfies(tagged, range, true)) {
-      return addNamed(name, tagged, data.versions[tagged], inFlightURLs, cb)
+      return addNamed(name, tagged, data.versions[tagged], cb)
     }
 
     // find the max satisfying version.
@@ -248,7 +240,7 @@ function addNameRange (name, range, data, inFlightURLs, cb) {
 
     // if we don't have a registry connection, try to see if
     // there's a cached copy that will be ok.
-    addNamed(name, ms, data.versions[ms], inFlightURLs, cb)
+    addNamed(name, ms, data.versions[ms], cb)
   }
 }
 
diff --git a/lib/cache/add-remote-git.js b/lib/cache/add-remote-git.js
index 5b9114c88..8ef268bab 100644
--- a/lib/cache/add-remote-git.js
+++ b/lib/cache/add-remote-git.js
@@ -16,6 +16,7 @@ var mkdir = require("mkdirp")
   , chmodr = require("chmodr")
   , npm = require("../npm.js")
   , rm = require("../utils/gently-rm.js")
+  , inflight = require("inflight")
   , locker = require("../utils/locker.js")
   , lock = locker.lock
   , unlock = locker.unlock
@@ -30,16 +31,18 @@ var mkdir = require("mkdirp")
 // 5. git archive /tmp/random.tgz
 // 6. addLocalTarball(/tmp/random.tgz) <gitref> --format=tar --prefix=package/
 // silent flag is used if this should error quietly
-module.exports = function addRemoteGit (u, parsed, silent, inFlightURLs, cb_) {
+module.exports = function addRemoteGit (u, parsed, silent, cb_) {
   assert(typeof u === "string", "must have git URL")
   assert(typeof parsed === "object", "must have parsed query")
-  assert(typeof inFlightURLs === "object", "must have inflight URL cache")
   assert(typeof cb_ === "function", "must have callback")
 
-  if (!inFlightURLs[u]) inFlightURLs[u] = []
-  var iF = inFlightURLs[u]
-  iF.push(cb_)
-  if (iF.length > 1) return
+  function cb (er, data) {
+    unlock(u, function () { cb_(er, data) })
+  }
+
+  cb_ = inflight(u, cb_)
+
+  if (!cb_) return
 
   // git is so tricky!
   // if the path is like ssh://foo:22/some/path then it works, but
@@ -55,14 +58,6 @@ module.exports = function addRemoteGit (u, parsed, silent, inFlightURLs, cb_) {
     u = u.replace(/^ssh:\/\//, "")
   }
 
-  function cb (er, data) {
-    unlock(u, function () {
-      var c
-      while (c = iF.shift()) c(er, data)
-      delete inFlightURLs[origUrl]
-    })
-  }
-
   lock(u, function (er) {
     if (er) return cb(er)
 
diff --git a/lib/cache/add-remote-tarball.js b/lib/cache/add-remote-tarball.js
index a8ce3f48b..e5c344ef4 100644
--- a/lib/cache/add-remote-tarball.js
+++ b/lib/cache/add-remote-tarball.js
@@ -8,50 +8,46 @@ var mkdir = require("mkdirp")
   , retry = require("retry")
   , npm = require("../npm.js")
   , fetch = require("../utils/fetch.js")
+  , inflight = require("inflight")
   , locker = require("../utils/locker.js")
   , lock = locker.lock
   , unlock = locker.unlock
   , addLocalTarball = require("./add-local-tarball.js")
 
-module.exports = function addRemoteTarball (u, shasum, name, version, inFlightURLs, cb_) {
+module.exports = function addRemoteTarball (u, shasum, name, version, cb_) {
   assert(typeof u === "string", "must have module URL")
-  assert(typeof inFlightURLs === "object", "must have inflight URL cache")
   assert(typeof cb_ === "function", "must have callback")
 
-  if (!inFlightURLs[u]) inFlightURLs[u] = []
-  var iF = inFlightURLs[u]
-  iF.push(cb_)
-  if (iF.length > 1) return
-
   function cb (er, data) {
     if (data) {
       data._from = u
       data._shasum = data._shasum || shasum
       data._resolved = u
     }
-    unlock(u, function () {
-      var c
-      while (c = iF.shift()) c(er, data)
-      delete inFlightURLs[u]
+    unlock(u, function () { cb_(er, data)
     })
   }
 
+  cb_ = inflight(u, cb_)
+
+  if (!cb_) return
+
   var tmp = path.join(npm.tmp, Date.now()+"-"+Math.random(), "tmp.tgz")
 
+  function next (er, resp, shasum) {
+    if (er) return cb(er)
+    addLocalTarball(tmp, name, version, shasum, cb)
+  }
+
   lock(u, function (er) {
     if (er) return cb(er)
 
     log.verbose("addRemoteTarball", [u, shasum])
     mkdir(path.dirname(tmp), function (er) {
       if (er) return cb(er)
-      addRemoteTarball_(u, tmp, shasum, done)
+      addRemoteTarball_(u, tmp, shasum, next)
     })
   })
-
-  function done (er, resp, shasum) {
-    if (er) return cb(er)
-    addLocalTarball(tmp, name, version, shasum, cb)
-  }
 }
 
 function addRemoteTarball_(u, tmp, shasum, cb) {