Fix permanently stale cache by computing signature before compile (#160)

Expose cache.generateSignature to be called by plugin and passed
into cache.set.

Fixes #159

lib/HappyFSCache.js

@@ -21,6 +21,8 @@ module.exports = function HappyFSCache(config) {
   var cache = { context: {}, mtimes: {} };
   var generateSignature = config.generateSignature || getMTime;
 
+  exports.generateSignature = generateSignature;
+
   assert(typeof cachePath === 'string',
     "HappyFSCache requires a @path parameter that points to where it will be stored.");
 
@@ -108,9 +110,9 @@ module.exports = function HappyFSCache(config) {
     delete cache.mtimes[filePath];
   };
 
-  exports.set = function(filePath, error, compiledPath) {
+  exports.set = function(filePath, error, compiledPath, signature) {
     cache.mtimes[filePath] = {
-      mtime: generateSignature(filePath),
+      mtime: signature,
       compiledPath: compiledPath,
       error: error
     };

lib/HappyPlugin.js

@@ -342,6 +342,11 @@ function compileAndUpdateCache(threadPool, loader, loaderContext, done) {
   var cache = this.cache;
   var filePath = loaderContext.resourcePath;
 
+  // Compute the signature /before/ compiling to avoid a race condition where the compiled file's source is
+  // updated in the filesytem (externally, e.g. via git) before computing the signature, which would cause
+  // a stale compiled file to be stuck in the cache.
+  var signature = cache.generateSignature(filePath);
+
   threadPool.compile(loaderContext.remoteLoaderId, loader, {
     loaders: this.state.loaders,
     compiledPath: path.resolve(this.config.tempDir, cache.getCompiledSourceCodePath(filePath) || HappyUtils.generateCompiledPath(filePath)),
@@ -352,11 +357,11 @@ function compileAndUpdateCache(threadPool, loader, loaderContext, done) {
 
     if (!result.success) {
       var error = ErrorSerializer.deserialize(contents);
-      cache.set(filePath, error, null);
+      cache.set(filePath, error, null, signature);
       done(error);
     }
     else {
-      cache.set(filePath, null, result.compiledPath);
+      cache.set(filePath, null, result.compiledPath, signature);
 
       compiledMap = SourceMapSerializer.deserialize(
         fs.readFileSync(cache.getCompiledSourceMapPath(filePath), 'utf-8')

lib/__tests__/HappyPlugin.test.js

@@ -1,3 +1,5 @@
+var sinon = require('sinon');
+
 var Subject = require('../HappyPlugin');
 var HappyTestUtils = require('../HappyTestUtils');
 var assert = HappyTestUtils.assert;
@@ -46,4 +48,47 @@ describe('HappyPlugin', function() {
       Subject({ loaders: [ { loader: 'babel' } ] })
     });
   })
-});
+
+  describe('#compile', function() {
+    it('avoids preserving stale cache on simultaneous file edit', function() {
+      // Create a file with a corresponding mtime.
+      var sourceFile = testSuite.createFile('src-file.coffee', 'abc = "123"');
+      var mtime = 0;
+      var signatureGenerator = function(filePath) {
+        return (filePath === sourceFile.path) ? mtime : 0;
+      }
+
+      // Use a fake compiler just so we can instantiate a plugin with a cache and threadPool.
+      var compiler = {plugin: sinon.spy(), options: {}};
+      var loader = testSuite.createLoader(function(s) { return s; });
+      var plugin = Subject({
+        loaders: [loader],
+        cacheSignatureGenerator: signatureGenerator,
+      });
+      plugin.apply(compiler);
+
+      // We don't care about testing the actual compile, so we'll stub it out.
+      var sandbox = testSuite.getSinonSandbox();
+      var threadCompileStub = sandbox.stub(plugin.threadPool, 'compile');
+
+      plugin.compile(loader, {resourcePath: sourceFile.path}, function() {});
+      assert(threadCompileStub.calledOnce);
+
+      // SIMULATE that the file has changed after reading the source but before writing to the cache.
+      // If the mtime is read for the first time too late, then the cache will think that the source file
+      // is up-to-date even though is it now out-of-date.
+      mtime++;
+
+      // "Compile" the file and call the callback to trigger the cache behavior.
+      var compiledFile = testSuite.createFile('build/src-file.js', 'var abc = "123";');
+      testSuite.createFile('build/src-file.js.map', 'abc source map');
+      threadCompileStub.getCall(0).args[3]({
+        success: true,
+        compiledPath: compiledFile.path,
+      });
+
+      // If we used the mtime of 0 when cacheing, then the file will correctly be identified as stale:
+      assert(plugin.cache.hasChanged(sourceFile.path));
+    });
+  });
+});