From 5d181f59d8f66cf94f79e612e83bdb6b8d1fb38c Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Sat, 3 Jan 2015 20:45:26 -0300 Subject: [PATCH 01/10] Apply exclude filter when handling fs events. --- src/project/ProjectModel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project/ProjectModel.js b/src/project/ProjectModel.js index d98fe18ad6f..18d1963a4ac 100644 --- a/src/project/ProjectModel.js +++ b/src/project/ProjectModel.js @@ -1143,7 +1143,7 @@ define(function (require, exports, module) { return; } - if (!this.isWithinProject(entry)) { + if (!this.isWithinProject(entry) || !shouldShow(entry)) { return; } From 6514f20da0f3612b4d873b2d0a6e4a8edf6763ac Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Sat, 3 Jan 2015 21:44:34 -0300 Subject: [PATCH 02/10] Do not add entry to an index if it's filtered out. This will prevent calls to FileSystem.resolve on excluded entries to add them to FileSystem index, thus avoiding generating fs events on these. --- src/filesystem/FileSystem.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index f1df5cccabf..4aaac15486b 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -544,7 +544,9 @@ define(function (require, exports, module) { if (!entry) { entry = new EntryConstructor(path, this); - this._index.addEntry(entry); + if (this._indexFilter(path, entry.name)) { + this._index.addEntry(entry); + } } return entry; From c798cea07a25eb90da51ea1a82080782e76865ad Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Fri, 9 Jan 2015 02:29:56 -0300 Subject: [PATCH 03/10] Revert "Do not add entry to an index if it's filtered out." This reverts commit 6514f20da0f3612b4d873b2d0a6e4a8edf6763ac. --- src/filesystem/FileSystem.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index 4aaac15486b..f1df5cccabf 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -544,9 +544,7 @@ define(function (require, exports, module) { if (!entry) { entry = new EntryConstructor(path, this); - if (this._indexFilter(path, entry.name)) { - this._index.addEntry(entry); - } + this._index.addEntry(entry); } return entry; From 4595b56f1ba1a6d19fc6479d8c7d5e57de8d2459 Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Fri, 9 Jan 2015 02:30:08 -0300 Subject: [PATCH 04/10] Revert "Apply exclude filter when handling fs events." This reverts commit 5d181f59d8f66cf94f79e612e83bdb6b8d1fb38c. --- src/project/ProjectModel.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/project/ProjectModel.js b/src/project/ProjectModel.js index 18d1963a4ac..d98fe18ad6f 100644 --- a/src/project/ProjectModel.js +++ b/src/project/ProjectModel.js @@ -1143,7 +1143,7 @@ define(function (require, exports, module) { return; } - if (!this.isWithinProject(entry) || !shouldShow(entry)) { + if (!this.isWithinProject(entry)) { return; } From eda5909ec55db80d9967d33a47a7d506ef9bdec9 Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Fri, 9 Jan 2015 04:31:03 -0300 Subject: [PATCH 05/10] Added integration test for #10183. --- test/spec/ProjectManager-test.js | 73 ++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/test/spec/ProjectManager-test.js b/test/spec/ProjectManager-test.js index 6035e241fea..d8e0cdb4406 100644 --- a/test/spec/ProjectManager-test.js +++ b/test/spec/ProjectManager-test.js @@ -120,6 +120,79 @@ define(function (require, exports, module) { expect(stat.isFile).toBe(true); }); }); + + it("should not display excluded entry when resolved and written to", function () { + var opFailed = false, + doneCreating = false, + doneResolving = false, + doneWriting = false, + doneLooking = false, + found = false, + entry; + + runs(function () { + ProjectManager.createNewItem(ProjectManager.getProjectRoot.fullPath, ".git/", true, true) + .fail(function () { + opFailed = true; + }) + .always(function () { + doneCreating = true; + }); + }); + + waitsFor(function () { + return !opFailed && doneCreating; + }, "create .git directory", 500); + + 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); + + runs(function () { + ProjectManager.showInTree(entry) + .done(function () { + found = true; + }) + .always(function () { + doneLooking = true; + }); + }); + + waitsFor(function () { + return doneLooking; + }, "lookup the file in the file tree", 500); + + runs(function () { + expect(found).toBe(false); + }); + + }); it("should fail when a file already exists", function () { var didCreate = false, gotError = false; From 8d30f53a62c7b87e5dc22a4d4be196425b248f81 Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Fri, 9 Jan 2015 18:32:03 -0300 Subject: [PATCH 06/10] Fix the way .git directory is created. --- test/spec/ProjectManager-test-files/git/index | 0 test/spec/ProjectManager-test.js | 13 +------------ 2 files changed, 1 insertion(+), 12 deletions(-) create mode 100644 test/spec/ProjectManager-test-files/git/index diff --git a/test/spec/ProjectManager-test-files/git/index b/test/spec/ProjectManager-test-files/git/index new file mode 100644 index 00000000000..e69de29bb2d diff --git a/test/spec/ProjectManager-test.js b/test/spec/ProjectManager-test.js index d8e0cdb4406..24c6dc382e8 100644 --- a/test/spec/ProjectManager-test.js +++ b/test/spec/ProjectManager-test.js @@ -123,7 +123,6 @@ define(function (require, exports, module) { it("should not display excluded entry when resolved and written to", function () { var opFailed = false, - doneCreating = false, doneResolving = false, doneWriting = false, doneLooking = false, @@ -131,19 +130,9 @@ define(function (require, exports, module) { entry; runs(function () { - ProjectManager.createNewItem(ProjectManager.getProjectRoot.fullPath, ".git/", true, true) - .fail(function () { - opFailed = true; - }) - .always(function () { - doneCreating = true; - }); + waitsForDone(SpecRunnerUtils.copy(tempDir + "/git/", tempDir + "/.git/"), "copy git to .git"); }); - waitsFor(function () { - return !opFailed && doneCreating; - }, "create .git directory", 500); - runs(function () { FileSystem.resolve(ProjectManager.getProjectRoot().fullPath + ".git/", function (err, e, stat) { if (err) { From 36e0f1e10b85e4e9536c862b0c45818cd2530248 Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Sun, 11 Jan 2015 17:23:43 -0300 Subject: [PATCH 07/10] Partial fix: ensure entry is watched. Make sure entry is watched before handling any external change related to it. --- src/filesystem/FileSystem.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index f1df5cccabf..db31605800e 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -797,7 +797,7 @@ define(function (require, exports, module) { path = this._normalizePath(path, false); var entry = this._index.getEntry(path); - if (entry) { + if (entry && entry._isWatched()) { var oldStat = entry._stat; if (entry.isFile) { // Update stat and clear contents, but only if out of date From 2a3ba2cd4bbf6dbf032c842fd825384458c09b2f Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Wed, 14 Jan 2015 00:09:24 -0300 Subject: [PATCH 08/10] Fix the test to use DOM lookup instead of API. ProjectManager.showInTree() does not reject the promise if not found. --- test/spec/ProjectManager-test.js | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/test/spec/ProjectManager-test.js b/test/spec/ProjectManager-test.js index 24c6dc382e8..31d9ee6340a 100644 --- a/test/spec/ProjectManager-test.js +++ b/test/spec/ProjectManager-test.js @@ -121,11 +121,12 @@ define(function (require, exports, module) { }); }); + // 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, - doneLooking = false, found = false, entry; @@ -163,24 +164,15 @@ define(function (require, exports, module) { return !opFailed && doneWriting; }, "create a file under .git", 500); - runs(function () { - ProjectManager.showInTree(entry) - .done(function () { - found = true; - }) - .always(function () { - doneLooking = true; - }); - }); - - waitsFor(function () { - return doneLooking; - }, "lookup the file in the file tree", 500); + // unfortunately, this wait has to happen for the fs event to propagate to + // the project model + waits(500); runs(function () { - expect(found).toBe(false); + found = testWindow.$(".jstree-brackets span:contains(\".git\")").length; + expect(found).toBe(0); }); - + }); it("should fail when a file already exists", function () { From 9794a6531e7868f55279a2114d032850b6377d69 Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Wed, 14 Jan 2015 02:56:28 -0300 Subject: [PATCH 09/10] Improve the test. --- test/node/TestingDomain.js | 19 ++++++ test/spec/ProjectManager-test.js | 114 ++++++++++++++++--------------- test/spec/SpecRunnerUtils.js | 9 +++ 3 files changed, 88 insertions(+), 54 deletions(-) diff --git a/test/node/TestingDomain.js b/test/node/TestingDomain.js index 1000191d266..b51e61e40e6 100644 --- a/test/node/TestingDomain.js +++ b/test/node/TestingDomain.js @@ -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; \ No newline at end of file diff --git a/test/spec/ProjectManager-test.js b/test/spec/ProjectManager-test.js index 31d9ee6340a..83c488ba4fa 100644 --- a/test/spec/ProjectManager-test.js +++ b/test/spec/ProjectManager-test.js @@ -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; @@ -121,60 +125,6 @@ define(function (require, exports, module) { }); }); - // 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, - found = false, - entry; - - runs(function () { - waitsForDone(SpecRunnerUtils.copy(tempDir + "/git/", tempDir + "/.git/"), "copy git to .git"); - }); - - 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); - - // unfortunately, this wait has to happen for the fs event to propagate to - // the project model - waits(500); - - runs(function () { - found = testWindow.$(".jstree-brackets span:contains(\".git\")").length; - expect(found).toBe(0); - }); - - }); - it("should fail when a file already exists", function () { var didCreate = false, gotError = false; @@ -297,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 () { diff --git a/test/spec/SpecRunnerUtils.js b/test/spec/SpecRunnerUtils.js index ac3b4922aa1..42c4d40e3d5 100644 --- a/test/spec/SpecRunnerUtils.js +++ b/test/spec/SpecRunnerUtils.js @@ -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 @@ -1337,6 +1345,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; From 97e684943436a167326f44a19646ece6804ea168 Mon Sep 17 00:00:00 2001 From: Arzhan Kinzhalin Date: Wed, 14 Jan 2015 03:03:37 -0300 Subject: [PATCH 10/10] Check if the parent entry is watched. Do not fire synthetic events if the directory is not watched. --- src/filesystem/Directory.js | 4 +++- src/filesystem/File.js | 9 ++++++--- src/filesystem/FileSystem.js | 12 +++++++----- src/filesystem/FileSystemEntry.js | 27 ++++++++++++++++++++------- 4 files changed, 36 insertions(+), 16 deletions(-) diff --git a/src/filesystem/Directory.js b/src/filesystem/Directory.js index e9ef881f1c1..78915b77d9b 100644 --- a/src/filesystem/Directory.js +++ b/src/filesystem/Directory.js @@ -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(); } diff --git a/src/filesystem/File.js b/src/filesystem/File.js index 407ac94cd42..db1ef9cfb9b 100644 --- a/src/filesystem/File.js +++ b/src/filesystem/File.js @@ -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(); } diff --git a/src/filesystem/FileSystem.js b/src/filesystem/FileSystem.js index db31605800e..72c4d325680 100644 --- a/src/filesystem/FileSystem.js +++ b/src/filesystem/FileSystem.js @@ -797,7 +797,7 @@ define(function (require, exports, module) { path = this._normalizePath(path, false); var entry = this._index.getEntry(path); - if (entry && entry._isWatched()) { + if (entry) { var oldStat = entry._stat; if (entry.isFile) { // Update stat and clear contents, but only if out of date @@ -810,10 +810,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)); } } diff --git a/src/filesystem/FileSystemEntry.js b/src/filesystem/FileSystemEntry.js index 73e9f293c62..b53ed3c3615 100644 --- a/src/filesystem/FileSystemEntry.js +++ b/src/filesystem/FileSystemEntry.js @@ -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. @@ -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; } } @@ -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(); @@ -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();