From d9b5e17c9cc5cf125561550ff8054818d3a73201 Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 10 Aug 2015 01:55:55 -0400 Subject: [PATCH] Asyncify Zotero.Attachments.getNumFiles() and add hasMultipleFiles() Latter is probably all that's needed --- chrome/content/zotero/xpcom/attachments.js | 100 +++++++++++++----- .../xpcom/translation/translate_item.js | 10 +- test/tests/attachmentsTest.js | 45 ++++++++ 3 files changed, 127 insertions(+), 28 deletions(-) diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js index 356dcc794a..049fa2509a 100644 --- a/chrome/content/zotero/xpcom/attachments.js +++ b/chrome/content/zotero/xpcom/attachments.js @@ -1001,18 +1001,9 @@ Zotero.Attachments = new function(){ } - /** - * Returns the number of files in the attachment directory - * - * Only counts if MIME type is text/html - * - * @param {Zotero.Item} item Attachment item - */ - this.getNumFiles = function (item) { - var funcName = "Zotero.Attachments.getNumFiles()"; - + this.hasMultipleFiles = Zotero.Promise.coroutine(function* (item) { if (!item.isAttachment()) { - throw ("Item is not an attachment in " + funcName); + throw new Error("Item is not an attachment"); } var linkMode = item.attachmentLinkMode; @@ -1022,31 +1013,90 @@ Zotero.Attachments = new function(){ break; default: - throw ("Invalid attachment link mode in " + funcName); + throw new Error("Invalid attachment link mode"); + } + + if (item.attachmentContentType != 'text/html') { + return false; + } + + var path = yield item.getFilePathAsync(); + if (!path) { + throw new Error("File not found"); + } + + var numFiles = 0; + var parent = OS.Path.dirname(path); + var iterator = new OS.File.DirectoryIterator(parent); + try { + while (true) { + let entry = yield iterator.next(); + if (entry.name.startsWith('.')) { + continue; + } + numFiles++; + if (numFiles > 1) { + break; + } + } + } + catch (e) { + iterator.close(); + if (e != StopIteration) { + throw e; + } + } + return numFiles > 1; + }); + + + /** + * Returns the number of files in the attachment directory + * + * Only counts if MIME type is text/html + * + * @param {Zotero.Item} item Attachment item + */ + this.getNumFiles = Zotero.Promise.coroutine(function* (item) { + if (!item.isAttachment()) { + throw new Error("Item is not an attachment"); + } + + var linkMode = item.attachmentLinkMode; + switch (linkMode) { + case Zotero.Attachments.LINK_MODE_IMPORTED_URL: + case Zotero.Attachments.LINK_MODE_IMPORTED_FILE: + break; + + default: + throw new Error("Invalid attachment link mode"); } if (item.attachmentContentType != 'text/html') { return 1; } - var file = item.getFile(); - if (!file) { - throw ("File not found in " + funcName); + var path = yield item.getFilePathAsync(); + if (!path) { + throw new Error("File not found"); } var numFiles = 0; - var parentDir = file.parent; - var files = parentDir.directoryEntries; - while (files.hasMoreElements()) { - file = files.getNext(); - file.QueryInterface(Components.interfaces.nsIFile); - if (file.leafName.indexOf('.') == 0) { - continue; - } - numFiles++; + var parent = OS.Path.dirname(path); + var iterator = new OS.File.DirectoryIterator(parent); + try { + yield iterator.forEach(function (entry) { + if (entry.name.startsWith('.')) { + return; + } + numFiles++; + }) + } + finally { + iterator.close(); } return numFiles; - } + }); /** diff --git a/chrome/content/zotero/xpcom/translation/translate_item.js b/chrome/content/zotero/xpcom/translation/translate_item.js index 76100b0d53..66601ed965 100644 --- a/chrome/content/zotero/xpcom/translation/translate_item.js +++ b/chrome/content/zotero/xpcom/translation/translate_item.js @@ -799,10 +799,14 @@ Zotero.Translate.ItemGetter.prototype = { var directory = targetFile.parent; // The only attachments that can have multiple supporting files are imported - // attachments of mime type text/html (specified in Attachments.getNumFiles()) + // attachments of mime type text/html + // + // TEMP: This used to check getNumFiles() here, but that's now async. + // It could be restored (using hasMultipleFiles()) when this is made + // async, but it's probably not necessary. (The below can also be changed + // to use OS.File.DirectoryIterator.) if(attachment.attachmentContentType == "text/html" - && linkMode != Zotero.Attachments.LINK_MODE_LINKED_FILE - && Zotero.Attachments.getNumFiles(attachment) > 1) { + && linkMode != Zotero.Attachments.LINK_MODE_LINKED_FILE) { // Attachment is a snapshot with supporting files. Check if any of the // supporting files would cause a name conflict, and build a list of transfers // that should be performed diff --git a/test/tests/attachmentsTest.js b/test/tests/attachmentsTest.js index 54d3c9304d..c20cb2d791 100644 --- a/test/tests/attachmentsTest.js +++ b/test/tests/attachmentsTest.js @@ -125,4 +125,49 @@ describe("Zotero.Attachments", function() { assert.equal((yield Zotero.Attachments.getTotalFileSize(item)), file.fileSize); }) }) + + describe("#hasMultipleFiles and #getNumFiles()", function () { + it("should return false and 1 for a single file", function* () { + var file = getTestDataDirectory(); + file.append('test.png'); + + // Create attachment and compare content + var item = yield Zotero.Attachments.importFromFile({ + file: file + }); + + assert.isFalse(yield Zotero.Attachments.hasMultipleFiles(item)); + assert.equal((yield Zotero.Attachments.getNumFiles(item)), 1); + }) + + it("should return false and 1 for single HTML file with hidden file", function* () { + var file = getTestDataDirectory(); + file.append('test.html'); + + // Create attachment and compare content + var item = yield Zotero.Attachments.importFromFile({ + file: file + }); + var path = OS.Path.join(OS.Path.dirname(item.getFilePath()), '.zotero-ft-cache'); + yield Zotero.File.putContentsAsync(path, ""); + + assert.isFalse(yield Zotero.Attachments.hasMultipleFiles(item)); + assert.equal((yield Zotero.Attachments.getNumFiles(item)), 1); + }) + + it("should return true and 2 for multiple files", function* () { + var file = getTestDataDirectory(); + file.append('test.html'); + + // Create attachment and compare content + var item = yield Zotero.Attachments.importFromFile({ + file: file + }); + var path = OS.Path.join(OS.Path.dirname(item.getFilePath()), 'test.png'); + yield Zotero.File.putContentsAsync(path, ""); + + assert.isTrue(yield Zotero.Attachments.hasMultipleFiles(item)); + assert.equal((yield Zotero.Attachments.getNumFiles(item)), 2); + }) + }) })