Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Commit

Permalink
Merge pull request #10304 from adobe/kai/fix-10183-apply-filter-on-fs…
Browse files Browse the repository at this point in the history
…-events

Apply exclude filter when handling fs events.
  • Loading branch information
abose committed Jul 24, 2015
2 parents 9b45573 + 97e6849 commit 7a9d7bf
Show file tree
Hide file tree
Showing 8 changed files with 124 additions and 16 deletions.
4 changes: 3 additions & 1 deletion src/filesystem/Directory.js
Expand Up @@ -253,7 +253,9 @@ define(function (require, exports, module) {
try {
callback(null, stat);
} finally {
this._fileSystem._fireChangeEvent(parent, added, removed);
if (parent._isWatched()) {
this._fileSystem._fireChangeEvent(parent, added, removed);
}
// Unblock external change events
this._fileSystem._endChange();
}
Expand Down
9 changes: 6 additions & 3 deletions src/filesystem/File.js
Expand Up @@ -183,9 +183,12 @@ define(function (require, exports, module) {
// Notify the caller
callback(null, stat);
} finally {
// If the write succeeded, fire a synthetic change event
this._fileSystem._fireChangeEvent(parent, added, removed);

if (parent._isWatched()) {
// If the write succeeded and the parent directory is watched,
// fire a synthetic change event
this._fileSystem._fireChangeEvent(parent, added, removed);

}
// Always unblock external change events
this._fileSystem._endChange();
}
Expand Down
10 changes: 6 additions & 4 deletions src/filesystem/FileSystem.js
Expand Up @@ -826,10 +826,12 @@ define(function (require, exports, module) {
this._handleDirectoryChange(entry, function (added, removed) {
entry._stat = stat;

// We send a change even if added & removed are both zero-length. Something may still have changed,
// e.g. a file may have been quickly removed & re-added before we got a chance to reread the directory
// listing.
this._fireChangeEvent(entry, added, removed);
if (entry._isWatched()) {
// We send a change even if added & removed are both zero-length. Something may still have changed,
// e.g. a file may have been quickly removed & re-added before we got a chance to reread the directory
// listing.
this._fireChangeEvent(entry, added, removed);
}
}.bind(this));
}
}
Expand Down
27 changes: 20 additions & 7 deletions src/filesystem/FileSystemEntry.js
Expand Up @@ -168,13 +168,13 @@ define(function (require, exports, module) {
* Cached copy of this entry's watched root
* @type {entry: File|Directory, filter: function(FileSystemEntry):boolean, active: boolean}
*/
FileSystemEntry.prototype._watchedRoot = null;
FileSystemEntry.prototype._watchedRoot = undefined;

/**
* Cached result of _watchedRoot.filter(this.name, this.parentPath).
* @type {boolean}
*/
FileSystemEntry.prototype._watchedRootFilterResult = false;
FileSystemEntry.prototype._watchedRootFilterResult = undefined;

/**
* Determines whether or not the entry is watched.
Expand All @@ -192,7 +192,16 @@ define(function (require, exports, module) {

if (watchedRoot) {
this._watchedRoot = watchedRoot;
filterResult = watchedRoot.filter(this._name, this._parentPath);
if (watchedRoot.entry !== this) { // avoid creating entries for root's parent
var parentEntry = this._fileSystem.getDirectoryForPath(this._parentPath);
if (parentEntry._isWatched() === false) {
filterResult = false;
} else {
filterResult = watchedRoot.filter(this._name, this._parentPath);
}
} else { // root itself is watched
filterResult = true;
}
this._watchedRootFilterResult = filterResult;
}
}
Expand Down Expand Up @@ -383,8 +392,10 @@ define(function (require, exports, module) {
// Notify the caller
callback(err);
} finally {
// Notify change listeners
this._fileSystem._fireChangeEvent(parent, added, removed);
if (parent._isWatched()) {
// Notify change listeners
this._fileSystem._fireChangeEvent(parent, added, removed);
}

// Unblock external change events
this._fileSystem._endChange();
Expand Down Expand Up @@ -421,8 +432,10 @@ define(function (require, exports, module) {
// Notify the caller
callback(err);
} finally {
// Notify change listeners
this._fileSystem._fireChangeEvent(parent, added, removed);
if (parent._isWatched()) {
// Notify change listeners
this._fileSystem._fireChangeEvent(parent, added, removed);
}

// Unblock external change events
this._fileSystem._endChange();
Expand Down
19 changes: 19 additions & 0 deletions test/node/TestingDomain.js
Expand Up @@ -68,6 +68,25 @@ function init(domainManager) {
}
]
);
domainManager.registerCommand(
"testing",
"rename",
fs.rename,
true,
"Rename a file or directory.",
[
{
name: "src",
type: "string",
description: "source path"
},
{
name: "dest",
type: "string",
description: "destination path"
}
]
);
}

exports.init = init;
Empty file.
62 changes: 61 additions & 1 deletion test/spec/ProjectManager-test.js
Expand Up @@ -54,6 +54,10 @@ define(function (require, exports, module) {
runs(function () {
waitsForDone(SpecRunnerUtils.copy(testPath, tempDir), "copy temp files");
});

runs(function () {
waitsForDone(SpecRunnerUtils.rename(tempDir + "/git/", tempDir + "/.git/"), "move files");
});

SpecRunnerUtils.createTestWindowAndRun(this, function (w) {
testWindow = w;
Expand Down Expand Up @@ -120,7 +124,7 @@ define(function (require, exports, module) {
expect(stat.isFile).toBe(true);
});
});

it("should fail when a file already exists", function () {
var didCreate = false, gotError = false;

Expand Down Expand Up @@ -243,6 +247,62 @@ define(function (require, exports, module) {
runs(assertFile);
}
});

// Issue #10183 -- Brackets writing to filtered directories could cause them to appear
// in the file tree
it("should not display excluded entry when resolved and written to", function () {
var opFailed = false,
doneResolving = false,
doneWriting = false,
entry;

runs(function () {
var found = testWindow.$(".jstree-brackets span:contains(\".git\")").length;
expect(found).toBe(0);
});

runs(function () {
FileSystem.resolve(ProjectManager.getProjectRoot().fullPath + ".git/", function (err, e, stat) {
if (err) {
opFailed = true;
return;
}
entry = e;
doneResolving = true;
});
});

waitsFor(function () {
return !opFailed && doneResolving;
}, "FileSystem.resolve()", 500);

runs(function () {
var file = FileSystem.getFileForPath(entry.fullPath + "test");
file.write("hi there!", function (err) {
if (err) {
opFailed = true;
return;
}
doneWriting = true;
});
});

waitsFor(function () {
return !opFailed && doneWriting;
}, "create a file under .git", 500);

// wait for the fs event to propagate to the project model
waits(500);

runs(function () {
var found = testWindow.$(".jstree-brackets span:contains(\".git\")").length,
sanity = testWindow.$(".jstree-brackets span:contains(\"file\") + span:contains(\".js\")").length;
expect(sanity).toBe(1);
expect(found).toBe(0);
});

});

});

describe("deleteItem", function () {
Expand Down
9 changes: 9 additions & 0 deletions test/spec/SpecRunnerUtils.js
Expand Up @@ -139,6 +139,14 @@ define(function (require, exports, module) {
return testDomain().copy(src, dest);
}

/**
* Rename a directory or file.
* @return {$.Promise} Resolved when the path is rename, rejected if there was a problem
*/
function rename(src, dest) {
return testDomain().rename(src, dest);
}

/**
* Resolves a path string to a File or Directory
* @param {!string} path Path to a file or directory
Expand Down Expand Up @@ -1350,6 +1358,7 @@ define(function (require, exports, module) {
exports.chmod = chmod;
exports.remove = remove;
exports.copy = copy;
exports.rename = rename;
exports.getTestRoot = getTestRoot;
exports.getTestPath = getTestPath;
exports.getTempDirectory = getTempDirectory;
Expand Down

0 comments on commit 7a9d7bf

Please sign in to comment.