Import base-directory-relative linked files

Zotero RDF contained 'attachments:' paths when files weren't included
but they weren't imported properly

chrome/content/zotero/xpcom/attachments.js

@@ -146,7 +146,7 @@ Zotero.Attachments = new function(){
 	
 	
 	/**
-	 * @param {nsIFile|String} [options.file] - File to add
+	 * @param {nsIFile|String} options.file - File to add
 	 * @param {Integer[]|String[]} [options.parentItemID] - Parent item to add item to
 	 * @param {Integer[]} [options.collections] - Collection keys or ids to add new item to
 	 * @param {Object} [options.saveOptions] - Options to pass to Zotero.Item::save()
@@ -185,6 +185,72 @@ Zotero.Attachments = new function(){
 	});
 	
 	
+	/**
+	 * @param {String} options.path - Relative path to file
+	 * @param {String} options.title
+	 * @param {String} options.contentType
+	 * @param {Integer[]|String[]} [options.parentItemID] - Parent item to add item to
+	 * @param {Integer[]} [options.collections] - Collection keys or ids to add new item to
+	 * @param {Object} [options.saveOptions] - Options to pass to Zotero.Item::save()
+	 * @return {Promise<Zotero.Item>}
+	 */
+	this.linkFromFileWithRelativePath = async function (options) {
+		Zotero.debug('Linking attachment from file in base directory');
+		
+		var path = options.path;
+		var title = options.title;
+		var contentType = options.contentType;
+		var parentItemID = options.parentItemID;
+		var collections = options.collections;
+		var saveOptions = options.saveOptions;
+		
+		if (!path) {
+			throw new Error("'path' not provided");
+		}
+		
+		if (path.startsWith('/') || path.match(/^[A-Z]:\\/)) {
+			throw new Error("'path' must be a relative path");
+		}
+		
+		if (!title) {
+			throw new Error("'title' not provided");
+		}
+		
+		if (!contentType) {
+			throw new Error("'contentType' not provided");
+		}
+		
+		if (parentItemID && collections) {
+			throw new Error("parentItemID and collections cannot both be provided");
+		}
+		
+		path = Zotero.Attachments.BASE_PATH_PLACEHOLDER + path;
+		var item = await _addToDB({
+			file: path,
+			title,
+			linkMode: this.LINK_MODE_LINKED_FILE,
+			contentType,
+			parentItemID,
+			collections,
+			saveOptions
+		});
+		
+		// If the file is found (which requires a base directory being set and the file existing),
+		// index it
+		var file = this.resolveRelativePath(path);
+		if (file && await OS.File.exists(file)) {
+			try {
+				await _postProcessFile(item, file, contentType);
+			}
+			catch (e) {
+				Zotero.logError(e);
+			}
+		}
+		
+		return item;
+	};
+	
+	
 	/**
 	 * @param {Object} options - 'file', 'url', 'title', 'contentType', 'charset', 'parentItemID', 'singleFile'
 	 * @return {Promise<Zotero.Item>}
@@ -1467,8 +1533,16 @@ Zotero.Attachments = new function(){
 	/**
 	 * Create a new item of type 'attachment' and add to the itemAttachments table
 	 *
-	 * @param {Object} options - 'file', 'url', 'title', 'linkMode', 'contentType', 'charsetID',
+	 * @param {Object} options
-	 *     'parentItemID', 'saveOptions'
+	 * @param {nsIFile|String} [file]
+	 * @param {String} [url]
+	 * @param {String} title
+	 * @param {Number} linkMode
+	 * @param {String} contentType
+	 * @param {String} [charset]
+	 * @param {Number} [parentItemID]
+	 * @param {String[]|Number[]} [collections]
+	 * @param {Object} [saveOptions]
 	 * @return {Promise<Zotero.Item>} - A promise for the new attachment
 	 */
 	function _addToDB(options) {
@@ -1504,7 +1578,7 @@ Zotero.Attachments = new function(){
 			attachmentItem.attachmentContentType = contentType;
 			attachmentItem.attachmentCharset = charset;
 			if (file) {
-				attachmentItem.attachmentPath = file.path;
+				attachmentItem.attachmentPath = typeof file == 'string' ? file : file.path;
 			}
 			
 			if (collections) {

chrome/content/zotero/xpcom/translation/translate_item.js

@@ -340,6 +340,18 @@ Zotero.Translate.ItemSaver.prototype = {
 		}
 		
 		if (attachment.path) {
+			// If we have an explicit "attachments:" value, just save that as a linked file
+			if (attachment.path.startsWith(Zotero.Attachments.BASE_PATH_PLACEHOLDER)) {
+				attachment.linkMode = "linked_file";
+				return Zotero.Attachments.linkFromFileWithRelativePath({
+					path: attachment.path.substr(Zotero.Attachments.BASE_PATH_PLACEHOLDER.length),
+					title: attachment.title,
+					contentType: attachment.mimeType,
+					parentItemID,
+					collections: !parentItemID ? this._collections : undefined
+				});
+			}
+			
 			var url = Zotero.Attachments.cleanAttachmentURI(attachment.path, false);
 			if (url && /^(?:https?|ftp):/.test(url)) {
 				// A web URL. Don't bother parsing it as path below

test/tests/attachmentsTest.js

@@ -112,6 +112,115 @@ describe("Zotero.Attachments", function() {
 		})
 	})
 	
+	
+	describe("#linkFromFileWithRelativePath()", function () {
+		afterEach(function () {
+			Zotero.Prefs.clear('baseAttachmentPath');
+		});
+		
+		it("should link to a file using a relative path with no base directory set", async function () {
+			Zotero.Prefs.clear('baseAttachmentPath');
+			
+			var item = await createDataObject('item');
+			var spy = sinon.spy(Zotero.Fulltext, 'indexPDF');
+			var relPath = 'a/b/test.pdf';
+			
+			var attachment = await Zotero.Attachments.linkFromFileWithRelativePath({
+				path: relPath,
+				title: 'test.pdf',
+				parentItemID: item.id,
+				contentType: 'application/pdf'
+			});
+			
+			assert.ok(spy.notCalled);
+			spy.restore();
+			assert.equal(
+				attachment.attachmentPath,
+				Zotero.Attachments.BASE_PATH_PLACEHOLDER + relPath
+			);
+		});
+		
+		
+		it("should link to a file using a relative path within the base directory", async function () {
+			var baseDir = await getTempDirectory();
+			Zotero.Prefs.set('baseAttachmentPath', baseDir);
+			Zotero.Prefs.set('saveRelativeAttachmentPath', true);
+			
+			var subDir = OS.Path.join(baseDir, 'foo');
+			await OS.File.makeDir(subDir);
+			
+			var file = OS.Path.join(subDir, 'test.pdf');
+			await OS.File.copy(OS.Path.join(getTestDataDirectory().path, 'test.pdf'), file);
+			
+			var item = await createDataObject('item');
+			var spy = sinon.spy(Zotero.Fulltext, 'indexPDF');
+			var relPath = 'foo/test.pdf';
+			
+			var attachment = await Zotero.Attachments.linkFromFileWithRelativePath({
+				path: relPath,
+				title: 'test.pdf',
+				parentItemID: item.id,
+				contentType: 'application/pdf'
+			});
+			
+			assert.ok(spy.called);
+			spy.restore();
+			assert.equal(
+				attachment.attachmentPath,
+				Zotero.Attachments.BASE_PATH_PLACEHOLDER + relPath
+			);
+			
+			assert.ok(await attachment.fileExists());
+		});
+		
+		
+		it("should link to a nonexistent file using a relative path within the base directory", async function () {
+			var baseDir = await getTempDirectory();
+			Zotero.Prefs.set('baseAttachmentPath', baseDir);
+			Zotero.Prefs.set('saveRelativeAttachmentPath', true);
+			
+			var subDir = OS.Path.join(baseDir, 'foo');
+			await OS.File.makeDir(subDir);
+			
+			var item = await createDataObject('item');
+			var spy = sinon.spy(Zotero.Fulltext, 'indexPDF');
+			var relPath = 'foo/test.pdf';
+			
+			var attachment = await Zotero.Attachments.linkFromFileWithRelativePath({
+				path: relPath,
+				title: 'test.pdf',
+				parentItemID: item.id,
+				contentType: 'application/pdf'
+			});
+			
+			assert.ok(spy.notCalled);
+			spy.restore();
+			assert.equal(
+				attachment.attachmentPath,
+				Zotero.Attachments.BASE_PATH_PLACEHOLDER + relPath
+			);
+			
+			assert.isFalse(await attachment.fileExists());
+		});
+		
+		
+		it("should reject absolute paths", async function () {
+			try {
+				await Zotero.Attachments.linkFromFileWithRelativePath({
+					path: '/a/b/test.pdf',
+					title: 'test.pdf',
+					contentType: 'application/pdf'
+				});
+			}
+			catch (e) {
+				return;
+			}
+			
+			assert.fail();
+		});
+	});
+	
+	
 	describe("#importSnapshotFromFile()", function () {
 		it("should import an HTML file", function* () {
 			var item = yield createDataObject('item');