From 0dbc0201b2d1f7c909f74816cc50bc68013fc70f Mon Sep 17 00:00:00 2001 From: Maksim Ryzhikov Date: Tue, 3 Jun 2014 14:29:02 +0400 Subject: [PATCH] fix(file_list): Incorrect response after remove and add file Remove and then immediately add same file can lead to incorrect content in response. Because pending timeout after remove may resolve early then file will be processed, and in response get not processed content. --- lib/file_list.js | 20 +++++++++++--------- test/unit/file_list.spec.coffee | 31 ++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/lib/file_list.js b/lib/file_list.js index 64bed149a..9b33ddb56 100644 --- a/lib/file_list.js +++ b/lib/file_list.js @@ -111,9 +111,7 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) { }; var resolveDeferred = function(files) { - if (pendingTimeout) { - clearTimeout(pendingTimeout); - } + clearPendingTimeout(); if (!errors.length) { pendingDeferred.resolve(files || resolveFiles(self.buckets)); @@ -125,9 +123,7 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) { }; var fireEventAndDefer = function() { - if (pendingTimeout) { - clearTimeout(pendingTimeout); - } + clearPendingTimeout(); if (!pendingDeferred) { pendingDeferred = q.defer(); @@ -137,6 +133,12 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) { pendingTimeout = setTimeout(resolveDeferred, batchInterval); }; + var clearPendingTimeout = function() { + if (pendingTimeout) { + clearTimeout(pendingTimeout); + } + }; + // re-glob all the patterns this.refresh = function() { @@ -171,9 +173,7 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) { emitter.emit('file_list_modified', pendingDeferred.promise); } - if (pendingTimeout) { - clearTimeout(pendingTimeout); - } + clearPendingTimeout(); patterns.forEach(function(patternObject, i) { var pattern = patternObject.pattern; @@ -302,6 +302,8 @@ var List = function(patterns, excludes, emitter, preprocess, batchInterval) { var addedFile = new File(path); buckets[i].push(addedFile); + clearPendingTimeout(); + return fs.stat(path, function(err, stat) { // in the case someone refresh() the list before stat callback if (self.buckets === buckets) { diff --git a/test/unit/file_list.spec.coffee b/test/unit/file_list.spec.coffee index f2e228aad..00ee4097c 100644 --- a/test/unit/file_list.spec.coffee +++ b/test/unit/file_list.spec.coffee @@ -589,7 +589,6 @@ describe 'file_list', -> # Batch Interval processing #============================================================================ describe 'batch interval', -> - it 'should batch multiple changes within an interval', (done) -> timeoutSpy = sinon.stub().returns true globals_ = setTimeout: timeoutSpy @@ -631,6 +630,36 @@ describe 'file_list', -> expect(timeoutSpy).to.have.been.called timeoutSpy.lastCall.args[0]() + it 'should wait while file preprocessing, if file was deleted and immediately added', (done) -> + clock = sinon.useFakeTimers() + + m = mocks.loadFile __dirname + '/../../lib/file_list.js', mocks_, global + list = new m.List patterns('/a.*'), [], emitter, preprocessMock, 1000 + + refreshListAndThen (files) -> + preprocessMock.reset() + + emitter.once 'file_list_modified', (promise) -> + expect(promise).to.be.fulfilled.then((files)-> + + # expect file will be preprocessed when file list promise + # resolved, else we will get incorrect(not processed) content in response + expect(preprocessMock.callCount).to.equal 1 + ).should.notify(done) + + # Remove and then immediately add file to the bucket + list.removeFile '/a.txt' + list.addFile '/a.txt' + + # Timed remove pending timeout + # but processing file after add is not completed + clock.tick(1000) + + # Finish processing file + process.nextTick -> process.nextTick -> + clock.tick(1000) + + clock.tick() #============================================================================ # Win Globbing