From 41c9a4a433b84cf68669de88d4df7effb2d7a5b7 Mon Sep 17 00:00:00 2001
From: Dan Stillman <dstillman@zotero.org>
Date: Fri, 19 Apr 2013 01:18:59 -0400
Subject: [PATCH] Fix relative path support to sync properly

Dual boot and VM setups worked, but only absolute paths were synced,
because syncing uses attachmentPath, which was resolving relative paths
itself (#51). attachmentPath now returns what's in the database (as it
did originally), and getFile() does the resolving instead.

This means that anything relying on attachmentPath directly needs to be
aware that it might get a placeholder-prefixed relative path.
---
 .../preferences/preferences_advanced.js       |  28 ++-
 chrome/content/zotero/xpcom/attachments.js    |  98 ++++++++++
 chrome/content/zotero/xpcom/data/item.js      | 175 ++++++------------
 chrome/content/zotero/xpcom/sync.js           |   4 +-
 chrome/content/zotero/zoteroPane.js           |   3 +-
 5 files changed, 178 insertions(+), 130 deletions(-)

diff --git a/chrome/content/zotero/preferences/preferences_advanced.js b/chrome/content/zotero/preferences/preferences_advanced.js
index c185a358e..3fb5489de 100644
--- a/chrome/content/zotero/preferences/preferences_advanced.js
+++ b/chrome/content/zotero/preferences/preferences_advanced.js
@@ -364,22 +364,30 @@ Zotero_Preferences.Attachment_Base_Directory = {
 			.createInstance(Components.interfaces.nsILocalFile);
 		for (let i=0; i<allAttachments.length; i++) {
 			let attachmentID = allAttachments[i];
+			let relPath = false
 			
 			try {
 				let attachment = Zotero.Items.get(attachmentID);
-				attachmentFile.persistentDescriptor = attachment.attachmentPath;
+				// This will return FALSE for relative paths if base directory
+				// isn't currently set
+				attachmentFile = attachment.getFile(false, true);
+				// Get existing relative path
+				let path = attachment.attachmentPath;
+				if (path.indexOf(Zotero.Attachments.BASE_PATH_PLACEHOLDER) == 0) {
+					relPath = path.substr(Zotero.Attachments.BASE_PATH_PLACEHOLDER.length);
+				}
 			}
 			catch (e) {
 				// Don't deal with bad attachment paths. Just skip them.
+				Zotero.debug(e, 2);
 				continue;
 			}
 			
 			// 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
-			let isExistingRelativeAttachment = oldRelativeAttachmentIDs.indexOf(attachmentID) != -1;
-			if (isExistingRelativeAttachment && oldBasePathFile) {
-				let relFile = attachmentFile.clone();
-				let relPath = attachmentFile.getRelativeDescriptor(oldBasePathFile);
+			if (relPath) {
+				let relFile = Components.classes["@mozilla.org/file/local;1"]
+					.createInstance(Components.interfaces.nsILocalFile);
 				relFile.setRelativeDescriptor(newBasePathFile, relPath);
 				if (relFile.exists()) {
 					numNewAttachments++;
@@ -390,14 +398,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 (Zotero.File.directoryContains(newBasePathFile, attachmentFile)) {
-				newAttachmentPaths[attachmentID] = isExistingRelativeAttachment
+			if (attachmentFile && Zotero.File.directoryContains(newBasePathFile, attachmentFile)) {
+				newAttachmentPaths[attachmentID] = relPath
 					? attachmentFile.persistentDescriptor : null;
 				numNewAttachments++;
 			}
 			// Existing relative attachments not within the new base directory
 			// will be converted to absolute paths
-			else if (isExistingRelativeAttachment && oldBasePathFile) {
+			else if (relPath && oldBasePathFile) {
 				newAttachmentPaths[attachmentID] = attachmentFile.persistentDescriptor;
 				numOldAttachments++;
 			}
@@ -464,7 +472,9 @@ Zotero_Preferences.Attachment_Base_Directory = {
 			let attachment = Zotero.Items.get(id);
 			if (newAttachmentPaths[id]) {
 				attachment.attachmentPath = newAttachmentPaths[id];
-				attachment.save();
+				attachment.save({
+					skipDateModifiedUpdate: true
+				});
 			}
 			else {
 				attachment.updateAttachmentPath();
diff --git a/chrome/content/zotero/xpcom/attachments.js b/chrome/content/zotero/xpcom/attachments.js
index 5db680f07..113e2de61 100644
--- a/chrome/content/zotero/xpcom/attachments.js
+++ b/chrome/content/zotero/xpcom/attachments.js
@@ -1013,6 +1013,104 @@ Zotero.Attachments = new function(){
 	}
 	
 	
+	/**
+	 * If file is within the attachment base directory, return a relative
+	 * path prefixed by BASE_PATH_PLACEHOLDER. Otherwise, return unchanged.
+	 */
+	this.getBaseDirectoryRelativePath = function (path) {
+		if (!path || path.indexOf(this.BASE_PATH_PLACEHOLDER) == 0) {
+			return path;
+		}
+		
+		var basePath = Zotero.Prefs.get('baseAttachmentPath');
+		if (!basePath) {
+			return path;
+		}
+		
+		// Get nsIFile for base directory
+		var baseDir = Components.classes["@mozilla.org/file/local;1"]
+			.createInstance(Components.interfaces.nsILocalFile);
+		try {
+			baseDir.persistentDescriptor = basePath;
+		}
+		catch (e) {
+			Zotero.debug(e, 1);
+			Components.utils.reportError(e);
+			return path;
+		}
+		
+		// Get nsIFile for file
+		var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
+			.createInstance(Components.interfaces.nsILocalFile);
+		try {
+			attachmentFile.persistentDescriptor = path;
+		}
+		catch (e) {
+			Zotero.debug(e, 1);
+			Components.utils.reportError(e);
+			return path;
+		}
+		
+		if (Zotero.File.directoryContains(baseDir, attachmentFile)) {
+			path = this.BASE_PATH_PLACEHOLDER
+				+ attachmentFile.getRelativeDescriptor(baseDir);
+		}
+		
+		return path;
+	}
+	
+	
+	/**
+	 * Get a file from this path, if we can
+	 *
+	 * @param {String} path  Absolute path or relative path prefixed
+	 *                       by BASE_PATH_PLACEHOLDER
+	 * @param {Boolean} asFile Return nsIFile instead of path
+	 * @return {String|nsIFile|FALSE} Persistent descriptor string, file,
+	 *                                of FALSE if no path
+	 */
+	this.resolveRelativePath = function (path) {
+		if (path.indexOf(Zotero.Attachments.BASE_PATH_PLACEHOLDER) != 0) {
+			return false;
+		}
+		
+		var basePath = Zotero.Prefs.get('baseAttachmentPath');
+		if (!basePath) {
+			Zotero.debug("No base attachment path set -- can't resolve '" + path + "'", 2);
+			return false;
+		}
+		
+		// Get file from base directory
+		var baseDir = Components.classes["@mozilla.org/file/local;1"]
+			.createInstance(Components.interfaces.nsILocalFile);
+		try {
+			baseDir.persistentDescriptor = basePath;
+		}
+		catch (e) {
+			Zotero.debug(e, 1);
+			Components.utils.reportError(e);
+			Zotero.debug("Invalid base attachment path -- can't resolve'" + row.path + "'", 2);
+			return false;
+		}
+		
+		// Get file from relative path
+		var relativePath = path.substr(
+			Zotero.Attachments.BASE_PATH_PLACEHOLDER.length
+		);
+		var file = Components.classes["@mozilla.org/file/local;1"]
+			.createInstance(Components.interfaces.nsILocalFile);
+		try {
+			file.setRelativeDescriptor(baseDir, relativePath);
+		}
+		catch (e) {
+			Zotero.debug("Invalid relative descriptor '" + relativePath + "'", 2);
+			return false;
+		}
+		
+		return file;
+	}
+	
+	
 	/**
 	 * Returns the number of files in the attachment directory
 	 *
diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js
index 796be2e6f..243feaa5c 100644
--- a/chrome/content/zotero/xpcom/data/item.js
+++ b/chrome/content/zotero/xpcom/data/item.js
@@ -76,7 +76,6 @@ Zotero.Item.prototype._init = function () {
 	this._changedSource = false;
 	this._changedAttachmentData = false;
 	
-	this._skipModTimeUpdate = false;
 	this._previousData = {};
 	
 	this._bestAttachmentState = null;
@@ -1251,7 +1250,11 @@ Zotero.Item.prototype.removeRelatedItem = function (itemID) {
  *
  * Returns true on item update or itemID of new item
  */
-Zotero.Item.prototype.save = function() {
+Zotero.Item.prototype.save = function(options) {
+	if (!options) {
+		options = {};
+	}
+	
 	Zotero.Items.editCheck(this);
 	
 	if (!this.hasChanged()) {
@@ -1527,37 +1530,16 @@ Zotero.Item.prototype.save = function() {
 				var path = this.attachmentPath;
 				var syncState = this.attachmentSyncState;
 				
-				// Save attachment within attachment base directory as relative path
-				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE && path) {
-					let basePath = Zotero.Prefs.get('baseAttachmentPath');
-					if (basePath != '' && Zotero.Prefs.get('saveRelativeAttachmentPath')) {
-						let baseDir = Components.classes["@mozilla.org/file/local;1"]
-							.createInstance(Components.interfaces.nsILocalFile);
-						try {
-							baseDir.persistentDescriptor = basePath;
-						}
-						catch (e) {
-							Zotero.debug(e, 1);
-							Components.utils.reportError(e);
-							baseDir = null;
-						}
-						
-						if (baseDir) {
-							let attachmentFile = Components.classes["@mozilla.org/file/local;1"]
-								.createInstance(Components.interfaces.nsILocalFile);
-							try {
-								attachmentFile.persistentDescriptor = path;
-							}
-							catch (e) {
-								Zotero.debug(e, 1);
-								Components.utils.reportError(e);
-								attachmentFile = null;
-							}
-							
-							if (attachmentFile && Zotero.File.directoryContains(baseDir, attachmentFile)) {
-								path = Zotero.Attachments.BASE_PATH_PLACEHOLDER
-									+ attachmentFile.getRelativeDescriptor(baseDir);
-							}
+				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) {
+					// Save attachment within attachment base directory as relative path
+					if (Zotero.Prefs.get('saveRelativeAttachmentPath')) {
+						path = Zotero.Attachments.getBaseDirectoryRelativePath(path);
+					}
+					// If possible, convert relative path to absolute
+					else {
+						let file = Zotero.Attachments.resolveRelativePath(path);
+						if (file) {
+							path = file.persistentDescriptor;
 						}
 					}
 				}
@@ -1669,8 +1651,11 @@ Zotero.Item.prototype.save = function() {
 					sql += field + '=?, ';
 					sqlValues.push(this.getField(field));
 				}
-				else if ((field == 'dateModified' || field == 'clientDateModified')
-							&& !this._skipModTimeUpdate) {
+				else if (field == 'dateModified' && !options.skipDateModifiedUpdate) {
+					sql += field + '=?, ';
+					sqlValues.push(Zotero.DB.transactionDateTime);
+				}
+				else if (field == 'clientDateModified' && !options.skipClientDateModifiedUpdate) {
 					sql += field + '=?, ';
 					sqlValues.push(Zotero.DB.transactionDateTime);
 				}
@@ -1962,37 +1947,16 @@ Zotero.Item.prototype.save = function() {
 				var path = this.attachmentPath;
 				var syncState = this.attachmentSyncState;
 				
-				// Save attachment within attachment base directory as relative path
-				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE && path) {
-					let basePath = Zotero.Prefs.get('baseAttachmentPath');
-					if (basePath != '' && Zotero.Prefs.get('saveRelativeAttachmentPath')) {
-						let baseDir = Components.classes["@mozilla.org/file/local;1"]
-							.createInstance(Components.interfaces.nsILocalFile);
-						try {
-							baseDir.persistentDescriptor = basePath;
-						}
-						catch (e) {
-							Zotero.debug(e, 1);
-							Components.utils.reportError(e);
-							baseDir = null;
-						}
-						
-						if (baseDir) {
-							let attachmentFile = Components.classes["@mozilla.org/file/local;1"]
-								.createInstance(Components.interfaces.nsILocalFile);
-							try {
-								attachmentFile.persistentDescriptor = path;
-							}
-							catch (e) {
-								Zotero.debug(e, 1);
-								Components.utils.reportError(e);
-								attachmentFile = null;
-							}
-							
-							if (attachmentFile && Zotero.File.directoryContains(baseDir, attachmentFile)) {
-								path = Zotero.Attachments.BASE_PATH_PLACEHOLDER
-									+ attachmentFile.getRelativeDescriptor(baseDir);
-							}
+				if (this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) {
+					// Save attachment within attachment base directory as relative path
+					if (Zotero.Prefs.get('saveRelativeAttachmentPath')) {
+						path = Zotero.Attachments.getBaseDirectoryRelativePath(path);
+					}
+					// If possible, convert relative path to absolute
+					else {
+						let file = Zotero.Attachments.resolveRelativePath(path);
+						if (file) {
+							path = file.persistentDescriptor;
 						}
 					}
 				}
@@ -2864,6 +2828,7 @@ Zotero.Item.prototype.getFile = function(row, skipExistsCheck) {
 		return false;
 	}
 	
+	// Imported file with relative path
 	if (row.linkMode == Zotero.Attachments.LINK_MODE_IMPORTED_URL ||
 			row.linkMode == Zotero.Attachments.LINK_MODE_IMPORTED_FILE) {
 		try {
@@ -2913,6 +2878,15 @@ Zotero.Item.prototype.getFile = function(row, skipExistsCheck) {
 			}
 		}
 	}
+	// Linked file with relative path
+	else if (row.linkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE &&
+			row.path.indexOf(Zotero.Attachments.BASE_PATH_PLACEHOLDER) == 0) {
+		var file = Zotero.Attachments.resolveRelativePath(row.path);
+		if (!file) {
+			updateAttachmentStates(false);
+			return false;
+		}
+	}
 	else {
 		var file = Components.classes["@mozilla.org/file/local;1"].
 			createInstance(Components.interfaces.nsILocalFile);
@@ -3073,11 +3047,10 @@ Zotero.Item.prototype.relinkAttachmentFile = function(file, skipItemUpdate) {
 	var path = Zotero.Attachments.getPath(file, linkMode);
 	this.attachmentPath = path;
 	
-	if (skipItemUpdate) {
-		this._skipModTimeUpdate = true;
-	}
-	this.save();
-	this._skipModTimeUpdate = false;
+	this.save({
+		skipDateModifiedUpdate: true,
+		skipClientDateModifiedUpdate: skipItemUpdate
+	});
 	
 	return false;
 }
@@ -3274,58 +3247,20 @@ Zotero.Item.prototype.__defineGetter__('attachmentPath', function () {
 		return undefined;
 	}
 	
-	var pathIsRelative = false;
-	
 	if (this._attachmentPath !== null)  {
-		pathIsRelative = this._attachmentPath.indexOf(Zotero.Attachments.BASE_PATH_PLACEHOLDER) == 0
-			&& this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE;
-		if (!pathIsRelative) {
-			return this._attachmentPath;
-		}
-	} 
-	else if (!this.id) {
+		return this._attachmentPath;
+	}
+	
+	if (!this.id) {
 		return '';
 	}
-	else {
-		var sql = "SELECT path FROM itemAttachments WHERE itemID=?";
-		var path = Zotero.DB.valueQuery(sql, this.id);
-		if (!path) {
-			this._attachmentPath = '';
-			return this._attachmentPath;
-		}
-		
-		this._attachmentPath = path;
-		
-		pathIsRelative = path.indexOf(Zotero.Attachments.BASE_PATH_PLACEHOLDER) == 0
-			&& this.attachmentLinkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE;
-	}
 	
-	if (pathIsRelative) {
-		var basePath = Zotero.Prefs.get('baseAttachmentPath');
-		// If the base path has been cleared, don't try to recreate the full attachment path
-		if (basePath == '') {
-			return '';
-		}
-		var baseDir = Components.classes["@mozilla.org/file/local;1"]
-			.createInstance(Components.interfaces.nsILocalFile);
-		try {
-			baseDir.persistentDescriptor = basePath;
-		}
-		catch (e) {
-			Zotero.debug(e, 1);
-			Components.utils.reportError(e);
-			return '';
-		}
-		
-		var relativePath = this._attachmentPath.substr(
-			Zotero.Attachments.BASE_PATH_PLACEHOLDER.length
-		);
-		var attachmentFile = Components.classes["@mozilla.org/file/local;1"]
-			.createInstance(Components.interfaces.nsILocalFile);
-		attachmentFile.setRelativeDescriptor(baseDir,relativePath);
-		return attachmentFile.persistentDescriptor;
+	var sql = "SELECT path FROM itemAttachments WHERE itemID=?";
+	var path = Zotero.DB.valueQuery(sql, this.id);
+	if (!path) {
+		path = '';
 	}
-	
+	this._attachmentPath = path;
 	return path;
 });
 
@@ -3366,7 +3301,9 @@ Zotero.Item.prototype.updateAttachmentPath = function () {
 		this._changedAttachmentData = {};
 	}
 	this._changedAttachmentData.path = true;
-	this.save();
+	this.save({
+		skipDateModifiedUpdate: true
+	});
 };
 
 
diff --git a/chrome/content/zotero/xpcom/sync.js b/chrome/content/zotero/xpcom/sync.js
index 341d5b7d1..d1b9d64eb 100644
--- a/chrome/content/zotero/xpcom/sync.js
+++ b/chrome/content/zotero/xpcom/sync.js
@@ -4019,7 +4019,9 @@ Zotero.Sync.Server.Data = new function() {
 					var msg = Zotero.getString('sync.error.invalidCharsFilename', filename);
 					var e = new Zotero.Error(msg, 0, { dialogButtonText: null });
 					throw (e);
-
+				}
+				if (item.attachment.linkMode == Zotero.Attachments.LINK_MODE_LINKED_FILE) {
+					path = Zotero.Attachments.getBaseDirectoryRelativePath(path);
 				}
 				var pathElem = doc.createElement('path');
 				pathElem.appendChild(doc.createTextNode(path));
diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js
index d14127064..13591ae6c 100644
--- a/chrome/content/zotero/zoteroPane.js
+++ b/chrome/content/zotero/zoteroPane.js
@@ -3444,8 +3444,9 @@ var ZoteroPane = new function()
 			}
 			
 			var file = item.getFile();
-			Zotero.debug("Opening " + file.path);
 			if (file) {
+				Zotero.debug("Opening " + file.path);
+				
 				if(forceExternalViewer !== undefined) {
 					var externalViewer = forceExternalViewer;
 				} else {