Use progress dialog for PDF retrieval

chrome/content/zotero/xpcom/attachments.js

@@ -31,6 +31,9 @@ Zotero.Attachments = new function(){
 	this.LINK_MODE_LINKED_URL = 3;
 	this.BASE_PATH_PLACEHOLDER = 'attachments:';
 	
+	var _findPDFQueue = [];
+	var _findPDFQueuePromise = null;
+	
 	var self = this;
 	
 	
@@ -1108,25 +1111,15 @@ Zotero.Attachments = new function(){
 	 * @param {Zotero.Item[]} items
 	 * @param {Object} [options]
 	 * @param {String[]} [options.methods] - See getPDFResolvers()
-	 * @param {Function} [options.onProgress]
 	 * @param {Number} [options.sameDomainRequestDelay=1000] - Minimum number of milliseconds
 	 *     between requests to the same domain (used in tests)
+	 * @return {Promise}
 	 */
 	this.addAvailablePDFs = async function (items, options = {}) {
 		const MAX_CONSECUTIVE_DOMAIN_FAILURES = 5;
 		const SAME_DOMAIN_REQUEST_DELAY = options.sameDomainRequestDelay || 1000;
 		
 		var domains = new Map();
-		var queue = items.map((item) => {
-			return {
-				item,
-				urlResolvers: this.getPDFResolvers(item, options.methods),
-				domain: null,
-				continuation: null,
-				result: null,
-			};
-		});
-		
 		function getDomainInfo(domain) {
 			var domainInfo = domains.get(domain);
 			if (!domainInfo) {
@@ -1139,29 +1132,109 @@ Zotero.Attachments = new function(){
 			return domainInfo;
 		}
 		
-		var completed = 0;
+		var progressQueue = Zotero.ProgressQueues.get('findPDF');
-		var lastQueueStart = new Date();
+		if (!progressQueue) {
-		var i = 0;
+			progressQueue = Zotero.ProgressQueues.create({
+				id: 'findPDF',
+				title: 'findPDF.title',
+				columns: [
+					'general.item',
+					'general.pdf'
+				]
+			});
+		}
+		var queue = _findPDFQueue;
+		
+		for (let item of items) {
+			// Skip items that aren't eligible. This is sort of weird, because it means some
+			// selected items just don't appear in the list, but there are several different reasons
+			// why items might not be eligible (non-regular items, no URL or DOI, already has a PDF)
+			// and listing each one seems a little unnecessary.
+			if (!this.canFindPDFForItem(item)) {
+				continue;
+			}
+			
+			let entry = {
+				item,
+				urlResolvers: this.getPDFResolvers(item, options.methods),
+				domain: null,
+				continuation: null,
+				processing: false,
+				result: null
+			};
+			
+			let pos = queue.findIndex(x => x.item == item);
+			if (pos != -1) {
+				let current = queue[pos];
+				// Skip items that are already processing or that returned a result
+				if (current.processing || current.result) {
+					continue;
+				}
+				// Replace current queue entry
+				queue[pos] = entry;
+			}
+			else {
+				// Add new items to queue
+				queue.push(entry);
+			}
+			
+			progressQueue.addRow(item);
+		}
+		
+		// If no eligible items, just show a popup saying no PDFs were found
+		if (!queue.length) {
+			let icon = 'chrome://zotero/skin/treeitem-attachment-pdf.png';
+			let progressWin = new Zotero.ProgressWindow();
+			let title = Zotero.getString('findPDF.title');
+			progressWin.changeHeadline(title);
+			let itemProgress = new progressWin.ItemProgress(
+				icon,
+				Zotero.getString('findPDF.noPDFsFound')
+			);
+			progressWin.show();
+			itemProgress.setProgress(100);
+			itemProgress.setIcon(icon);
+			progressWin.startCloseTimer(4000);
+			return;
+		}
+		
+		var dialog = progressQueue.getDialog();
+		dialog.showMinimizeButton(false);
+		dialog.open();
+		
+		// If queue was already in progress, just wait for it to finish
+		if (_findPDFQueuePromise) {
+			return _findPDFQueuePromise;
+		}
+		
+		var queueResolve;
+		_findPDFQueuePromise = new Zotero.Promise((resolve) => {
+			queueResolve = resolve;
+		});
+		
+		// Only one listener can be added, so we just add each time
+		progressQueue.addListener('cancel', () => {
+			queue = [];
+		});
 		
 		//
 		// Process items in the queue
 		//
-		await new Promise((resolve) => {
+		var i = 0;
+		await new Zotero.Promise((resolve) => {
 			var processNextItem = function () {
-				// All items processed
-				if (completed == queue.length) {
-					resolve();
-					return;
-				}
-				
-				if (i == 0) {
-					lastQueueStart = new Date();
-				}
 				var current = queue[i++];
 				
-				// If we got to the end of the queue, wait until the next time a pending request
+				// We reached the end of the queue
-				// is ready to process
 				if (!current) {
+					// If all entries are resolved, we're done
+					if (queue.every(x => x instanceof Zotero.Item || x.result === false)) {
+						resolve();
+						return;
+					}
+					
+					// Otherwise, wait until the next time a pending request is ready to process
+					// and restart
 					let nextStart = queue
 						.map(x => x.result === null && getDomainInfo(x.domain).nextRequestTime)
 						.filter(x => x)
@@ -1192,15 +1265,19 @@ Zotero.Attachments = new function(){
 					return;
 				}
 				
-				if (!this.canFindPDFForItem(current.item)) {
+				// Currently filtered out above
+				/*if (!this.canFindPDFForItem(current.item)) {
 					current.result = false;
-					completed++;
+					progressQueue.updateRow(
-					if (options.onProgress) {
+						current.item.id,
-						options.onProgress(completed, queue.length);
+						Zotero.ProgressQueue.ROW_FAILED,
-					}
+						""
+					);
 					processNextItem();
 					return;
-				}
+				}*/
+				
+				current.processing = true;
 				
 				// Process item
 				this.addPDFFromURLs(
@@ -1220,7 +1297,6 @@ Zotero.Attachments = new function(){
 							
 							// If too many requests have failed, stop trying
 							if (domainInfo.consecutiveFailures > MAX_CONSECUTIVE_DOMAIN_FAILURES) {
-								current.result = false;
 								throw new Error(`Too many failed requests for ${urlToDomain(url)}`);
 							}
 							
@@ -1259,6 +1335,7 @@ Zotero.Attachments = new function(){
 							domainInfo.consecutiveFailures = 0;
 						},
 						
+						// Return true to retry request or false to skip
 						onRequestError: function (e) {
 							const maxDelay = 3600;
 							
@@ -1301,29 +1378,35 @@ Zotero.Attachments = new function(){
 									domainInfo.nextRequestTime = Date.now() + 10000;
 									return true;
 								}
-								
-								current.result = false;
-							}
-							else {
-								current.result = false;
 							}
+							
+							return false;
 						}
 					}
 				)
 				.then((attachment) => {
 					current.result = attachment;
+					progressQueue.updateRow(
+						current.item.id,
+						Zotero.ProgressQueue.ROW_FAILED,
+						attachment
+							? attachment.getField('title')
+							: Zotero.getString('findPDF.noPDFFound')
+					);
 				})
 				.catch((e) => {
 					Zotero.logError(e);
 					current.result = false;
+					progressQueue.updateRow(
+						current.item.id,
+						Zotero.ProgressQueue.ROW_FAILED,
+						Zotero.getString('general.failed')
+					);
 				})
 				// finally() isn't implemented until Firefox 58, but then() is the same here
 				//.finally(() => {
 				.then(function () {
-					completed++;
+					current.processing = false;
-					if (options.onProgress) {
-						options.onProgress(completed, queue.length);
-					}
 					processNextItem();
 				});
 			}.bind(this);
@@ -1331,7 +1414,17 @@ Zotero.Attachments = new function(){
 			processNextItem();
 		});
 		
-		return queue.map(x => x.result);
+		var numPDFs = queue.reduce((accumulator, currentValue) => {
+			return accumulator + (currentValue.result ? 1 : 0);
+		}, 0);
+		dialog.setStatus(
+			numPDFs
+				? Zotero.getString('findPDF.pdfsAdded', numPDFs, numPDFs)
+				: Zotero.getString('findPDF.noPDFsFound')
+		);
+		_findPDFQueue = [];
+		queueResolve();
+		_findPDFQueuePromise = null;
 	};
 	
 	

chrome/content/zotero/zoteroPane.js

@@ -3708,40 +3708,7 @@ var ZoteroPane = new function()
 			this.displayCannotEditLibraryMessage();
 			return;
 		}
-		
+		await Zotero.Attachments.addAvailablePDFs(this.getSelectedItems());
-		var items = this.getSelectedItems();
-		
-		var icon = 'chrome://zotero/skin/treeitem-attachment-pdf.png';
-		var progressWin = new Zotero.ProgressWindow();
-		var title = Zotero.getString('findPDF.searchingForAvailablePDFs');
-		progressWin.changeHeadline(title);
-		var itemProgress = new progressWin.ItemProgress(
-			icon,
-			Zotero.getString('findPDF.checkingItems', items.length, items.length)
-		);
-		progressWin.show();
-		
-		var results = await Zotero.Attachments.addAvailablePDFs(
-			items,
-			{
-				onProgress: (progress, progressMax) => {
-					itemProgress.setProgress((progress / progressMax) * 100);
-				}
-			}
-		);
-		
-		itemProgress.setProgress(100);
-		itemProgress.setIcon(icon);
-		
-		var successful = results.filter(x => x).length;
-		if (successful) {
-			itemProgress.setText(Zotero.getString('findPDF.pdfsAdded', successful, successful));
-		}
-		else {
-			itemProgress.setText(Zotero.getString('findPDF.noPDFsFound'))
-		}
-		
-		progressWin.startCloseTimer(4000);
 	};
 	
 	

chrome/locale/en-US/zotero/zotero.properties

@@ -71,6 +71,8 @@ general.submitted      = Submitted
 general.thanksForHelpingImprove = Thanks for helping to improve %S!
 general.describeProblem			= Briefly describe the problem:
 general.nMegabytes                 = %S MB
+general.item                       = Item
+general.pdf                        = PDF
 
 general.operationInProgress = A Zotero operation is currently in progress.
 general.operationInProgress.waitUntilFinished = Please wait until it has finished.
@@ -595,12 +597,14 @@ ingester.importFile.intoNewCollection = Import into new collection
 ingester.lookup.performing		= Performing Lookup…
 ingester.lookup.error			= An error occurred while performing lookup for this item.
 
+findPDF.title = Find Available PDFs
 findPDF.searchingForAvailablePDFs = Searching for available PDFs…
 findPDF.checkingItems = Checking %1$S item;Checking %1$S items
 findPDF.pdfsAdded = %1$S PDF added;%1$S PDFs added
 findPDF.openAccessPDF = Open-Access PDF
 findPDF.pdfWithMethod = PDF (%S)
 findPDF.noPDFsFound = No PDFs found
+findPDF.noPDFFound = No PDF found
 
 attachment.fullText = Full Text
 attachment.acceptedVersion = Accepted Version