Update linked attachment base directory code

- Replace nsIFile and persistent descriptors with OS.File and string paths
- Add tests for base dir settings

chrome/content/zotero/preferences/preferences_advanced.js

@@ -323,69 +323,77 @@ Zotero_Preferences.Advanced = {
 
 
 Zotero_Preferences.Attachment_Base_Directory = {
-	choosePath: function () {
+	getPath: function () {
-		// Get existing base directory
+		var oldPath = Zotero.Prefs.get('baseAttachmentPath');
-		var oldBasePath = Zotero.Prefs.get('baseAttachmentPath');
+		if (oldPath) {
-		if (oldBasePath) {
-			var oldBasePathFile = Components.classes["@mozilla.org/file/local;1"]
-				.createInstance(Components.interfaces.nsILocalFile);
 			try {
-				oldBasePathFile.persistentDescriptor = oldBasePath;
+				return OS.Path.normalize(oldPath);
 			}
 			catch (e) {
-				Zotero.debug(e, 1);
+				Zotero.logError(e);
-				Components.utils.reportError(e);
+				return false;
-				oldBasePathFile = null;
 			}
 		}
+	},
+	
+	
+	choosePath: Zotero.Promise.coroutine(function* () {
+		var oldPath = this.getPath();
 		
 		//Prompt user to choose new base path
+		if (oldPath) {
+			var oldPathFile = Zotero.File.pathToFile(oldPath);
+		}
 		var nsIFilePicker = Components.interfaces.nsIFilePicker;
 		var fp = Components.classes["@mozilla.org/filepicker;1"]
 					.createInstance(nsIFilePicker);
-		if (oldBasePathFile) {
+		if (oldPathFile) {
-			fp.displayDirectory = oldBasePathFile;
+			fp.displayDirectory = oldPathFile;
 		}
 		fp.init(window, Zotero.getString('attachmentBasePath.selectDir'), nsIFilePicker.modeGetFolder);
 		fp.appendFilters(nsIFilePicker.filterAll);
 		if (fp.show() != nsIFilePicker.returnOK) {
 			return false;
 		}
-		var newBasePathFile = fp.file;
+		var newPath = OS.Path.normalize(fp.file.path);
 		
-		if (oldBasePathFile && oldBasePathFile.equals(newBasePathFile)) {
+		if (oldPath && oldPath == newPath) {
 			Zotero.debug("Base directory hasn't changed");
 			return false;
 		}
 		
+		return changePath(newPath);
+	}),
+	
+	
+	changePath: Zotero.Promise.coroutine(function* (basePath) {
 		// Find all current attachments with relative attachment paths
 		var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=? AND path LIKE '"
 			+ Zotero.Attachments.BASE_PATH_PLACEHOLDER  + "%'";
 		var params = [Zotero.Attachments.LINK_MODE_LINKED_FILE];
-		var oldRelativeAttachmentIDs = Zotero.DB.columnQuery(sql, params) || [];
+		var oldRelativeAttachmentIDs = yield Zotero.DB.columnQueryAsync(sql, params);
 		
 		//Find all attachments on the new base path
 		var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=?";
 		var params = [Zotero.Attachments.LINK_MODE_LINKED_FILE];
-		var allAttachments = Zotero.DB.columnQuery(sql,params);
+		var allAttachments = yield Zotero.DB.columnQueryAsync(sql, params);
 		var newAttachmentPaths = {};
 		var numNewAttachments = 0;
 		var numOldAttachments = 0;
-		var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
-			.createInstance(Components.interfaces.nsILocalFile);
 		for (let i=0; i<allAttachments.length; i++) {
 			let attachmentID = allAttachments[i];
+			let attachmentPath;
 			let relPath = false
 			
 			try {
-				let attachment = Zotero.Items.get(attachmentID);
+				let attachment = yield Zotero.Items.getAsync(attachmentID);
 				// This will return FALSE for relative paths if base directory
 				// isn't currently set
-				attachmentFile = attachment.getFile(false, true);
+				attachmentPath = attachment.getFilePath();
 				// Get existing relative path
-				let path = attachment.attachmentPath;
+				let storedPath = attachment.attachmentPath;
-				if (path.indexOf(Zotero.Attachments.BASE_PATH_PLACEHOLDER) == 0) {
+				if (storedPath.startsWith(Zotero.Attachments.BASE_PATH_PLACEHOLDER)) {
-					relPath = path.substr(Zotero.Attachments.BASE_PATH_PLACEHOLDER.length);
+					relPath = storedPath.substr(Zotero.Attachments.BASE_PATH_PLACEHOLDER.length);
 				}
 			}
 			catch (e) {
@@ -397,10 +405,7 @@ Zotero_Preferences.Attachment_Base_Directory = {
 			// If a file with the same relative path exists within the new base directory,
 			// don't touch the attachment, since it will continue to work
 			if (relPath) {
-				let relFile = Components.classes["@mozilla.org/file/local;1"]
+				if (yield OS.File.exists(OS.Path.join(basePath, relPath))) {
-					.createInstance(Components.interfaces.nsILocalFile);
-				relFile.setRelativeDescriptor(newBasePathFile, relPath);
-				if (relFile.exists()) {
 					numNewAttachments++;
 					continue;
 				}
@@ -409,15 +414,14 @@ Zotero_Preferences.Attachment_Base_Directory = {
 			// Files within the new base directory need to be updated to use
 			// relative paths (or, if the new base directory is an ancestor or
 			// descendant of the old one, new relative paths)
-			if (attachmentFile && Zotero.File.directoryContains(newBasePathFile, attachmentFile)) {
+			if (attachmentPath && Zotero.File.directoryContains(basePath, attachmentPath)) {
-				newAttachmentPaths[attachmentID] = relPath
+				newAttachmentPaths[attachmentID] = relPath ? attachmentPath : null;
-					? attachmentFile.persistentDescriptor : null;
 				numNewAttachments++;
 			}
 			// Existing relative attachments not within the new base directory
 			// will be converted to absolute paths
-			else if (relPath && oldBasePathFile) {
+			else if (relPath && this.getPath()) {
-				newAttachmentPaths[attachmentID] = attachmentFile.persistentDescriptor;
+				newAttachmentPaths[attachmentID] = attachmentPath;
 				numOldAttachments++;
 			}
 		}
@@ -475,50 +479,41 @@ Zotero_Preferences.Attachment_Base_Directory = {
 		
 		// Set new data directory
 		Zotero.debug("Setting new base directory");
-		Zotero.Prefs.set('baseAttachmentPath', newBasePathFile.persistentDescriptor);
+		Zotero.Prefs.set('baseAttachmentPath', basePath);
 		Zotero.Prefs.set('saveRelativeAttachmentPath', true);
 		// Resave all attachments on base path (so that their paths become relative)
 		// and all other relative attachments (so that their paths become absolute)
-		for (let id in newAttachmentPaths) {
+		yield Zotero.Utilities.Internal.forEachChunkAsync(
-			let attachment = Zotero.Items.get(id);
+			Object.keys(newAttachmentPaths),
-			if (newAttachmentPaths[id]) {
+			100,
-				attachment.attachmentPath = newAttachmentPaths[id];
+			function (chunk) {
-				attachment.save({
+				return Zotero.DB.executeTransaction(function* () {
-					skipDateModifiedUpdate: true
+					for (let id of chunk) {
-				});
+						let attachment = Zotero.Items.get(id);
+						if (newAttachmentPaths[id]) {
+							attachment.attachmentPath = newAttachmentPaths[id];
+						}
+						else {
+							attachment.attachmentPath = attachment.getFilePath();
+						}
+						yield attachment.save({
+							skipDateModifiedUpdate: true
+						});
+					}
+				})
 			}
-			else {
+		);
-				attachment.updateAttachmentPath();
-			}
-		}
-		return newBasePathFile.persistentDescriptor;
-	},
-	
-	
-	getPath: function (asFile) {
-		var desc = Zotero.Prefs.get('baseAttachmentPath');
-		if (desc == '') {
-			return asFile ? null : '';
-		}
 		
-		var file = Components.classes["@mozilla.org/file/local;1"]
+		return true;
-			.createInstance(Components.interfaces.nsILocalFile);
+	}),
-		try {
-			file.persistentDescriptor = desc;
-		}
-		catch (e) {
-			return asFile ? null : '';
-		}
-		return asFile ? file : file.path;
-	},
 	
 	
-	clearPath: function () {
+	clearPath: Zotero.Promise.coroutine(function* () {
 		// Find all current attachments with relative paths
 		var sql = "SELECT itemID FROM itemAttachments WHERE linkMode=? AND path LIKE '"
 			+ Zotero.Attachments.BASE_PATH_PLACEHOLDER  + "%'";
 		var params = [Zotero.Attachments.LINK_MODE_LINKED_FILE];
-		var relativeAttachmentIDs = Zotero.DB.columnQuery(sql, params) || [];
+		var relativeAttachmentIDs = yield Zotero.DB.columnQueryAsync(sql, params);
 		
 		// Prompt for confirmation
 		var ps = Components.classes["@mozilla.org/embedcomp/prompt-service;1"]
@@ -562,26 +557,40 @@ Zotero_Preferences.Attachment_Base_Directory = {
 		// attachments so that their absolute paths are stored
 		Zotero.debug('Clearing base directory');
 		Zotero.Prefs.set('saveRelativeAttachmentPath', false);
-		for (var i=0; i<relativeAttachmentIDs.length; i++) {
+		
-			Zotero.Items.get(relativeAttachmentIDs[i]).updateAttachmentPath();
+		yield Zotero.Utilities.Internal.forEachChunkAsync(
-		}
+			relativeAttachmentIDs,
+			100,
+			function (chunk) {
+				return Zotero.DB.executeTransaction(function* () {
+					for (let id of chunk) {
+						let attachment = yield Zotero.Items.getAsync(id);
+						attachment.attachmentPath = attachment.getFilePath();
+						yield attachment.save({
+							skipDateModifiedUpdate: true
+						});
+					}
+				}.bind(this));
+			}.bind(this)
+		);
+		
 		Zotero.Prefs.set('baseAttachmentPath', '');
-	},
+	}),
 	
 	
-	updateUI: function () {
+	updateUI: Zotero.Promise.coroutine(function* () {
 		var filefield = document.getElementById('baseAttachmentPath');
-		var file = this.getPath(true);
+		var path = Zotero.Prefs.get('baseAttachmentPath');
-		filefield.file = file;
+		Components.utils.import("resource://gre/modules/osfile.jsm");
-		if (file) {
+		if (yield OS.File.exists(path)) {
-			filefield.label = file.path;
+			filefield.file = Zotero.File.pathToFile(path);
+			filefield.label = path;
 		}
 		else {
 			filefield.label = '';
 		}
-		
+		document.getElementById('resetBasePath').disabled = !path;
-		document.getElementById('resetBasePath').disabled = !Zotero.Prefs.get('baseAttachmentPath');
+	})
-	}
 };
 
 

chrome/content/zotero/preferences/preferences_advanced.xul

@@ -154,7 +154,8 @@
 								readonly="true"
 								flex="1"
 								tabindex="-1"/>
-							<button label="&zotero.preferences.attachmentBaseDir.selectBasePath;"
+							<button id="baseAttachmentPathButton"
+								label="&zotero.preferences.attachmentBaseDir.selectBasePath;"
 								oncommand="Zotero_Preferences.Attachment_Base_Directory.choosePath()"/>
 						</hbox>
 						

test/tests/preferences_advancedTest.js

@@ -0,0 +1,129 @@
+describe("Advanced Preferences", function () {
+	describe("Files & Folders", function () {
+		describe("Linked Attachment Base Directory", function () {
+			var setBaseDirectory = Zotero.Promise.coroutine(function* (basePath) {
+				var win = yield loadWindow("chrome://zotero/content/preferences/preferences.xul", {
+					pane: 'zotero-prefpane-advanced',
+					tabIndex: 1
+				});
+				
+				// Wait for tab to load
+				var doc = win.document;
+				var prefwindow = doc.documentElement;
+				var defer = Zotero.Promise.defer();
+				var pane = doc.getElementById('zotero-prefpane-advanced');
+				if (!pane.loaded) {
+					pane.addEventListener('paneload', function () {
+						defer.resolve();
+					})
+					yield defer.promise;
+				}
+				
+				var promise = waitForDialog();
+				yield win.Zotero_Preferences.Attachment_Base_Directory.changePath(basePath);
+				yield promise;
+				
+				win.close();
+			});
+			
+			var clearBaseDirectory = Zotero.Promise.coroutine(function* (basePath) {
+				var win = yield loadWindow("chrome://zotero/content/preferences/preferences.xul", {
+					pane: 'zotero-prefpane-advanced',
+					tabIndex: 1
+				});
+				
+				// Wait for tab to load
+				var doc = win.document;
+				var prefwindow = doc.documentElement;
+				var defer = Zotero.Promise.defer();
+				var pane = doc.getElementById('zotero-prefpane-advanced');
+				if (!pane.loaded) {
+					pane.addEventListener('paneload', function () {
+						defer.resolve();
+					})
+					yield defer.promise;
+				}
+				
+				var promise = waitForDialog();
+				yield win.Zotero_Preferences.Attachment_Base_Directory.clearPath();
+				yield promise;
+				
+				win.close();
+			});
+			
+			beforeEach(function () {
+				Zotero.Prefs.clear('baseAttachmentPath');
+				Zotero.Prefs.clear('saveRelativeAttachmentPath');
+			});
+			
+			it("should set new base directory", function* () {
+				var basePath = getTestDataDirectory().path;
+				yield setBaseDirectory(basePath);
+				assert.equal(Zotero.Prefs.get('baseAttachmentPath'), basePath);
+				assert.isTrue(Zotero.Prefs.get('saveRelativeAttachmentPath'));
+			})
+			
+			it("should clear base directory", function* () {
+				var basePath = getTestDataDirectory().path;
+				yield setBaseDirectory(basePath);
+				yield clearBaseDirectory();
+				
+				assert.equal(Zotero.Prefs.get('baseAttachmentPath'), '');
+				assert.isFalse(Zotero.Prefs.get('saveRelativeAttachmentPath'));
+			})
+			
+			it("should change absolute path of linked attachment under new base dir to prefixed path", function* () {
+				var file = getTestDataDirectory();
+				file.append('test.png');
+				var attachment = yield Zotero.Attachments.linkFromFile({ file });
+				assert.equal(attachment.attachmentPath, file.path);
+				
+				var basePath = getTestDataDirectory().path;
+				yield setBaseDirectory(basePath);
+				
+				assert.equal(
+					attachment.attachmentPath,
+					Zotero.Attachments.BASE_PATH_PLACEHOLDER + 'test.png'
+				);
+			})
+			
+			it("should change prefixed path to absolute when changing base directory", function* () {
+				var basePath = getTestDataDirectory().path;
+				yield setBaseDirectory(basePath);
+				
+				var file = getTestDataDirectory();
+				file.append('test.png');
+				var attachment = yield Zotero.Attachments.linkFromFile({ file });
+				assert.equal(
+					attachment.attachmentPath,
+					Zotero.Attachments.BASE_PATH_PLACEHOLDER + 'test.png'
+				);
+				
+				var basePath = Zotero.getTempDirectory().path;
+				yield setBaseDirectory(basePath);
+				
+				assert.equal(attachment.attachmentPath, file.path);
+			})
+			
+			it("should change prefixed path to absolute when clearing base directory", function* () {
+				var basePath = getTestDataDirectory().path;
+				yield setBaseDirectory(basePath);
+				
+				var file = getTestDataDirectory();
+				file.append('test.png');
+				var attachment = yield Zotero.Attachments.linkFromFile({ file });
+				assert.equal(
+					attachment.attachmentPath,
+					Zotero.Attachments.BASE_PATH_PLACEHOLDER + 'test.png'
+				);
+				
+				yield clearBaseDirectory();
+				
+				assert.equal(Zotero.Prefs.get('baseAttachmentPath'), '');
+				assert.isFalse(Zotero.Prefs.get('saveRelativeAttachmentPath'));
+				
+				assert.equal(attachment.attachmentPath, file.path);
+			})
+		})
+	})
+})