From 5f9e8f5b7e322473d0bafe724533e374e443dc90 Mon Sep 17 00:00:00 2001 From: Abe Jellinek Date: Tue, 1 Mar 2022 11:34:46 -0800 Subject: [PATCH] Automatically relink attachments from LABD (#2374) Fixes #2092 --- chrome/content/zotero/locateMenu.js | 8 +- chrome/content/zotero/xpcom/data/items.js | 23 +++ chrome/content/zotero/xpcom/file.js | 30 ++- chrome/content/zotero/zoteroPane.js | 201 ++++++++++++++++++- chrome/locale/en-US/zotero/zotero.properties | 10 + test/tests/fileTest.js | 14 ++ test/tests/zoteroPaneTest.js | 165 +++++++++++++++ 7 files changed, 437 insertions(+), 14 deletions(-) diff --git a/chrome/content/zotero/locateMenu.js b/chrome/content/zotero/locateMenu.js index e353ccd2e6..7927f4d28d 100644 --- a/chrome/content/zotero/locateMenu.js +++ b/chrome/content/zotero/locateMenu.js @@ -521,7 +521,13 @@ var Zotero_LocateMenu = new function() { if(attachment.attachmentLinkMode !== Zotero.Attachments.LINK_MODE_LINKED_URL) { var path = yield attachment.getFilePathAsync(); if (path) { - var ext = Zotero.File.getExtension(Zotero.File.pathToFile(path)); + try { + var ext = Zotero.File.getExtension(Zotero.File.pathToFile(path)); + } + catch (e) { + Zotero.logError(e); + return false; + } if(!attachment.attachmentContentType || Zotero.MIME.hasNativeHandler(attachment.attachmentContentType, ext)) { return false; diff --git a/chrome/content/zotero/xpcom/data/items.js b/chrome/content/zotero/xpcom/data/items.js index a68b773a0e..6bb83eeb03 100644 --- a/chrome/content/zotero/xpcom/data/items.js +++ b/chrome/content/zotero/xpcom/data/items.js @@ -2055,6 +2055,29 @@ Zotero.Items = function() { } return title.trim(); }; + + + /** + * Find attachment items whose paths point to missing files and begin with + * the passed `pathPrefix`. + * + * @param {Number} libraryID + * @param {String} pathPrefix + * @return {Zotero.Item[]} + */ + this.findMissingLinkedFiles = async function (libraryID, pathPrefix) { + let sql = "SELECT itemID FROM items JOIN itemAttachments USING (itemID) " + + "WHERE itemID NOT IN (SELECT itemID FROM deletedItems) " + + `AND linkMode=${Zotero.Attachments.LINK_MODE_LINKED_FILE} ` + + "AND path LIKE ? ESCAPE '\\' " + + "AND libraryID=?"; + let ids = await Zotero.DB.columnQueryAsync(sql, [Zotero.DB.escapeSQLExpression(pathPrefix) + '%', libraryID]); + let items = await this.getAsync(ids); + let missingItems = await Promise.all( + items.map(async item => (await item.fileExists() ? false : item)) + ); + return missingItems.filter(Boolean); + }; Zotero.DataObjects.call(this); diff --git a/chrome/content/zotero/xpcom/file.js b/chrome/content/zotero/xpcom/file.js index c273efe845..f5045b1517 100644 --- a/chrome/content/zotero/xpcom/file.js +++ b/chrome/content/zotero/xpcom/file.js @@ -1088,7 +1088,31 @@ Zotero.File = new function(){ } throw e; } - } + }; + + + /** + * Normalize to a Unix-style path, replacing backslashes (interpreted as + * separators only on Windows) with forward slashes (interpreted as + * separators everywhere) + * + * @param {String} path + * @return {String} + */ + this.normalizeToUnix = function (path) { + // If we're on Windows, we need to normalize first and then replace + // the slashes, because OS.Path.normalize won't handle forward slashes + // correctly. Otherwise, we replace slashes first and *then* normalize. + // This should ensure consistent behavior across platforms. + if (Zotero.isWin) { + let normalized = OS.Path.normalize(path); + return normalized.replace(/\\/g, '/'); + } + else { + let replaced = path.replace(/\\/g, '/'); + return OS.Path.normalize(replaced); + } + }; /** @@ -1098,8 +1122,8 @@ Zotero.File = new function(){ if (typeof dir != 'string') throw new Error("dir must be a string"); if (typeof file != 'string') throw new Error("file must be a string"); - dir = OS.Path.normalize(dir).replace(/\\/g, "/"); - file = OS.Path.normalize(file).replace(/\\/g, "/"); + dir = this.normalizeToUnix(dir); + file = this.normalizeToUnix(file); // Normalize D:\ vs. D:\foo if (dir != file && !dir.endsWith('/')) { dir += '/'; diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 431d714a75..51be9d5577 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -4461,7 +4461,7 @@ var ZoteroPane = new function() let path = item.getFilePath(); if (!path) { ZoteroPane_Local.showAttachmentNotFoundDialog( - item.id, + item, path, { noLocate: true, @@ -4550,7 +4550,7 @@ var ZoteroPane = new function() if (isLinkedFile || !fileSyncingEnabled) { this.showAttachmentNotFoundDialog( - itemID, + item, path, { noLocate: noLocateOnMissing, @@ -4576,7 +4576,7 @@ var ZoteroPane = new function() if (!await item.getFilePathAsync()) { ZoteroPane_Local.showAttachmentNotFoundDialog( - item.id, + item, path, { noLocate: noLocateOnMissing, @@ -4643,7 +4643,7 @@ var ZoteroPane = new function() if (!fileExists) { this.showAttachmentNotFoundDialog( - attachment.id, + attachment, path, { noLocate: noLocateOnMissing, @@ -4789,9 +4789,13 @@ var ZoteroPane = new function() } - this.showAttachmentNotFoundDialog = function (itemID, path, options = {}) { + this.showAttachmentNotFoundDialog = async function (item, path, options = {}) { var { noLocate, notOnServer, linkedFile } = options; - + + if (item.isLinkedFileAttachment() && await this.checkForLinkedFilesToRelink(item)) { + return; + } + var title = Zotero.getString('pane.item.attachments.fileNotFound.title'); var text = Zotero.getString( 'pane.item.attachments.fileNotFound.text1' + (path ? '.path' : '') @@ -4806,7 +4810,7 @@ var ZoteroPane = new function() ), [ZOTERO_CONFIG.CLIENT_NAME, ZOTERO_CONFIG.DOMAIN_NAME] ); - var supportURL = options.linkedFile + var supportURL = linkedFile ? 'https://www.zotero.org/support/kb/missing_linked_file' : 'https://www.zotero.org/support/kb/files_not_syncing'; @@ -4844,12 +4848,69 @@ var ZoteroPane = new function() ); if (index == 0) { - this.relinkAttachment(itemID); + this.relinkAttachment(item.id); } else if (index == 2) { this.loadURI(supportURL, { metaKey: true, shiftKey: true }); } } + + + /** + * Prompt the user to relink one or all of the attachment files found in + * the LABD. + * + * @param {Zotero.Item} item + * @param {String} path Path to the file matching `item` + * @param {Number} numOthers If zero, "Relink All" option is not offered + * @return {'one' | 'all' | 'manual' | 'cancel'} + */ + this.showLinkedFileFoundAutomaticallyDialog = function (item, path, numOthers) { + let ps = Services.prompt; + + let title = Zotero.getString('pane.item.attachments.autoRelink.title'); + let text = Zotero.getString('pane.item.attachments.autoRelink.text1') + '\n\n' + + Zotero.getString('pane.item.attachments.autoRelink.text2', item.getFilePath()) + '\n' + + Zotero.getString('pane.item.attachments.autoRelink.text3', path) + '\n\n' + + Zotero.getString('pane.item.attachments.autoRelink.text4', Zotero.appName); + let buttonFlags = ps.BUTTON_POS_0 * ps.BUTTON_TITLE_IS_STRING + + ps.BUTTON_POS_1 * ps.BUTTON_TITLE_CANCEL + + ps.BUTTON_POS_2 * ps.BUTTON_TITLE_IS_STRING; + let index = ps.confirmEx(null, + title, + text, + buttonFlags, + Zotero.getString('pane.item.attachments.autoRelink.relink'), + null, + Zotero.getString('pane.item.attachments.autoRelink.locateManually'), + null, {} + ); + + if (index == 1) { + // Cancel + return 'cancel'; + } + else if (index == 2) { + // Locate Manually... + return 'manual'; + } + + // Relink + if (!numOthers) { + return 'one'; + } + + title = Zotero.getString('pane.item.attachments.autoRelinkOthers.title'); + text = Zotero.getString('pane.item.attachments.autoRelinkOthers.text', numOthers, numOthers); + index = ps.confirmEx(null, + title, + text, + ps.STD_YES_NO_BUTTONS, + null, null, null, null, {} + ); + + return index == 0 ? 'all' : 'one'; + }; this.syncAlert = function (e) { @@ -5346,6 +5407,120 @@ var ZoteroPane = new function() } } }; + + + /** + * Attempt to find a file in the LABD matching the passed attachment + * by searching successive subdirectories. Prompt the user if a match is + * found and offer to relink one or all matching files in the directory. + * The user can also choose to relink manually, which opens a file picker. + * + * If the synced path is 'C:\Users\user\Documents\Dissertation\Files\Paper.pdf', + * the LABD is '/Users/user/Documents', and the (not yet known) correct local + * path is '/Users/user/Documents/Dissertation/Files/Paper.pdf', check: + * + * 1. /Users/user/Documents/Users/user/Documents/Dissertation/Files/Paper.pdf + * 2. /Users/user/Documents/user/Documents/Dissertation/Files/Paper.pdf + * 3. /Users/user/Documents/Documents/Dissertation/Files/Paper.pdf + * 4. /Users/user/Documents/Dissertation/Files/Paper.pdf + * + * If line 4 had not been the correct local path (in other words, if no file + * existed at that path), we would have continued on to check + * '/Users/user/Documents/Dissertation/Paper.pdf'. If that did not match, + * with no more segments in the synced path to drop, we would have given up. + * + * Once we find the file, check for other linked files beginning with + * C:\Users\user\Documents\Dissertation\Files and see if they exist relative + * to /Users/user/Documents/Dissertation/Files, and prompt to relink them + * all if so. + * + * @param {Zotero.Item} item + * @return {Promise} True if relinked successfully or canceled + */ + this.checkForLinkedFilesToRelink = async function (item) { + Zotero.debug('Attempting to relink automatically'); + + let basePath = Zotero.Prefs.get('baseAttachmentPath'); + if (!basePath) { + Zotero.debug('No LABD'); + return false; + } + Zotero.debug('LABD path: ' + basePath); + + let syncedPath = item.getFilePath(); + if (!syncedPath) { + Zotero.debug('No synced path'); + return false; + } + syncedPath = Zotero.File.normalizeToUnix(syncedPath); + Zotero.debug('Synced path: ' + syncedPath); + + if (Zotero.File.directoryContains(basePath, syncedPath)) { + // Already in the LABD - nothing to do + Zotero.debug('Synced path is already within LABD'); + return false; + } + + // We can't use OS.Path.dirname because that function expects paths valid for the current platform... + // but we can't normalize first because we're going to be comparing it to other un-normalized paths + let unNormalizedDirname = item.getFilePath(); + let lastSlash = Math.max( + unNormalizedDirname.lastIndexOf('/'), + unNormalizedDirname.lastIndexOf('\\') + ); + if (lastSlash != -1) { + unNormalizedDirname = unNormalizedDirname.substring(0, lastSlash + 1); + } + + let parts = OS.Path.split(syncedPath).components; + for (let segmentsToDrop = 0; segmentsToDrop < parts.length; segmentsToDrop++) { + let correctedPath = OS.Path.join(basePath, ...parts.slice(segmentsToDrop)); + + if (!(await OS.File.exists(correctedPath))) { + Zotero.debug('Does not exist: ' + correctedPath); + continue; + } + Zotero.debug('Exists! ' + correctedPath); + + let otherUnlinked = await Zotero.Items.findMissingLinkedFiles( + item.libraryID, + unNormalizedDirname + ); + let othersToRelink = new Map(); + for (let otherItem of otherUnlinked) { + if (otherItem.id === item.id) continue; + let otherParts = otherItem.getFilePath() + .split(/[/\\]/) + // Slice as much off the beginning as when creating correctedPath + .slice(segmentsToDrop); + if (!otherParts.length) continue; + let otherCorrectedPath = OS.Path.join(basePath, ...otherParts); + if (await OS.File.exists(otherCorrectedPath)) { + othersToRelink.set(otherItem, otherCorrectedPath); + } + } + + let choice = this.showLinkedFileFoundAutomaticallyDialog(item, correctedPath, othersToRelink.size); + switch (choice) { + case 'one': + await item.relinkAttachmentFile(correctedPath); + return true; + case 'all': + await item.relinkAttachmentFile(correctedPath); + await Promise.all([...othersToRelink] + .map(([i, p]) => i.relinkAttachmentFile(p))); + return true; + case 'manual': + await this.relinkAttachment(item.id); + return true; + case 'cancel': + return true; + } + } + + Zotero.debug('No segments left to drop; match not found in LABD'); + return false; + }; this.relinkAttachment = async function (itemID) { @@ -5358,7 +5533,7 @@ var ZoteroPane = new function() if (!item) { throw new Error('Item ' + itemID + ' not found in ZoteroPane_Local.relinkAttachment()'); } - + while (true) { let fp = new FilePicker(); fp.init(window, Zotero.getString('pane.item.attachments.select'), fp.modeOpen); @@ -5371,7 +5546,13 @@ var ZoteroPane = new function() var dir = await Zotero.File.getClosestDirectory(file); if (dir) { - fp.displayDirectory = dir; + try { + fp.displayDirectory = dir; + } + catch (e) { + // Directory is invalid; ignore and go with the home directory + fp.displayDirectory = OS.Constants.Path.homeDir; + } } fp.appendFilters(fp.filterAll); diff --git a/chrome/locale/en-US/zotero/zotero.properties b/chrome/locale/en-US/zotero/zotero.properties index 14f7ba3c5d..a4d14dfc95 100644 --- a/chrome/locale/en-US/zotero/zotero.properties +++ b/chrome/locale/en-US/zotero/zotero.properties @@ -415,6 +415,16 @@ pane.item.attachments.fileNotFound.text2.stored = It may have been moved or dele pane.item.attachments.fileNotFound.text2.stored.notOnServer = It may have been moved or deleted outside of %1$S, or, if the file was added on another computer, it may not yet have been synced to %2$S. pane.item.attachments.fileNotFound.text2.linked = It may have been moved or deleted outside of %1$S, or a Linked Attachment Base Directory may be set incorrectly on one of your computers. pane.item.attachments.delete.confirm = Are you sure you want to delete this attachment? +pane.item.attachments.autoRelink.title = File Located Automatically +pane.item.attachments.autoRelink.text1 = The file could not be found at the specified location, but a matching file was found in your Linked Attachment Base Directory: +pane.item.attachments.autoRelink.text2 = Old Location: %S +pane.item.attachments.autoRelink.text3 = New Location: %S +pane.item.attachments.autoRelink.text4 = %1$S can automatically relink this attachment. +pane.item.attachments.autoRelink.relink = Relink +pane.item.attachments.autoRelinkOthers.title = More Files Located +pane.item.attachments.autoRelinkOthers.text = One other unlinked attachment in this library was found in the same directory. Relink the additional located attachment?;%S other unlinked attachments in this library were found in the same directory. Relink the additional located attachments? +pane.item.attachments.autoRelink.locateManually = Locate Manually… +pane.item.attachments.autoRelink.relinkAll = Relink All pane.item.attachments.count.zero = %S attachments: pane.item.attachments.count.singular = %S attachment: pane.item.attachments.count.plural = %S attachments: diff --git a/test/tests/fileTest.js b/test/tests/fileTest.js index c5926dd560..ee9070443e 100644 --- a/test/tests/fileTest.js +++ b/test/tests/fileTest.js @@ -535,4 +535,18 @@ describe("Zotero.File", function () { assert.equal(contents, 'Hello Zotero\n'); }); }); + + describe("#normalizeToUnix()", function () { + it("should normalize a Unix-style path", async function () { + assert.equal(Zotero.File.normalizeToUnix('/path/to/directory/'), '/path/to/directory'); + }); + + it("should normalize '.' and '..'", async function () { + assert.equal(Zotero.File.normalizeToUnix('/path/./to/some/../file'), '/path/to/file'); + }); + + it("should replace backslashes with forward slashes and trim trailing", async function () { + assert.equal(Zotero.File.normalizeToUnix('C:\\Zotero\\Some\\Directory\\'), 'C:/Zotero/Some/Directory'); + }); + }); }) diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index b0f8eb48e4..1076f39e1c 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -1269,4 +1269,169 @@ describe("ZoteroPane", function() { assert.isFalse(attachment3.deleted); }); }); + + describe("#checkForLinkedFilesToRelink()", function () { + let labdDir; + + this.beforeEach(async () => { + labdDir = await getTempDirectory(); + Zotero.Prefs.set('baseAttachmentPath', labdDir); + Zotero.Prefs.set('saveRelativeAttachmentPath', true); + }); + + it("should detect and relink a single attachment", async function () { + let item = await createDataObject('item'); + let file = getTestDataDirectory(); + file.append('test.pdf'); + let outsideStorageDir = await getTempDirectory(); + let outsideFile = OS.Path.join(outsideStorageDir, 'test.pdf'); + + let labdFile = OS.Path.join(labdDir, 'test.pdf'); + + await OS.File.copy(file.path, outsideFile); + + let attachment = await Zotero.Attachments.linkFromFile({ + file: outsideFile, + parentItemID: item.id + }); + + await assert.eventually.isTrue(attachment.fileExists()); + await OS.File.move(outsideFile, labdFile); + await assert.eventually.isFalse(attachment.fileExists()); + + let stub = sinon.stub(zp, 'showLinkedFileFoundAutomaticallyDialog') + .returns('one'); + await zp.checkForLinkedFilesToRelink(attachment); + assert.ok(stub.calledOnce); + assert.ok(stub.calledWith(attachment, sinon.match.string, 0)); + + await assert.eventually.isTrue(attachment.fileExists()); + assert.equal(attachment.getFilePath(), labdFile); + assert.equal(attachment.attachmentPath, 'attachments:test.pdf'); + + stub.restore(); + }); + + it("should detect and relink multiple attachments when user chooses", async function () { + for (let choice of ['one', 'all']) { + let file1 = getTestDataDirectory(); + file1.append('test.pdf'); + let file2 = getTestDataDirectory(); + file2.append('empty.pdf'); + let outsideStorageDir = await getTempDirectory(); + let outsideFile1 = OS.Path.join(outsideStorageDir, 'test.pdf'); + let outsideFile2 = OS.Path.join(outsideStorageDir, 'empty.pdf'); + + let labdFile1 = OS.Path.join(labdDir, 'test.pdf'); + let labdFile2 = OS.Path.join(labdDir, 'empty.pdf'); + + await OS.File.copy(file1.path, outsideFile1); + await OS.File.copy(file2.path, outsideFile2); + + let attachment1 = await Zotero.Attachments.linkFromFile({ file: outsideFile1 }); + let attachment2 = await Zotero.Attachments.linkFromFile({ file: outsideFile2 }); + + await assert.eventually.isTrue(attachment1.fileExists()); + await assert.eventually.isTrue(attachment2.fileExists()); + await OS.File.move(outsideFile1, labdFile1); + await OS.File.move(outsideFile2, labdFile2); + await assert.eventually.isFalse(attachment1.fileExists()); + await assert.eventually.isFalse(attachment2.fileExists()); + + let stub = sinon.stub(zp, 'showLinkedFileFoundAutomaticallyDialog') + .returns(choice); + await zp.checkForLinkedFilesToRelink(attachment1); + assert.ok(stub.calledOnce); + assert.ok(stub.calledWith(attachment1, sinon.match.string, 1)); + + await assert.eventually.isTrue(attachment1.fileExists()); + await assert.eventually.equal(attachment2.fileExists(), choice === 'all'); + assert.equal(attachment1.getFilePath(), labdFile1); + assert.equal(attachment1.attachmentPath, 'attachments:test.pdf'); + if (choice === 'all') { + assert.equal(attachment2.getFilePath(), labdFile2); + assert.equal(attachment2.attachmentPath, 'attachments:empty.pdf'); + } + else { + assert.equal(attachment2.getFilePath(), outsideFile2); + } + + stub.restore(); + } + }); + + it("should use subdirectories of original path", async function () { + let file = getTestDataDirectory(); + file.append('test.pdf'); + let outsideStorageDir = OS.Path.join(await getTempDirectory(), 'subdir'); + await OS.File.makeDir(outsideStorageDir); + let outsideFile = OS.Path.join(outsideStorageDir, 'test.pdf'); + + let labdSubdir = OS.Path.join(labdDir, 'subdir'); + await OS.File.makeDir(labdSubdir); + let labdFile = OS.Path.join(labdSubdir, 'test.pdf'); + + await OS.File.copy(file.path, outsideFile); + + let attachment = await Zotero.Attachments.linkFromFile({ file: outsideFile }); + + await assert.eventually.isTrue(attachment.fileExists()); + await OS.File.move(outsideFile, labdFile); + await assert.eventually.isFalse(attachment.fileExists()); + + let dialogStub = sinon.stub(zp, 'showLinkedFileFoundAutomaticallyDialog') + .returns('one'); + let existsSpy = sinon.spy(OS.File, 'exists'); + await zp.checkForLinkedFilesToRelink(attachment); + assert.ok(dialogStub.calledOnce); + assert.ok(dialogStub.calledWith(attachment, sinon.match.string, 0)); + assert.ok(existsSpy.calledWith(OS.Path.join(labdSubdir, 'test.pdf'))); + assert.notOk(existsSpy.calledWith(OS.Path.join(labdDir, 'test.pdf'))); // Should never get there + + await assert.eventually.isTrue(attachment.fileExists()); + assert.equal(attachment.getFilePath(), labdFile); + assert.equal(attachment.attachmentPath, 'attachments:subdir/test.pdf'); + + dialogStub.restore(); + existsSpy.restore(); + }); + + it("should handle Windows paths", async function () { + let filenames = [['test.pdf'], ['empty.pdf'], ['search', 'baz.pdf']]; + let labdFiles = []; + let attachments = []; + + for (let parts of filenames) { + let file = getTestDataDirectory(); + parts.forEach(part => file.append(part)); + + await OS.File.makeDir(OS.Path.join(labdDir, ...parts.slice(0, -1))); + let labdFile = OS.Path.join(labdDir, ...parts); + await OS.File.copy(file.path, labdFile); + labdFiles.push(labdFile); + + let attachment = await Zotero.Attachments.linkFromFile({ file }); + attachment.attachmentPath = `C:\\test\\${parts.join('\\')}`; + await attachment.saveTx(); + attachments.push(attachment); + + await assert.eventually.isFalse(attachment.fileExists()); + } + + let stub = sinon.stub(zp, 'showLinkedFileFoundAutomaticallyDialog') + .returns('all'); + await zp.checkForLinkedFilesToRelink(attachments[0]); + assert.ok(stub.calledOnce); + assert.ok(stub.calledWith(attachments[0], sinon.match.string, filenames.length - 1)); + + for (let i = 0; i < filenames.length; i++) { + let attachment = attachments[i]; + await assert.eventually.isTrue(attachment.fileExists()); + assert.equal(attachment.getFilePath(), labdFiles[i]); + assert.equal(attachment.attachmentPath, 'attachments:' + OS.Path.join(...filenames[i])); + } + + stub.restore(); + }); + }); })