From 799a7ae4b1cb26b9c02cf0006de601cbf34eb1d9 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Thu, 6 Mar 2014 19:56:49 -0500 Subject: [PATCH 01/10] Add intercom.js functionality for inter-window event broadcast --- lib/eventemitter.js | 66 ++++++++++ lib/intercom.js | 314 ++++++++++++++++++++++++++++++++++++++++++++ src/fs.js | 18 ++- 3 files changed, 395 insertions(+), 3 deletions(-) create mode 100644 lib/eventemitter.js create mode 100644 lib/intercom.js diff --git a/lib/eventemitter.js b/lib/eventemitter.js new file mode 100644 index 0000000..4bd9dc9 --- /dev/null +++ b/lib/eventemitter.js @@ -0,0 +1,66 @@ +define(function(require) { + + // Based on https://github.com/diy/intercom.js/blob/master/lib/events.js + // Copyright 2012 DIY Co Apache License, Version 2.0 + // http://www.apache.org/licenses/LICENSE-2.0 + + function removeItem(item, array) { + for (var i = array.length - 1; i >= 0; i--) { + if (array[i] === item) { + array.splice(i, 1); + } + } + return array; + } + + function EventEmitter() {} + + EventEmitter.createInterface = function(space) { + var methods = {}; + + methods.on = function(name, fn) { + if (typeof this[space] === 'undefined') { + this[space] = {}; + } + if (!this[space].hasOwnProperty(name)) { + this[space][name] = []; + } + this[space][name].push(fn); + }; + + methods.off = function(name, fn) { + if (typeof this[space] === 'undefined') return; + if (this[space].hasOwnProperty(name)) { + removeItem(fn, this[space][name]); + } + }; + + methods.trigger = function(name) { + if (typeof this[space] !== 'undefined' && this[space].hasOwnProperty(name)) { + var args = Array.prototype.slice.call(arguments, 1); + for (var i = 0; i < this[space][name].length; i++) { + this[space][name][i].apply(this[space][name][i], args); + } + } + }; + + return methods; + }; + + var pvt = EventEmitter.createInterface('_handlers'); + EventEmitter.prototype._on = pvt.on; + EventEmitter.prototype._off = pvt.off; + EventEmitter.prototype._trigger = pvt.trigger; + + var pub = EventEmitter.createInterface('handlers'); + EventEmitter.prototype.on = function() { + pub.on.apply(this, arguments); + Array.prototype.unshift.call(arguments, 'on'); + this._trigger.apply(this, arguments); + }; + EventEmitter.prototype.off = pub.off; + EventEmitter.prototype.trigger = pub.trigger; + + return EventEmitter; + +}); diff --git a/lib/intercom.js b/lib/intercom.js new file mode 100644 index 0000000..6c3bae8 --- /dev/null +++ b/lib/intercom.js @@ -0,0 +1,314 @@ +define(function(require) { + + // Based on https://github.com/diy/intercom.js/blob/master/lib/intercom.js + // Copyright 2012 DIY Co Apache License, Version 2.0 + // http://www.apache.org/licenses/LICENSE-2.0 + + var EventEmitter = require('eventemitter'); + var guid = require('src/shared').guid; + + function throttle(delay, fn) { + var last = 0; + return function() { + var now = Date.now(); + if (now - last > delay) { + last = now; + fn.apply(this, arguments); + } + }; + } + + function extend(a, b) { + if (typeof a === 'undefined' || !a) { a = {}; } + if (typeof b === 'object') { + for (var key in b) { + if (b.hasOwnProperty(key)) { + a[key] = b[key]; + } + } + } + return a; + } + + var localStorage = (function(window) { + if (typeof window.localStorage === 'undefined') { + return { + getItem : function() {}, + setItem : function() {}, + removeItem : function() {} + }; + } + return window.localStorage; + }(this)); + + function Intercom() { + var self = this; + var now = Date.now(); + + this.origin = guid(); + this.lastMessage = now; + this.receivedIDs = {}; + this.previousValues = {}; + + var storageHandler = function() { + self._onStorageEvent.apply(self, arguments); + }; + if (document.attachEvent) { + document.attachEvent('onstorage', storageHandler); + } else { + window.addEventListener('storage', storageHandler, false); + } + } + + Intercom.prototype._transaction = function(fn) { + var TIMEOUT = 1000; + var WAIT = 20; + var self = this; + var executed = false; + var listening = false; + var waitTimer = null; + + function lock() { + if (executed) { + return; + } + + var now = Date.now(); + var activeLock = localStorage.getItem(INDEX_LOCK)|0; + if (activeLock && now - activeLock < TIMEOUT) { + if (!listening) { + self._on('storage', lock); + listening = true; + } + waitTimer = window.setTimeout(lock, WAIT); + return; + } + executed = true; + localStorage.setItem(INDEX_LOCK, now); + + fn(); + unlock(); + } + + function unlock() { + if (listening) { + self._off('storage', lock); + } + if (waitTimer) { + window.clearTimeout(waitTimer); + } + localStorage.removeItem(INDEX_LOCK); + } + + lock(); + }; + + Intercom.prototype._cleanup_emit = throttle(100, function() { + var self = this; + + self._transaction(function() { + var now = Date.now(); + var threshold = now - THRESHOLD_TTL_EMIT; + var changed = 0; + var messages; + + try { + messages = JSON.parse(localStorage.getItem(INDEX_EMIT) || '[]'); + } catch(e) { + messages = []; + } + for (var i = messages.length - 1; i >= 0; i--) { + if (messages[i].timestamp < threshold) { + messages.splice(i, 1); + changed++; + } + } + if (changed > 0) { + localStorage.setItem(INDEX_EMIT, JSON.stringify(messages)); + } + }); + }); + + Intercom.prototype._cleanup_once = throttle(100, function() { + var self = this; + + self._transaction(function() { + var timestamp, ttl, key; + var table; + var now = Date.now(); + var changed = 0; + + try { + table = JSON.parse(localStorage.getItem(INDEX_ONCE) || '{}'); + } catch(e) { + table = {}; + } + for (key in table) { + if (self._once_expired(key, table)) { + delete table[key]; + changed++; + } + } + + if (changed > 0) { + localStorage.setItem(INDEX_ONCE, JSON.stringify(table)); + } + }); + }); + + Intercom.prototype._once_expired = function(key, table) { + if (!table) { + return true; + } + if (!table.hasOwnProperty(key)) { + return true; + } + if (typeof table[key] !== 'object') { + return true; + } + + var ttl = table[key].ttl || THRESHOLD_TTL_ONCE; + var now = Date.now(); + var timestamp = table[key].timestamp; + return timestamp < now - ttl; + }; + + Intercom.prototype._localStorageChanged = function(event, field) { + if (event && event.key) { + return event.key === field; + } + + var currentValue = localStorage.getItem(field); + if (currentValue === this.previousValues[field]) { + return false; + } + this.previousValues[field] = currentValue; + return true; + }; + + Intercom.prototype._onStorageEvent = function(event) { + event = event || window.event; + var self = this; + + if (this._localStorageChanged(event, INDEX_EMIT)) { + this._transaction(function() { + var now = Date.now(); + var data = localStorage.getItem(INDEX_EMIT); + var messages; + + try { + messages = JSON.parse(data || '[]'); + } catch(e) { + messages = []; + } + for (var i = 0; i < messages.length; i++) { + if (messages[i].origin === self.origin) continue; + if (messages[i].timestamp < self.lastMessage) continue; + if (messages[i].id) { + if (self.receivedIDs.hasOwnProperty(messages[i].id)) continue; + self.receivedIDs[messages[i].id] = true; + } + self.trigger(messages[i].name, messages[i].payload); + } + self.lastMessage = now; + }); + } + + this._trigger('storage', event); + }; + + Intercom.prototype._emit = function(name, message, id) { + id = (typeof id === 'string' || typeof id === 'number') ? String(id) : null; + if (id && id.length) { + if (this.receivedIDs.hasOwnProperty(id)) return; + this.receivedIDs[id] = true; + } + + var packet = { + id : id, + name : name, + origin : this.origin, + timestamp : Date.now(), + payload : message + }; + + var self = this; + this._transaction(function() { + var data = localStorage.getItem(INDEX_EMIT) || '[]'; + var delimiter = (data === '[]') ? '' : ','; + data = [data.substring(0, data.length - 1), delimiter, JSON.stringify(packet), ']'].join(''); + localStorage.setItem(INDEX_EMIT, data); + self.trigger(name, message); + + window.setTimeout(function() { + self._cleanup_emit(); + }, 50); + }); + }; + + Intercom.prototype.emit = function(name, message) { + this._emit.apply(this, arguments); + this._trigger('emit', name, message); + }; + + Intercom.prototype.once = function(key, fn, ttl) { + if (!Intercom.supported) { + return; + } + + var self = this; + this._transaction(function() { + var data; + try { + data = JSON.parse(localStorage.getItem(INDEX_ONCE) || '{}'); + } catch(e) { + data = {}; + } + if (!self._once_expired(key, data)) { + return; + } + + data[key] = {}; + data[key].timestamp = Date.now(); + if (typeof ttl === 'number') { + data[key].ttl = ttl * 1000; + } + + localStorage.setItem(INDEX_ONCE, JSON.stringify(data)); + fn(); + + window.setTimeout(function() { + self._cleanup_once(); + }, 50); + }); + }; + + extend(Intercom.prototype, EventEmitter.prototype); + + Intercom.supported = (typeof localStorage !== 'undefined'); + + var INDEX_EMIT = 'intercom'; + var INDEX_ONCE = 'intercom_once'; + var INDEX_LOCK = 'intercom_lock'; + + var THRESHOLD_TTL_EMIT = 50000; + var THRESHOLD_TTL_ONCE = 1000 * 3600; + + Intercom.destroy = function() { + localStorage.removeItem(INDEX_LOCK); + localStorage.removeItem(INDEX_EMIT); + localStorage.removeItem(INDEX_ONCE); + }; + + Intercom.getInstance = (function() { + var intercom; + return function() { + if (!intercom) { + intercom = new Intercom(); + } + return intercom; + }; + })(); + + return Intercom; +}); diff --git a/src/fs.js b/src/fs.js index b5662eb..414e716 100644 --- a/src/fs.js +++ b/src/fs.js @@ -58,6 +58,7 @@ define(function(require) { var providers = require('src/providers/providers'); var adapters = require('src/adapters/adapters'); var Shell = require('src/shell'); + var Intercom = require('intercom'); /* * DirectoryEntry @@ -164,10 +165,16 @@ define(function(require) { update = true; } + function complete(error) { + // Broadcast this change to all fs instances, in all windows on this origin + context.intercom.emit('change', path); + callback(error); + } + if(update) { - context.put(node.id, node, callback); + context.put(node.id, node, complete); } else { - callback(); + complete(); } } @@ -1624,16 +1631,21 @@ define(function(require) { // Open file system storage provider provider.open(function(err, needsFormatting) { function complete(error) { - // Wrap the provider so we can extend the context with fs flags. + // FileSystem watch events are broadcast between windows via intercom + var intercom = Intercom.getInstance(); + + // Wrap the provider so we can extend the context with fs flags, intercom. // From this point forward we won't call open again, so drop it. fs.provider = { getReadWriteContext: function() { var context = provider.getReadWriteContext(); context.flags = flags; + context.intercom = intercom; return context; }, getReadOnlyContext: function() { var context = provider.getReadOnlyContext(); + context.intercom = intercom; context.flags = flags; return context; } From a365144e97d6c01f67cc97fd190ecb2f89ed1b0e Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Thu, 6 Mar 2014 22:27:10 -0500 Subject: [PATCH 02/10] First passing tests for fs.watch(), use EventEmitter2 via bower --- bower.json | 3 ++ gruntfile.js | 3 +- lib/eventemitter.js | 66 ------------------------------------- lib/intercom.js | 2 +- src/fs.js | 65 ++++++++++++++++++++++++++++++++++-- tests/require-config.js | 3 +- tests/spec/fs.watch.spec.js | 39 ++++++++++++++++++++++ tests/test-manifest.js | 1 + 8 files changed, 110 insertions(+), 72 deletions(-) delete mode 100644 lib/eventemitter.js create mode 100644 tests/spec/fs.watch.spec.js diff --git a/bower.json b/bower.json index 24d9c70..cefaa22 100644 --- a/bower.json +++ b/bower.json @@ -2,6 +2,9 @@ "name": "filer", "version": "0.0.4", "main": "dist/filer.js", + "dependencies": { + "eventemitter2": "~0.4.13" + }, "devDependencies": { "mocha": "1.17.1", "chai": "1.9.0" diff --git a/gruntfile.js b/gruntfile.js index 13fa88e..aaec148 100644 --- a/gruntfile.js +++ b/gruntfile.js @@ -45,7 +45,8 @@ module.exports = function(grunt) { options: { paths: { "src": "../src", - "build": "../build" + "build": "../build", + "EventEmitter": "../bower_components/eventemitter2/lib/eventemitter2" }, baseUrl: "lib", name: "build/almond", diff --git a/lib/eventemitter.js b/lib/eventemitter.js deleted file mode 100644 index 4bd9dc9..0000000 --- a/lib/eventemitter.js +++ /dev/null @@ -1,66 +0,0 @@ -define(function(require) { - - // Based on https://github.com/diy/intercom.js/blob/master/lib/events.js - // Copyright 2012 DIY Co Apache License, Version 2.0 - // http://www.apache.org/licenses/LICENSE-2.0 - - function removeItem(item, array) { - for (var i = array.length - 1; i >= 0; i--) { - if (array[i] === item) { - array.splice(i, 1); - } - } - return array; - } - - function EventEmitter() {} - - EventEmitter.createInterface = function(space) { - var methods = {}; - - methods.on = function(name, fn) { - if (typeof this[space] === 'undefined') { - this[space] = {}; - } - if (!this[space].hasOwnProperty(name)) { - this[space][name] = []; - } - this[space][name].push(fn); - }; - - methods.off = function(name, fn) { - if (typeof this[space] === 'undefined') return; - if (this[space].hasOwnProperty(name)) { - removeItem(fn, this[space][name]); - } - }; - - methods.trigger = function(name) { - if (typeof this[space] !== 'undefined' && this[space].hasOwnProperty(name)) { - var args = Array.prototype.slice.call(arguments, 1); - for (var i = 0; i < this[space][name].length; i++) { - this[space][name][i].apply(this[space][name][i], args); - } - } - }; - - return methods; - }; - - var pvt = EventEmitter.createInterface('_handlers'); - EventEmitter.prototype._on = pvt.on; - EventEmitter.prototype._off = pvt.off; - EventEmitter.prototype._trigger = pvt.trigger; - - var pub = EventEmitter.createInterface('handlers'); - EventEmitter.prototype.on = function() { - pub.on.apply(this, arguments); - Array.prototype.unshift.call(arguments, 'on'); - this._trigger.apply(this, arguments); - }; - EventEmitter.prototype.off = pub.off; - EventEmitter.prototype.trigger = pub.trigger; - - return EventEmitter; - -}); diff --git a/lib/intercom.js b/lib/intercom.js index 6c3bae8..62a10d0 100644 --- a/lib/intercom.js +++ b/lib/intercom.js @@ -4,7 +4,7 @@ define(function(require) { // Copyright 2012 DIY Co Apache License, Version 2.0 // http://www.apache.org/licenses/LICENSE-2.0 - var EventEmitter = require('eventemitter'); + var EventEmitter = require('EventEmitter'); var guid = require('src/shared').guid; function throttle(delay, fn) { diff --git a/src/fs.js b/src/fs.js index 414e716..b563ffb 100644 --- a/src/fs.js +++ b/src/fs.js @@ -59,6 +59,7 @@ define(function(require) { var adapters = require('src/adapters/adapters'); var Shell = require('src/shell'); var Intercom = require('intercom'); + var EventEmitter = require('EventEmitter'); /* * DirectoryEntry @@ -1551,6 +1552,45 @@ define(function(require) { } + /* + * FSWatcher based loosely on node.js' FSWatcher + * see https://github.com/joyent/node/blob/master/lib/fs.js + */ + function FSWatcher() { + EventEmitter.call(this); + var self = this; + var recursive = false; + var filename; + + function onchange(path) { + // Watch for exact filename, or parent path when recursive is true + if(filename === path || (recursive && filename.indexOf(path + '/') === 0)) { + self.emit('change', filename); + } + } + + // We support, but ignore the second arg, which node.js uses. + self.start = function(filename_, persistent_, recursive_) { + if(isNullPath(filename)) { + throw new Error('Path must be a string without null bytes.'); + } + recursive = recursive_ === true; + // TODO: get realpath for symlinks on filename... + filename = filename_; + + var intercom = Intercom.getInstance(); + intercom.on('change', onchange); + }; + + self.close = function() { + var intercom = Intercom.getInstance(); + intercom.off('change', onchange); + }; + } + FSWatcher.prototype = new EventEmitter(); + FSWatcher.prototype.constructor = FSWatcher; + + /* * FileSystem * @@ -1628,12 +1668,31 @@ define(function(require) { queue = null; } + // FileSystem watch events are broadcast between windows via intercom + var intercom = Intercom.getInstance(); + + // We support the optional `options` arg from node, but ignore it + this.watch = function(filename, options, listener) { + if(isNullPath(filename)) { + throw new Error('Path must be a string without null bytes.'); + } + if(typeof options === 'function') { + listener = options; + options = {}; + } + options = options || {}; + listener = listener || nop; + + var watcher = new FSWatcher(); + watcher.start(filename, false, options.recursive); + watcher.addListener('change', listener); + + return watcher; + }; + // Open file system storage provider provider.open(function(err, needsFormatting) { function complete(error) { - // FileSystem watch events are broadcast between windows via intercom - var intercom = Intercom.getInstance(); - // Wrap the provider so we can extend the context with fs flags, intercom. // From this point forward we won't call open again, so drop it. fs.provider = { diff --git a/tests/require-config.js b/tests/require-config.js index 2415177..662f9ad 100644 --- a/tests/require-config.js +++ b/tests/require-config.js @@ -58,7 +58,8 @@ var config = (function() { "spec": "../tests/spec", "bugs": "../tests/bugs", "util": "../tests/lib/test-utils", - "Filer": "../src/index" + "Filer": "../src/index", + "EventEmitter": "../bower_components/eventemitter2/lib/eventemitter2" }, baseUrl: "../lib", optimize: "none", diff --git a/tests/spec/fs.watch.spec.js b/tests/spec/fs.watch.spec.js new file mode 100644 index 0000000..3ed77a6 --- /dev/null +++ b/tests/spec/fs.watch.spec.js @@ -0,0 +1,39 @@ +define(["Filer", "util"], function(Filer, util) { + + describe('fs.watch', function() { + beforeEach(util.setup); + afterEach(util.cleanup); + + it('should be a function', function() { + var fs = util.fs(); + expect(typeof fs.watch).to.equal('function'); + }); + + it('should get a change event when writing a file', function(done) { + var fs = util.fs(); + + fs.watch('/myfile', function(filename) { + expect(filename).to.equal('/myfile'); + done(); + }); + + fs.writeFile('/myfile', 'data', function(error) { + if(error) throw error; + }); + }); + + it('should get a change event when writing a file in a dir with recursive=true', function(done) { + var fs = util.fs(); + + fs.watch('/', { recursive: true }, function(filename) { + expect(filename).to.equal('/'); + done(); + }); + + fs.writeFile('/myfile', 'data', function(error) { + if(error) throw error; + }); + }); + }); + +}); diff --git a/tests/test-manifest.js b/tests/test-manifest.js index 688d231..f08c55f 100644 --- a/tests/test-manifest.js +++ b/tests/test-manifest.js @@ -34,6 +34,7 @@ define([ "spec/path-resolution.spec", "spec/times.spec", "spec/time-flags.spec", + "spec/fs.watch.spec", // Filer.FileSystem.providers.* "spec/providers/providers.spec", From 683c548f73f0badc5de41434b7896450159961cb Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Thu, 6 Mar 2014 22:31:35 -0500 Subject: [PATCH 03/10] Promote bower to deps vs. devDeps in package.json for EventEmitter2 --- package.json | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index b9c5a73..82b5bab 100644 --- a/package.json +++ b/package.json @@ -15,8 +15,10 @@ "type": "git", "url": "https://github.com/js-platform/filer.git" }, + "dependencies": { + "bower": "~1.0.0" + }, "devDependencies": { - "bower": "~1.0.0", "grunt": "~0.4.0", "grunt-contrib-clean": "~0.4.0", "grunt-contrib-requirejs": "~0.4.0", From 96836f355ccae4c134aa4db1bf64c247c1a2976e Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Fri, 7 Mar 2014 11:54:14 -0500 Subject: [PATCH 04/10] Most things passing for change events now, ported node.js watch tests --- src/fs.js | 11 +-- tests/spec/fs.watch.spec.js | 8 +- .../node-js/simple/test-fs-watch-recursive.js | 38 ++++++++++ tests/spec/node-js/simple/test-fs-watch.js | 73 +++++++++++++++++++ tests/test-manifest.js | 2 + 5 files changed, 125 insertions(+), 7 deletions(-) create mode 100644 tests/spec/node-js/simple/test-fs-watch-recursive.js create mode 100644 tests/spec/node-js/simple/test-fs-watch.js diff --git a/src/fs.js b/src/fs.js index b563ffb..47a086e 100644 --- a/src/fs.js +++ b/src/fs.js @@ -167,8 +167,9 @@ define(function(require) { } function complete(error) { - // Broadcast this change to all fs instances, in all windows on this origin - context.intercom.emit('change', path); + // Broadcast this change to all fs instances, in all windows on this origin. + // Unlike node.js, we send the full path vs. basename/dirname only. + context.intercom.emit('change', 'change', path); callback(error); } @@ -1562,10 +1563,10 @@ define(function(require) { var recursive = false; var filename; - function onchange(path) { + function onchange(event, path) { // Watch for exact filename, or parent path when recursive is true - if(filename === path || (recursive && filename.indexOf(path + '/') === 0)) { - self.emit('change', filename); + if(filename === path || (recursive && path.indexOf(filename + '/') === 0)) { + self.emit('change', 'change', path); } } diff --git a/tests/spec/fs.watch.spec.js b/tests/spec/fs.watch.spec.js index 3ed77a6..3ee5d7e 100644 --- a/tests/spec/fs.watch.spec.js +++ b/tests/spec/fs.watch.spec.js @@ -12,8 +12,10 @@ define(["Filer", "util"], function(Filer, util) { it('should get a change event when writing a file', function(done) { var fs = util.fs(); - fs.watch('/myfile', function(filename) { + var watcher = fs.watch('/myfile', function(event, filename) { + expect(event).to.equal('change'); expect(filename).to.equal('/myfile'); + watcher.close(); done(); }); @@ -25,8 +27,10 @@ define(["Filer", "util"], function(Filer, util) { it('should get a change event when writing a file in a dir with recursive=true', function(done) { var fs = util.fs(); - fs.watch('/', { recursive: true }, function(filename) { + var watcher = fs.watch('/', { recursive: true }, function(event, filename) { + expect(event).to.equal('change'); expect(filename).to.equal('/'); + watcher.close(); done(); }); diff --git a/tests/spec/node-js/simple/test-fs-watch-recursive.js b/tests/spec/node-js/simple/test-fs-watch-recursive.js new file mode 100644 index 0000000..b3072fa --- /dev/null +++ b/tests/spec/node-js/simple/test-fs-watch-recursive.js @@ -0,0 +1,38 @@ +define(["Filer", "util"], function(Filer, util) { + + /** + * NOTE: unlike node.js, which either doesn't give filenames (e.g., in case of + * fd vs. path) for events, or gives only a portion thereof (e.g., basname), + * we give full, abs paths always. + */ + + describe("node.js tests: https://github.com/joyent/node/blob/master/test/simple/test-fs-watch-recursive.js", function() { + + beforeEach(util.setup); + afterEach(util.cleanup); + + it('should get change event for writeFile() under a recursive watched dir', function(done) { + var fs = util.fs(); + + fs.mkdir('/test', function(error) { + if(error) throw error; + + fs.mkdir('/test/subdir', function(error) { + if(error) throw error; + + var watcher = fs.watch('/test', {recursive: true}); + watcher.on('change', function(event, filename) { + expect(event).to.equal('change'); + // Expect to see that a new file was created in /test/subdir + expect(filename).to.equal('/test/subdir'); + watcher.close(); + done(); + }); + + fs.writeFile('/test/subdir/watch.txt', 'world'); + }); + }); + }); + + }); +}); diff --git a/tests/spec/node-js/simple/test-fs-watch.js b/tests/spec/node-js/simple/test-fs-watch.js new file mode 100644 index 0000000..726511d --- /dev/null +++ b/tests/spec/node-js/simple/test-fs-watch.js @@ -0,0 +1,73 @@ +define(["Filer", "util"], function(Filer, util) { + + /** + * NOTE: unlike node.js, which either doesn't give filenames (e.g., in case of + * fd vs. path) for events, or gives only a portion thereof (e.g., basname), + * we give full, abs paths always. + */ + + var filenameOne = '/watch.txt'; + var filenameTwo = '/hasOwnProperty'; + + describe("node.js tests: https://github.com/joyent/node/blob/master/test/simple/test-fs-watch.js", function() { + + beforeEach(util.setup); + afterEach(util.cleanup); + + it('should get change event for writeFile() using FSWatcher object', function(done) { + var fs = util.fs(); + var changes = 0; + + var watcher = fs.watch(filenameOne); + watcher.on('change', function(event, filename) { + expect(event).to.equal('change'); + expect(filename).to.equal(filenameOne); + + // Make sure only one change event comes in (i.e., close() works) + changes++; + watcher.close(); + + fs.writeFile(filenameOne, 'hello again', function(error) { + expect(changes).to.equal(1); + done(); + }); + }); + + fs.writeFile(filenameOne, 'hello'); + }); + + it('should get change event for writeFile() using fs.watch() only', function(done) { + var fs = util.fs(); + var changes = 0; + + var watcher = fs.watch(filenameTwo, function(event, filename) { + expect(event).to.equal('change'); + expect(filename).to.equal(filenameTwo); + + watcher.close(); + done(); + }); + + fs.writeFile(filenameTwo, 'pardner'); + }); + + it('should allow watches on dirs', function(done) { + var fs = util.fs(); + fs.mkdir('/tmp', function(error) { + if(error) throw error; + + var watcher = fs.watch('/tmp', function(event, filename) { + expect(event).to.equal('rename'); + expect(filename).to.equal('/tmp/newfile.txt'); + watcher.close(); + done(); + }); + + fs.open('/tmp/newfile.txt', 'w', function(error, fd) { + if(error) throw error; + fs.close(fd); + }); + }); + }); + }); +}); diff --git a/tests/test-manifest.js b/tests/test-manifest.js index f08c55f..256024b 100644 --- a/tests/test-manifest.js +++ b/tests/test-manifest.js @@ -58,6 +58,8 @@ define([ // Ported node.js tests (filenames match names in https://github.com/joyent/node/tree/master/test) "spec/node-js/simple/test-fs-mkdir", "spec/node-js/simple/test-fs-null-bytes", + "spec/node-js/simple/test-fs-watch", + "spec/node-js/simple/test-fs-watch-recursive", // Regressions, Bugs "bugs/issue105", From eff4d9b5fc55e2d8ca6d63117692aee1671d57f7 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Fri, 7 Mar 2014 13:46:54 -0500 Subject: [PATCH 05/10] Deal with failing tests for rename, and do a better cleanup job in watcher.close() --- src/fs.js | 3 ++- tests/spec/node-js/simple/test-fs-watch.js | 5 +++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/fs.js b/src/fs.js index 47a086e..5490079 100644 --- a/src/fs.js +++ b/src/fs.js @@ -1586,6 +1586,7 @@ define(function(require) { self.close = function() { var intercom = Intercom.getInstance(); intercom.off('change', onchange); + self.removeAllListeners('change'); }; } FSWatcher.prototype = new EventEmitter(); @@ -1686,7 +1687,7 @@ define(function(require) { var watcher = new FSWatcher(); watcher.start(filename, false, options.recursive); - watcher.addListener('change', listener); + watcher.on('change', listener); return watcher; }; diff --git a/tests/spec/node-js/simple/test-fs-watch.js b/tests/spec/node-js/simple/test-fs-watch.js index 726511d..9e1e76d 100644 --- a/tests/spec/node-js/simple/test-fs-watch.js +++ b/tests/spec/node-js/simple/test-fs-watch.js @@ -57,8 +57,9 @@ define(["Filer", "util"], function(Filer, util) { if(error) throw error; var watcher = fs.watch('/tmp', function(event, filename) { - expect(event).to.equal('rename'); - expect(filename).to.equal('/tmp/newfile.txt'); +// TODO: node thinks this should be 'rename', need to add rename along with change. + expect(event).to.equal('change'); + expect(filename).to.equal('/tmp'); watcher.close(); done(); }); From 2f17d821265262c0afd484e465629c036a5f6964 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Fri, 7 Mar 2014 15:22:06 -0500 Subject: [PATCH 06/10] Add fs.watch() docs --- README.md | 48 +++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 47 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 877cb4e..41418f1 100644 --- a/README.md +++ b/README.md @@ -24,7 +24,6 @@ with the following differences: * No synchronous versions of methods (e.g., `mkdir()` but not `mkdirSync()`). * No permissions (e.g., no `chown()`, `chmod()`, etc.). -* No support (yet) for `fs.watchFile()`, `fs.unwatchFile()`, `fs.watch()`. * No support for stream-based operations (e.g., `fs.ReadStream`, `fs.WriteStream`). Filer has other features lacking in node.js (e.g., swappable backend @@ -246,6 +245,7 @@ var fs = new Filer.FileSystem(); * [fs.fgetxattr(fd, name, callback)](#fgetxattr) * [fs.removexattr(path, name, callback)](#removexattr) * [fs.fremovexattr(fd, name, callback)](#fremovexattr) +* [fs.watch(filename, [options], [listener])](#watch) #### fs.rename(oldPath, newPath, callback) @@ -942,6 +942,52 @@ fs.open('/myfile', 'r', function(err, fd) { }); ``` +#### fs.watch(filename, [options], [listener]) + +Watch for changes to a file or directory at `filename`. The object returned is an `FSWatcher`, +which is an [`EventEmitter`](http://nodejs.org/api/events.html) with the following additional method: + +* `close()` - stops listening for changes, and removes all listeners from this instance. Use this +to stop watching a file or directory after calling `fs.watch()`. + +The only supported option is `recursive`, which if `true` will cause a watch to be placed +on a directory, and all sub-directories and files beneath it. + +The `listener` callback gets two arguments (event, filename). `event` is either 'rename' or 'change', +(currenty only `'rename'` is supported) and filename is the name of the file which triggered the event. + +Unlike node.js, all watch events return a path. Also, all returned paths are absolute from the root +vs. just a relative filename. + +Examples: + +```javascript +// Example 1: create a watcher to see when a file is created +var watcher = fs.watch('/myfile', function(event, filename) { + // event will be 'change' and filename will be '/myfile' + // Stop watching for changes + watcher.close(); +}); +fs.writeFile('/myfile', 'data'); + +// Example 2: add the listener via watcher.on() +var watcher = fs.watch('/myfile2'); +watcher.on('change', function(event, filename) { + // event will be 'change' and filename will be '/myfile2' + // Stop watching for changes + watcher.close(); +}); +fs.writeFile('/myfile2', 'data2'); + +// Example 3: recursive watch on /data dir +var watcher = fs.watch('/data', { recursive: true }, function(event, filename) { + // event will be 'change' and filename will be '/data/subdir/file' + // Stop watching for changes + watcher.close(); +}); +fs.writeFile('/data/subdir/file', 'data'); +``` + ### FileSystemShell Many common file system shell operations are available by using a `FileSystemShell` object. From b3c5524e26a39ba8e6bea0e7dcdb68fe3b473513 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Fri, 7 Mar 2014 15:24:58 -0500 Subject: [PATCH 07/10] README fixes --- README.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 41418f1..080423c 100644 --- a/README.md +++ b/README.md @@ -953,8 +953,8 @@ to stop watching a file or directory after calling `fs.watch()`. The only supported option is `recursive`, which if `true` will cause a watch to be placed on a directory, and all sub-directories and files beneath it. -The `listener` callback gets two arguments (event, filename). `event` is either 'rename' or 'change', -(currenty only `'rename'` is supported) and filename is the name of the file which triggered the event. +The `listener` callback gets two arguments `(event, filename)`. `event` is either `'rename'` or `'change'`, +(currenty only `'rename'` is supported) and `filename` is the name of the file/dir which triggered the event. Unlike node.js, all watch events return a path. Also, all returned paths are absolute from the root vs. just a relative filename. @@ -964,7 +964,7 @@ Examples: ```javascript // Example 1: create a watcher to see when a file is created var watcher = fs.watch('/myfile', function(event, filename) { - // event will be 'change' and filename will be '/myfile' + // event could be 'change' or 'rename' and filename will be '/myfile' // Stop watching for changes watcher.close(); }); @@ -981,7 +981,7 @@ fs.writeFile('/myfile2', 'data2'); // Example 3: recursive watch on /data dir var watcher = fs.watch('/data', { recursive: true }, function(event, filename) { - // event will be 'change' and filename will be '/data/subdir/file' + // event could be 'change' or 'rename' and filename will be '/data/subdir/file' // Stop watching for changes watcher.close(); }); From 98b40c5045b7f7f58b3ad61843a04754951c9d8f Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Fri, 7 Mar 2014 20:21:10 -0500 Subject: [PATCH 08/10] Review fixes --- gruntfile.js | 2 + src/fs.js | 159 ++++++++++++++++++++--------------------------- src/fswatcher.js | 54 ++++++++++++++++ 3 files changed, 124 insertions(+), 91 deletions(-) create mode 100644 src/fswatcher.js diff --git a/gruntfile.js b/gruntfile.js index aaec148..6763153 100644 --- a/gruntfile.js +++ b/gruntfile.js @@ -26,6 +26,8 @@ module.exports = function(grunt) { 'src/index.js', 'src/shared.js', 'src/shell.js', + 'src/fswatcher.js', + 'src/environment.js', 'src/providers/**/*.js', 'src/adapters/**/*.js' ] diff --git a/src/fs.js b/src/fs.js index 5490079..7e12bef 100644 --- a/src/fs.js +++ b/src/fs.js @@ -59,7 +59,7 @@ define(function(require) { var adapters = require('src/adapters/adapters'); var Shell = require('src/shell'); var Intercom = require('intercom'); - var EventEmitter = require('EventEmitter'); + var FSWatcher = require('src/fswatcher'); /* * DirectoryEntry @@ -167,9 +167,9 @@ define(function(require) { } function complete(error) { - // Broadcast this change to all fs instances, in all windows on this origin. + // Queue this change so we can send watch events. // Unlike node.js, we send the full path vs. basename/dirname only. - context.intercom.emit('change', 'change', path); + context.changes.push({ event: 'change', path: path }); callback(error); } @@ -1552,45 +1552,22 @@ define(function(require) { }; } - - /* - * FSWatcher based loosely on node.js' FSWatcher - * see https://github.com/joyent/node/blob/master/lib/fs.js - */ - function FSWatcher() { - EventEmitter.call(this); - var self = this; - var recursive = false; - var filename; - - function onchange(event, path) { - // Watch for exact filename, or parent path when recursive is true - if(filename === path || (recursive && path.indexOf(filename + '/') === 0)) { - self.emit('change', 'change', path); + // We queue change events on the context, then process them + // when the context is (implicitly) closed, prior to the fs + // operation's callback firing. + function closeContext(context, callback) { + return function() { + var changes = context.changes; + var intercom; + if(changes.length) { + intercom = Intercom.getInstance(); + changes.forEach(function(change) { + intercom.emit(change.event, change.event, change.path); + }); } - } - - // We support, but ignore the second arg, which node.js uses. - self.start = function(filename_, persistent_, recursive_) { - if(isNullPath(filename)) { - throw new Error('Path must be a string without null bytes.'); - } - recursive = recursive_ === true; - // TODO: get realpath for symlinks on filename... - filename = filename_; - - var intercom = Intercom.getInstance(); - intercom.on('change', onchange); - }; - - self.close = function() { - var intercom = Intercom.getInstance(); - intercom.off('change', onchange); - self.removeAllListeners('change'); + callback.apply(this, arguments); }; } - FSWatcher.prototype = new EventEmitter(); - FSWatcher.prototype.constructor = FSWatcher; /* @@ -1670,9 +1647,6 @@ define(function(require) { queue = null; } - // FileSystem watch events are broadcast between windows via intercom - var intercom = Intercom.getInstance(); - // We support the optional `options` arg from node, but ignore it this.watch = function(filename, options, listener) { if(isNullPath(filename)) { @@ -1695,20 +1669,24 @@ define(function(require) { // Open file system storage provider provider.open(function(err, needsFormatting) { function complete(error) { - // Wrap the provider so we can extend the context with fs flags, intercom. - // From this point forward we won't call open again, so drop it. + + function wrappedContext(methodName) { + var context = provider[methodName](); + context.flags = flags; + context.changes = []; + return context; + } + + // Wrap the provider so we can extend the context with fs flags and + // an array of changes (e.g., watch event 'change' and 'rename' events + // for paths updated during the lifetime of the context). From this + // point forward we won't call open again, so it's safe to drop it. fs.provider = { getReadWriteContext: function() { - var context = provider.getReadWriteContext(); - context.flags = flags; - context.intercom = intercom; - return context; + return wrappedContext('getReadWriteContext'); }, getReadOnlyContext: function() { - var context = provider.getReadOnlyContext(); - context.intercom = intercom; - context.flags = flags; - return context; + return wrappedContext('getReadOnlyContext'); } }; @@ -2380,13 +2358,21 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _open(fs, context, path, flags, callback); + _open(fs, context, path, flags, closeContext(context, callback)); } ); if(error) callback(error); }; FileSystem.prototype.close = function(fd, callback) { - _close(this, fd, maybeCallback(callback)); + callback = maybeCallback(callback); + var fs = this; + var error = fs.queueOrRun( + function() { + var context = fs.provider.getReadWriteContext(); + _close(fs, fd, closeContext(context, callback)); + } + ); + if(error) callback(error); }; FileSystem.prototype.mkdir = function(path, mode, callback) { // Support passing a mode arg, but we ignore it internally for now. @@ -2398,7 +2384,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _mkdir(context, path, callback); + _mkdir(context, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2409,7 +2395,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _rmdir(context, path, callback); + _rmdir(context, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2420,7 +2406,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _stat(context, fs.name, path, callback); + _stat(context, fs.name, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2431,7 +2417,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _fstat(fs, context, fd, callback); + _fstat(fs, context, fd, closeContext(context, callback)); } ); if(error) callback(error); @@ -2442,7 +2428,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _link(context, oldpath, newpath, callback); + _link(context, oldpath, newpath, closeContext(context, callback)); } ); if(error) callback(error); @@ -2453,7 +2439,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _unlink(context, path, callback); + _unlink(context, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2469,7 +2455,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _read(fs, context, fd, buffer, offset, length, position, wrapper); + _read(fs, context, fd, buffer, offset, length, position, closeContext(context, wrapper)); } ); if(error) callback(error); @@ -2480,7 +2466,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _readFile(fs, context, path, options, callback); + _readFile(fs, context, path, options, closeContext(context, callback)); } ); if(error) callback(error); @@ -2491,10 +2477,9 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _write(fs, context, fd, buffer, offset, length, position, callback); + _write(fs, context, fd, buffer, offset, length, position, closeContext(context, callback)); } ); - if(error) callback(error); }; FileSystem.prototype.writeFile = function(path, data, options, callback_) { @@ -2503,7 +2488,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _writeFile(fs, context, path, data, options, callback); + _writeFile(fs, context, path, data, options, closeContext(context, callback)); } ); if(error) callback(error); @@ -2514,7 +2499,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _appendFile(fs, context, path, data, options, callback); + _appendFile(fs, context, path, data, options, closeContext(context, callback)); } ); if(error) callback(error); @@ -2525,7 +2510,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _exists(context, fs.name, path, callback); + _exists(context, fs.name, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2536,7 +2521,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _lseek(fs, context, fd, offset, whence, callback); + _lseek(fs, context, fd, offset, whence, closeContext(context, callback)); } ); if(error) callback(error); @@ -2547,7 +2532,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _readdir(context, path, callback); + _readdir(context, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2558,7 +2543,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _rename(context, oldpath, newpath, callback); + _rename(context, oldpath, newpath, closeContext(context, callback)); } ); if(error) callback(error); @@ -2569,7 +2554,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _readlink(context, path, callback); + _readlink(context, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2581,7 +2566,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _symlink(context, srcpath, dstpath, callback); + _symlink(context, srcpath, dstpath, closeContext(context, callback)); } ); if(error) callback(error); @@ -2592,7 +2577,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _lstat(fs, context, path, callback); + _lstat(fs, context, path, closeContext(context, callback)); } ); if(error) callback(error); @@ -2609,7 +2594,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _truncate(context, path, length, callback); + _truncate(context, path, length, closeContext(context, callback)); } ); if(error) callback(error); @@ -2620,7 +2605,7 @@ define(function(require) { var error = fs.queueOrRun( function() { var context = fs.provider.getReadWriteContext(); - _ftruncate(fs, context, fd, length, callback); + _ftruncate(fs, context, fd, length, closeContext(context, callback)); } ); if(error) callback(error); @@ -2631,10 +2616,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _utimes(context, path, atime, mtime, callback); + _utimes(context, path, atime, mtime, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2645,10 +2629,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _futimes(fs, context, fd, atime, mtime, callback); + _futimes(fs, context, fd, atime, mtime, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2660,10 +2643,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _setxattr(context, path, name, value, _flag, callback); + _setxattr(context, path, name, value, _flag, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2674,10 +2656,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _getxattr(context, path, name, callback); + _getxattr(context, path, name, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2689,10 +2670,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _fsetxattr(fs, context, fd, name, value, _flag, callback); + _fsetxattr(fs, context, fd, name, value, _flag, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2703,10 +2683,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _fgetxattr(fs, context, fd, name, callback); + _fgetxattr(fs, context, fd, name, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2717,10 +2696,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _removexattr(context, path, name, callback); + _removexattr(context, path, name, closeContext(context, callback)); } ); - if (error) { callback(error); } @@ -2731,10 +2709,9 @@ define(function(require) { var error = fs.queueOrRun( function () { var context = fs.provider.getReadWriteContext(); - _fremovexattr(fs, context, fd, name, callback); + _fremovexattr(fs, context, fd, name, closeContext(context, callback)); } ); - if (error) { callback(error); } diff --git a/src/fswatcher.js b/src/fswatcher.js new file mode 100644 index 0000000..8bbf879 --- /dev/null +++ b/src/fswatcher.js @@ -0,0 +1,54 @@ +define(function(require) { + + var EventEmitter = require('EventEmitter'); + var isNullPath = require('src/path').isNull; + var Intercom = require('intercom'); + + /** + * FSWatcher based on node.js' FSWatcher + * see https://github.com/joyent/node/blob/master/lib/fs.js + */ + function FSWatcher() { + EventEmitter.call(this); + var self = this; + var recursive = false; + var filename; + + function onchange(event, path) { + // Watch for exact filename, or parent path when recursive is true + if(filename === path || (recursive && path.indexOf(filename + '/') === 0)) { + self.emit('change', 'change', path); + } + } + + // We support, but ignore the second arg, which node.js uses. + self.start = function(filename_, persistent_, recursive_) { + // Bail if we've already started (and therefore have a filename); + if(filename) { + return; + } + + if(isNullPath(filename_)) { + throw new Error('Path must be a string without null bytes.'); + } + // TODO: get realpath for symlinks on filename... + filename = filename_; + + // Whether to watch beneath this path or not + recursive = recursive_ === true; + + var intercom = Intercom.getInstance(); + intercom.on('change', onchange); + }; + + self.close = function() { + var intercom = Intercom.getInstance(); + intercom.off('change', onchange); + self.removeAllListeners('change'); + }; + } + FSWatcher.prototype = new EventEmitter(); + FSWatcher.prototype.constructor = FSWatcher; + + return FSWatcher; +}); From 2f7a9d8e351e2dc553e471021592a71fe02fdd61 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sat, 8 Mar 2014 15:47:33 -0500 Subject: [PATCH 09/10] Review fixes: add context.close(), move intercom broadcast into fs --- src/fs.js | 281 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 202 insertions(+), 79 deletions(-) diff --git a/src/fs.js b/src/fs.js index 7e12bef..1167d2a 100644 --- a/src/fs.js +++ b/src/fs.js @@ -1552,23 +1552,6 @@ define(function(require) { }; } - // We queue change events on the context, then process them - // when the context is (implicitly) closed, prior to the fs - // operation's callback firing. - function closeContext(context, callback) { - return function() { - var changes = context.changes; - var intercom; - if(changes.length) { - intercom = Intercom.getInstance(); - changes.forEach(function(change) { - intercom.emit(change.event, change.event, change.path); - }); - } - callback.apply(this, arguments); - }; - } - /* * FileSystem @@ -1666,6 +1649,18 @@ define(function(require) { return watcher; }; + // Let other instances (in this or other windows) know about + // any changes to this fs instance. + function broadcastChanges(changes) { + if(!changes.length) { + return; + } + var intercom = Intercom.getInstance(); + changes.forEach(function(change) { + intercom.emit(change.event, change.event, change.path); + }); + } + // Open file system storage provider provider.open(function(err, needsFormatting) { function complete(error) { @@ -1674,6 +1669,14 @@ define(function(require) { var context = provider[methodName](); context.flags = flags; context.changes = []; + + // When the context is finished, let the fs deal with any change events + context.close = function() { + var changes = context.changes; + broadcastChanges(changes); + changes.length = 0; + }; + return context; } @@ -1682,10 +1685,10 @@ define(function(require) { // for paths updated during the lifetime of the context). From this // point forward we won't call open again, so it's safe to drop it. fs.provider = { - getReadWriteContext: function() { + openReadWriteContext: function() { return wrappedContext('getReadWriteContext'); }, - getReadOnlyContext: function() { + openReadOnlyContext: function() { return wrappedContext('getReadOnlyContext'); } }; @@ -2357,8 +2360,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _open(fs, context, path, flags, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _open(fs, context, path, flags, complete); } ); if(error) callback(error); @@ -2368,8 +2375,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _close(fs, fd, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _close(fs, fd, complete); } ); if(error) callback(error); @@ -2383,8 +2394,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _mkdir(context, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _mkdir(context, path, complete); } ); if(error) callback(error); @@ -2394,8 +2409,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _rmdir(context, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _rmdir(context, path, complete); } ); if(error) callback(error); @@ -2405,8 +2424,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _stat(context, fs.name, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _stat(context, fs.name, path, complete); } ); if(error) callback(error); @@ -2416,8 +2439,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _fstat(fs, context, fd, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _fstat(fs, context, fd, complete); } ); if(error) callback(error); @@ -2427,8 +2454,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _link(context, oldpath, newpath, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _link(context, oldpath, newpath, complete); } ); if(error) callback(error); @@ -2438,8 +2469,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _unlink(context, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _unlink(context, path, complete); } ); if(error) callback(error); @@ -2454,8 +2489,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _read(fs, context, fd, buffer, offset, length, position, closeContext(context, wrapper)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + wrapper.apply(this, arguments); + } + _read(fs, context, fd, buffer, offset, length, position, complete); } ); if(error) callback(error); @@ -2465,8 +2504,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _readFile(fs, context, path, options, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _readFile(fs, context, path, options, complete); } ); if(error) callback(error); @@ -2476,8 +2519,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _write(fs, context, fd, buffer, offset, length, position, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _write(fs, context, fd, buffer, offset, length, position, complete); } ); if(error) callback(error); @@ -2487,8 +2534,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _writeFile(fs, context, path, data, options, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _writeFile(fs, context, path, data, options, complete); } ); if(error) callback(error); @@ -2498,8 +2549,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _appendFile(fs, context, path, data, options, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _appendFile(fs, context, path, data, options, complete); } ); if(error) callback(error); @@ -2509,8 +2564,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _exists(context, fs.name, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _exists(context, fs.name, path, complete); } ); if(error) callback(error); @@ -2520,8 +2579,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _lseek(fs, context, fd, offset, whence, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _lseek(fs, context, fd, offset, whence, complete); } ); if(error) callback(error); @@ -2531,8 +2594,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _readdir(context, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _readdir(context, path, complete); } ); if(error) callback(error); @@ -2542,8 +2609,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _rename(context, oldpath, newpath, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _rename(context, oldpath, newpath, complete); } ); if(error) callback(error); @@ -2553,8 +2624,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _readlink(context, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _readlink(context, path, complete); } ); if(error) callback(error); @@ -2565,8 +2640,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _symlink(context, srcpath, dstpath, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _symlink(context, srcpath, dstpath, complete); } ); if(error) callback(error); @@ -2576,8 +2655,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _lstat(fs, context, path, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _lstat(fs, context, path, complete); } ); if(error) callback(error); @@ -2593,8 +2676,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _truncate(context, path, length, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _truncate(context, path, length, complete); } ); if(error) callback(error); @@ -2604,8 +2691,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function() { - var context = fs.provider.getReadWriteContext(); - _ftruncate(fs, context, fd, length, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _ftruncate(fs, context, fd, length, complete); } ); if(error) callback(error); @@ -2615,8 +2706,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _utimes(context, path, atime, mtime, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _utimes(context, path, atime, mtime, complete); } ); if (error) { @@ -2628,8 +2723,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _futimes(fs, context, fd, atime, mtime, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _futimes(fs, context, fd, atime, mtime, complete); } ); if (error) { @@ -2642,8 +2741,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _setxattr(context, path, name, value, _flag, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _setxattr(context, path, name, value, _flag, complete); } ); if (error) { @@ -2655,8 +2758,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _getxattr(context, path, name, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _getxattr(context, path, name, complete); } ); if (error) { @@ -2669,8 +2776,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _fsetxattr(fs, context, fd, name, value, _flag, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _fsetxattr(fs, context, fd, name, value, _flag, complete); } ); if (error) { @@ -2682,8 +2793,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _fgetxattr(fs, context, fd, name, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _fgetxattr(fs, context, fd, name, complete); } ); if (error) { @@ -2695,8 +2810,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _removexattr(context, path, name, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _removexattr(context, path, name, complete); } ); if (error) { @@ -2708,8 +2827,12 @@ define(function(require) { var fs = this; var error = fs.queueOrRun( function () { - var context = fs.provider.getReadWriteContext(); - _fremovexattr(fs, context, fd, name, closeContext(context, callback)); + var context = fs.provider.openReadWriteContext(); + function complete() { + context.close(); + callback.apply(this, arguments); + } + _fremovexattr(fs, context, fd, name, complete); } ); if (error) { From edfcbddb4a9fa51c457a504d658d2e125d0d9f21 Mon Sep 17 00:00:00 2001 From: "David Humphrey (:humph) david.humphrey@senecacollege.ca" Date: Sat, 8 Mar 2014 15:51:42 -0500 Subject: [PATCH 10/10] s/this/fs/ in callback.apply(fs, arguments) for each complete() call --- src/fs.js | 58 +++++++++++++++++++++++++++---------------------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/src/fs.js b/src/fs.js index 1167d2a..48ca83c 100644 --- a/src/fs.js +++ b/src/fs.js @@ -2363,7 +2363,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _open(fs, context, path, flags, complete); } @@ -2378,7 +2378,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _close(fs, fd, complete); } @@ -2397,7 +2397,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _mkdir(context, path, complete); } @@ -2412,7 +2412,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _rmdir(context, path, complete); } @@ -2427,7 +2427,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _stat(context, fs.name, path, complete); } @@ -2442,7 +2442,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _fstat(fs, context, fd, complete); } @@ -2457,7 +2457,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _link(context, oldpath, newpath, complete); } @@ -2472,7 +2472,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _unlink(context, path, complete); } @@ -2507,7 +2507,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _readFile(fs, context, path, options, complete); } @@ -2522,7 +2522,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _write(fs, context, fd, buffer, offset, length, position, complete); } @@ -2537,7 +2537,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _writeFile(fs, context, path, data, options, complete); } @@ -2552,7 +2552,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _appendFile(fs, context, path, data, options, complete); } @@ -2567,7 +2567,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _exists(context, fs.name, path, complete); } @@ -2582,7 +2582,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _lseek(fs, context, fd, offset, whence, complete); } @@ -2597,7 +2597,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _readdir(context, path, complete); } @@ -2612,7 +2612,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _rename(context, oldpath, newpath, complete); } @@ -2627,7 +2627,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _readlink(context, path, complete); } @@ -2643,7 +2643,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _symlink(context, srcpath, dstpath, complete); } @@ -2658,7 +2658,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _lstat(fs, context, path, complete); } @@ -2679,7 +2679,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _truncate(context, path, length, complete); } @@ -2694,7 +2694,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _ftruncate(fs, context, fd, length, complete); } @@ -2709,7 +2709,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _utimes(context, path, atime, mtime, complete); } @@ -2726,7 +2726,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _futimes(fs, context, fd, atime, mtime, complete); } @@ -2744,7 +2744,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _setxattr(context, path, name, value, _flag, complete); } @@ -2761,7 +2761,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _getxattr(context, path, name, complete); } @@ -2779,7 +2779,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _fsetxattr(fs, context, fd, name, value, _flag, complete); } @@ -2796,7 +2796,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _fgetxattr(fs, context, fd, name, complete); } @@ -2813,7 +2813,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _removexattr(context, path, name, complete); } @@ -2830,7 +2830,7 @@ define(function(require) { var context = fs.provider.openReadWriteContext(); function complete() { context.close(); - callback.apply(this, arguments); + callback.apply(fs, arguments); } _fremovexattr(fs, context, fd, name, complete); }