Asyncify Zotero.Attachments.getNumFiles() and add hasMultipleFiles()

Latter is probably all that's needed

chrome/content/zotero/xpcom/attachments.js

@@ -1001,18 +1001,9 @@ Zotero.Attachments = new function(){
 	}
 	
 	
-	/**
+	this.hasMultipleFiles = Zotero.Promise.coroutine(function* (item) {
-	 * 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()";
-		
 		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();
+		var path = yield item.getFilePathAsync();
-		if (!file) {
+		if (!path) {
-			throw ("File not found in " + funcName);
+			throw new Error("File not found");
 		}
 		
 		var numFiles = 0;
-		var parentDir = file.parent;
+		var parent = OS.Path.dirname(path);
-		var files = parentDir.directoryEntries;
+		var iterator = new OS.File.DirectoryIterator(parent);
-		while (files.hasMoreElements()) {
+		try {
-			file = files.getNext();
+			yield iterator.forEach(function (entry) {
-			file.QueryInterface(Components.interfaces.nsIFile);
+				if (entry.name.startsWith('.')) {
-			if (file.leafName.indexOf('.') == 0) {
+					return;
-				continue;
+				}
-			}
+				numFiles++;
-			numFiles++;
+			})
+		}
+		finally {
+			iterator.close();
 		}
 		return numFiles;
-	}
+	});
 	
 	
 	/**

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
+								&& linkMode != Zotero.Attachments.LINK_MODE_LINKED_FILE) {
-								&& Zotero.Attachments.getNumFiles(attachment) > 1) {
 							// 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

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);
+		})
+	})
 })