fs-write-stream-atomic@1.0.8

Rewrite to use modern streams even on 0.8 plus a bunch of tests

Credit: @iarna

7 files changed, 244 insertions, 98 deletions

diff --git a/node_modules/fs-write-stream-atomic/index.js b/node_modules/fs-write-stream-atomic/index.js
index 949250531..59b50db6d 100644
--- a/node_modules/fs-write-stream-atomic/index.js
+++ b/node_modules/fs-write-stream-atomic/index.js
@@ -1,6 +1,8 @@
 var fs = require('graceful-fs')
+var Writable = require('readable-stream').Writable
 var util = require('util')
 var MurmurHash3 = require('imurmurhash')
+var iferr = require('iferr')
 
 function murmurhex () {
   var hash = MurmurHash3('')
@@ -15,82 +17,108 @@ function getTmpname (filename) {
   return filename + '.' + murmurhex(__filename, process.pid, ++invocations)
 }
 
-module.exports = WriteStream
+var setImmediate = global.setImmediate || setTimeout
 
-util.inherits(WriteStream, fs.WriteStream)
-function WriteStream (path, options) {
-  if (!options) options = {}
+module.exports = WriteStreamAtomic
 
-  if (!(this instanceof WriteStream)) {
-    return new WriteStream(path, options)
+// Requirements:
+//   1. Write everything written to the stream to a temp file.
+//   2. If there are no errors:
+//      a. moves the temp file into its final destination
+//      b. emits `finish` & `closed` ONLY after the file is
+//         fully flushed and renamed.
+//   3. If there's an error, removes the temp file.
+
+util.inherits(WriteStreamAtomic, Writable)
+function WriteStreamAtomic (path, options) {
+  if (!(this instanceof WriteStreamAtomic)) {
+    return new WriteStreamAtomic(path, options)
   }
+  Writable.call(this, options)
 
   this.__atomicTarget = path
-  this.__atomicChown = options.chown
-  this.__atomicDidStuff = false
   this.__atomicTmp = getTmpname(path)
 
-  fs.WriteStream.call(this, this.__atomicTmp, options)
-}
+  this.__atomicChown = options && options.chown
+
+  this.__atomicClosed = false
+
+  this.__atomicStream = fs.WriteStream(this.__atomicTmp, options)
 
-function cleanup (er) {
-  fs.unlink(this.__atomicTmp, function () {
-    fs.WriteStream.prototype.emit.call(this, 'error', er)
-  }.bind(this))
+  this.__atomicStream.once('open', handleOpen(this))
+  this.__atomicStream.once('close', handleClose(this))
+  this.__atomicStream.once('error', handleError(this))
 }
 
-function cleanupSync () {
-  try {
-    fs.unlinkSync(this.__atomicTmp)
-  } finally {
-    return
-  }
+// We have to suppress default finish emitting, because ordinarily it
+// would happen as soon as `end` is called on us and all of the
+// data has been written to our target stream. So we suppress
+// finish from being emitted here, and only emit it after our
+// target stream is closed and we've moved everything around.
+WriteStreamAtomic.prototype.emit = function (event) {
+  if (event === 'finish') return this.__atomicStream.end()
+  return Writable.prototype.emit.apply(this, arguments)
 }
 
-// When we *would* emit 'close' or 'finish', instead do our stuff
-WriteStream.prototype.emit = function (ev) {
-  if (ev === 'error') cleanupSync.call(this)
+WriteStreamAtomic.prototype._write = function (buffer, encoding, cb) {
+  var flushed = this.__atomicStream.write(buffer, encoding)
+  if (flushed) return cb()
+  this.__atomicStream.once('drain', cb)
+}
 
-  if (ev !== 'close' && ev !== 'finish') {
-    return fs.WriteStream.prototype.emit.apply(this, arguments)
+function handleOpen (writeStream) {
+  return function (fd) {
+    writeStream.emit('open', fd)
   }
+}
 
-  // We handle emitting finish and close after the rename.
-  if (ev === 'close' || ev === 'finish') {
-    if (!this.__atomicDidStuff) {
-      atomicDoStuff.call(this, function (er) {
-        if (er) cleanup.call(this, er)
-      }.bind(this))
+function handleClose (writeStream) {
+  return function () {
+    if (writeStream.__atomicClosed) return
+    writeStream.__atomicClosed = true
+    if (writeStream.__atomicChown) {
+      var uid = writeStream.__atomicChown.uid
+      var gid = writeStream.__atomicChown.gid
+      return fs.chown(writeStream.__atomicTmp, uid, gid, iferr(cleanup, moveIntoPlace))
+    } else {
+      moveIntoPlace()
     }
   }
-}
-
-function atomicDoStuff (cb) {
-  if (this.__atomicDidStuff) {
-    throw new Error('Already did atomic move-into-place')
+  function cleanup (err) {
+    fs.unlink(writeStream.__atomicTmp, function () {
+      writeStream.emit('error', err)
+      writeStream.emit('close')
+    })
+  }
+  function moveIntoPlace () {
+    fs.rename(writeStream.__atomicTmp, writeStream.__atomicTarget, iferr(cleanup, end))
   }
+  function end () {
+    // We have to use our parent class directly because we suppress `finish`
+    // events fired via our own emit method.
+    Writable.prototype.emit.call(writeStream, 'finish')
 
-  this.__atomicDidStuff = true
-  if (this.__atomicChown) {
-    var uid = this.__atomicChown.uid
-    var gid = this.__atomicChown.gid
-    return fs.chown(this.__atomicTmp, uid, gid, function (er) {
-      if (er) return cb(er)
-      moveIntoPlace.call(this, cb)
-    }.bind(this))
-  } else {
-    moveIntoPlace.call(this, cb)
+    // Delay the close to provide the same temporal separation a physical
+    // file operation would have– that is, the close event is emitted only
+    // after the async close operation completes.
+    setImmediate(function () {
+      writeStream.emit('close')
+    })
   }
 }
 
-function moveIntoPlace (cb) {
-  fs.rename(this.__atomicTmp, this.__atomicTarget, function (er) {
-    cb(er)
-    // emit finish, and then close on the next tick
-    // This makes finish/close consistent across Node versions also.
-    fs.WriteStream.prototype.emit.call(this, 'finish')
-    process.nextTick(function () {
-      fs.WriteStream.prototype.emit.call(this, 'close')
-    }.bind(this))
-  }.bind(this))
+function handleError (writeStream) {
+  return function (er) {
+    cleanupSync()
+    writeStream.emit('error', er)
+    writeStream.__atomicClosed = true
+    writeStream.emit('close')
+  }
+  function cleanupSync () {
+    try {
+      fs.unlinkSync(writeStream.__atomicTmp)
+    } finally {
+      return
+    }
+  }
 }
diff --git a/node_modules/fs-write-stream-atomic/package.json b/node_modules/fs-write-stream-atomic/package.json
index e61078ede..e65fa32c0 100644
--- a/node_modules/fs-write-stream-atomic/package.json
+++ b/node_modules/fs-write-stream-atomic/package.json
@@ -1,37 +1,37 @@
 {
   "_args": [
     [
-      "fs-write-stream-atomic@^1.0.5",
-      "/Users/ogd/Documents/projects/npm/npm"
+      "fs-write-stream-atomic@1.0.8",
+      "/Users/rebecca/code/npm"
     ]
   ],
-  "_from": "fs-write-stream-atomic@>=1.0.5 <2.0.0",
-  "_id": "fs-write-stream-atomic@1.0.5",
+  "_from": "fs-write-stream-atomic@1.0.8",
+  "_id": "fs-write-stream-atomic@1.0.8",
   "_inCache": true,
   "_installable": true,
   "_location": "/fs-write-stream-atomic",
-  "_nodeVersion": "5.1.0",
+  "_nodeVersion": "4.2.2",
   "_npmUser": {
-    "email": "ogd@aoaioxxysz.net",
-    "name": "othiym23"
+    "email": "me@re-becca.org",
+    "name": "iarna"
   },
-  "_npmVersion": "3.5.1",
+  "_npmVersion": "3.5.2",
   "_phantomChildren": {},
   "_requested": {
     "name": "fs-write-stream-atomic",
-    "raw": "fs-write-stream-atomic@^1.0.5",
-    "rawSpec": "^1.0.5",
+    "raw": "fs-write-stream-atomic@1.0.8",
+    "rawSpec": "1.0.8",
     "scope": null,
-    "spec": ">=1.0.5 <2.0.0",
-    "type": "range"
+    "spec": "1.0.8",
+    "type": "version"
   },
   "_requiredBy": [
     "/"
   ],
-  "_shasum": "862a4dabdffcafabfc16499458e37310c39925f6",
+  "_shasum": "e49aaddf288f87d46ff9e882f216a13abc40778b",
   "_shrinkwrap": null,
-  "_spec": "fs-write-stream-atomic@^1.0.5",
-  "_where": "/Users/ogd/Documents/projects/npm/npm",
+  "_spec": "fs-write-stream-atomic@1.0.8",
+  "_where": "/Users/rebecca/code/npm",
   "author": {
     "email": "i@izs.me",
     "name": "Isaac Z. Schlueter",
@@ -42,10 +42,13 @@
   },
   "dependencies": {
     "graceful-fs": "^4.1.2",
-    "imurmurhash": "^0.1.4"
+    "iferr": "^0.1.5",
+    "imurmurhash": "^0.1.4",
+    "readable-stream": "1 || 2"
   },
   "description": "Like `fs.createWriteStream(...)`, but atomic.",
   "devDependencies": {
+    "rimraf": "^2.4.4",
     "standard": "^5.4.1",
     "tap": "^2.3.1"
   },
@@ -53,10 +56,10 @@
     "test": "test"
   },
   "dist": {
-    "shasum": "862a4dabdffcafabfc16499458e37310c39925f6",
-    "tarball": "http://registry.npmjs.org/fs-write-stream-atomic/-/fs-write-stream-atomic-1.0.5.tgz"
+    "shasum": "e49aaddf288f87d46ff9e882f216a13abc40778b",
+    "tarball": "http://registry.npmjs.org/fs-write-stream-atomic/-/fs-write-stream-atomic-1.0.8.tgz"
   },
-  "gitHead": "1bc752bf0e0d5b7aaaad7be696dbc0e4ea64258c",
+  "gitHead": "b55824ee4de7f1ca23784929d68b1b8f5edbf4a4",
   "homepage": "https://github.com/npm/fs-write-stream-atomic",
   "license": "ISC",
   "main": "index.js",
@@ -88,5 +91,5 @@
   "scripts": {
     "test": "standard && tap --coverage test/*.js"
   },
-  "version": "1.0.5"
+  "version": "1.0.8"
 }
diff --git a/node_modules/fs-write-stream-atomic/test/basic.js b/node_modules/fs-write-stream-atomic/test/basic.js
index 2dae137a6..d0205e15f 100644
--- a/node_modules/fs-write-stream-atomic/test/basic.js
+++ b/node_modules/fs-write-stream-atomic/test/basic.js
@@ -1,7 +1,14 @@
+var fs = require('graceful-fs')
 var test = require('tap').test
-var writeStream = require('../index.js')
-var fs = require('fs')
 var path = require('path')
+var writeStream = require('../index.js')
+
+var rename = fs.rename
+fs.rename = function (from, to, cb) {
+  setTimeout(function () {
+    rename(from, to, cb)
+  }, 100)
+}
 
 test('basic', function (t) {
   // open 10 write streams to the same file.
@@ -10,40 +17,39 @@ test('basic', function (t) {
   var target = path.resolve(__dirname, 'test.txt')
   var n = 10
 
+  // We run all of our assertions twice:
+  //   once for finish, once for close
+  // There are 6 assertions, two fixed, plus 4 lines in the file.
+  t.plan(n * 2 * 6)
+
   var streams = []
   for (var i = 0; i < n; i++) {
     var s = writeStream(target)
-    s.on('finish', verifier('finish'))
-    s.on('close', verifier('close'))
+    s.on('finish', verifier('finish', i))
+    s.on('close', verifier('close', i))
     streams.push(s)
   }
 
-  var verifierCalled = 0
-  function verifier (ev) {
+  function verifier (ev, num) {
     return function () {
       if (ev === 'close') {
-        t.equal(this.__emittedFinish, true)
+        t.equal(this.__emittedFinish, true, num + '. closed only after finish')
       } else {
         this.__emittedFinish = true
-        t.equal(ev, 'finish')
+        t.equal(ev, 'finish', num + '. finished')
       }
 
       // make sure that one of the atomic streams won.
       var res = fs.readFileSync(target, 'utf8')
       var lines = res.trim().split(/\n/)
-      lines.forEach(function (line) {
+      lines.forEach(function (line, lineno) {
         var first = lines[0].match(/\d+$/)[0]
         var cur = line.match(/\d+$/)[0]
-        t.equal(cur, first)
+        t.equal(cur, first, num + '. line ' + lineno + ' matches')
       })
 
       var resExpr = /^first write \d+\nsecond write \d+\nthird write \d+\nfinal write \d+\n$/
-      t.similar(res, resExpr)
-
-      // should be called once for each close, and each finish
-      if (++verifierCalled === n * 2) {
-        t.end()
-      }
+      t.similar(res, resExpr, num + '. content matches')
     }
   }
 
diff --git a/node_modules/fs-write-stream-atomic/test/chown.js b/node_modules/fs-write-stream-atomic/test/chown.js
new file mode 100644
index 000000000..1733cf27e
--- /dev/null
+++ b/node_modules/fs-write-stream-atomic/test/chown.js
@@ -0,0 +1,44 @@
+'use strict'
+var fs = require('graceful-fs')
+var path = require('path')
+var test = require('tap').test
+var rimraf = require('rimraf')
+var writeStream = require('../index.js')
+
+var target = path.resolve(__dirname, 'test-chown')
+
+test('chown works', function (t) {
+  t.plan(1)
+  var stream = writeStream(target, {chown: {uid: process.getuid(), gid: process.getgid()}})
+  var hadError = false
+  stream.on('error', function (er) {
+    hadError = true
+    console.log('#', er)
+  })
+  stream.on('close', function () {
+    t.is(hadError, false, 'no errors before close')
+  })
+  stream.end()
+})
+
+test('chown fails', function (t) {
+  t.plan(1)
+  fs.chown = function (file, uid, gid, cb) {
+    cb(new Error('TEST BREAK'))
+  }
+  var stream = writeStream(target, {chown: {uid: process.getuid(), gid: process.getgid()}})
+  var hadError = false
+  stream.on('error', function (er) {
+    hadError = true
+    console.log('#', er)
+  })
+  stream.on('close', function () {
+    t.is(hadError, true, 'error before close')
+  })
+  stream.end()
+})
+
+test('cleanup', function (t) {
+  rimraf.sync(target)
+  t.end()
+})
diff --git a/node_modules/fs-write-stream-atomic/test/rename-fail.js b/node_modules/fs-write-stream-atomic/test/rename-fail.js
new file mode 100644
index 000000000..7e27f0bfb
--- /dev/null
+++ b/node_modules/fs-write-stream-atomic/test/rename-fail.js
@@ -0,0 +1,30 @@
+'use strict'
+var fs = require('graceful-fs')
+var path = require('path')
+var test = require('tap').test
+var rimraf = require('rimraf')
+var writeStream = require('../index.js')
+
+var target = path.resolve(__dirname, 'test-rename')
+
+test('rename fails', function (t) {
+  t.plan(1)
+  fs.rename = function (src, dest, cb) {
+    cb(new Error('TEST BREAK'))
+  }
+  var stream = writeStream(target)
+  var hadError = false
+  stream.on('error', function (er) {
+    hadError = true
+    console.log('#', er)
+  })
+  stream.on('close', function () {
+    t.is(hadError, true, 'error before close')
+  })
+  stream.end()
+})
+
+test('cleanup', function (t) {
+  rimraf.sync(target)
+  t.end()
+})
diff --git a/node_modules/fs-write-stream-atomic/test/slow-close.js b/node_modules/fs-write-stream-atomic/test/slow-close.js
new file mode 100644
index 000000000..9840a6ef0
--- /dev/null
+++ b/node_modules/fs-write-stream-atomic/test/slow-close.js
@@ -0,0 +1,40 @@
+'use strict'
+var fs = require('graceful-fs')
+var path = require('path')
+var test = require('tap').test
+var rimraf = require('rimraf')
+var writeStream = require('../index.js')
+
+var target = path.resolve(__dirname, 'test-chown')
+
+test('slow close', function (t) {
+  t.plan(2)
+  // The goal here is to simulate the "file close" step happening so slowly
+  // that the whole close/rename process could finish before the file is
+  // actually closed (and thus buffers truely flushed to the OS). In
+  // previous versions of this module, this would result in the module
+  // emitting finish & close before the file was fully written and in
+  // turn, could break other layers that tried to read the new file.
+  var realEmit = fs.WriteStream.prototype.emit
+  var reallyClosed = false
+  fs.WriteStream.prototype.emit = function (event) {
+    if (event !== 'close') return realEmit.apply(this, arguments)
+    setTimeout(function () {
+      reallyClosed = true
+      realEmit.call(this, 'close')
+    }.bind(this), 200)
+  }
+  var stream = writeStream(target)
+  stream.on('finish', function () {
+    t.is(reallyClosed, true, "didn't finish before target was closed")
+  })
+  stream.on('close', function () {
+    t.is(reallyClosed, true, "didn't close before target was closed")
+  })
+  stream.end()
+})
+
+test('cleanup', function (t) {
+  rimraf.sync(target)
+  t.end()
+})
diff --git a/node_modules/fs-write-stream-atomic/test/toolong.js b/node_modules/fs-write-stream-atomic/test/toolong.js
index be77f99cf..f146cc55b 100644
--- a/node_modules/fs-write-stream-atomic/test/toolong.js
+++ b/node_modules/fs-write-stream-atomic/test/toolong.js
@@ -13,12 +13,7 @@ function repeat (times, string) {
 var target = path.resolve(__dirname, repeat(1000, 'test'))
 
 test('name too long', function (t) {
-  // 0.8 streams smh
-  if (process.version.indexOf('v0.8') !== -1) {
-    t.plan(1)
-  } else {
-    t.plan(2)
-  }
+  t.plan(2)
   var stream = writeStream(target)
   var hadError = false
   stream.on('error', function (er) {