diff --git a/chrome/content/zotero/xpcom/data/item.js b/chrome/content/zotero/xpcom/data/item.js index 30fcd960c..9f6fc10e8 100644 --- a/chrome/content/zotero/xpcom/data/item.js +++ b/chrome/content/zotero/xpcom/data/item.js @@ -2416,14 +2416,18 @@ Zotero.Item.prototype.fileExistsCached = function () { -/* +/** * Rename file associated with an attachment * - * -1 Destination file exists -- use _force_ to overwrite - * -2 Error renaming - * false Attachment file not found + * @param {String} newName + * @param {Boolean} [overwrite=false] - Overwrite file if one exists + * @param {Boolean} [unique=false] - Add suffix to create unique filename if necessary + * @return {Number|false} -- true - Rename successful + * -1 - Destination file exists; use _force_ to overwrite + * -2 - Error renaming + * false - Attachment file not found */ -Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function* (newName, overwrite) { +Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function* (newName, overwrite=false, unique=false) { var origPath = yield this.getFilePathAsync(); if (!origPath) { Zotero.debug("Attachment file not found in renameAttachmentFile()", 2); @@ -2442,21 +2446,57 @@ Zotero.Item.prototype.renameAttachmentFile = Zotero.Promise.coroutine(function* return true; } - var destPath = OS.Path.join(OS.Path.dirname(origPath), newName); + var parentDir = OS.Path.dirname(origPath); + var destPath = OS.Path.join(parentDir, newName); var destName = OS.Path.basename(destPath); + // Get root + extension, if there is one + var pos = destName.lastIndexOf('.'); + if (pos > 0) { + var root = destName.substr(0, pos); + var ext = destName.substr(pos + 1); + } + else { + var root = destName; + } // Update mod time and clear hash so the file syncs // TODO: use an integer counter instead of mod time for change detection // Update mod time first, because it may fail for read-only files on Windows yield OS.File.setDates(origPath, null, null); - var result = yield OS.File.move(origPath, destPath, { noOverwrite: !overwrite }) - // If no overwriting and file exists, return -1 - .catch(OS.File.Error, function (e) { - if (e.becauseExists) { - return -1; + var result; + var incr = 0; + while (true) { + // If filename already exists, add a numeric suffix to the end of the root, before + // the extension if there is one + if (incr) { + if (ext) { + destName = root + ' ' + (incr + 1) + '.' + ext; + } + else { + destName = root + ' ' + (incr + 1); + } + destPath = OS.Path.join(parentDir, destName); } - throw e; - }); + + try { + result = yield OS.File.move(origPath, destPath, { noOverwrite: !overwrite }) + } + catch (e) { + if (e instanceof OS.File.Error) { + if (e.becauseExists) { + // Increment number to create unique suffix + if (unique) { + incr++; + continue; + } + // If no overwriting or making unique and file exists, return -1 + return -1; + } + } + throw e; + } + break; + } if (result) { return result; } diff --git a/chrome/content/zotero/zoteroPane.js b/chrome/content/zotero/zoteroPane.js index 6d24939d8..c529ea6a0 100644 --- a/chrome/content/zotero/zoteroPane.js +++ b/chrome/content/zotero/zoteroPane.js @@ -4541,7 +4541,7 @@ var ZoteroPane = new function() newName = newName + ext; } - var renamed = yield item.renameAttachmentFile(newName); + var renamed = yield item.renameAttachmentFile(newName, false, true); if (renamed !== true) { Zotero.debug("Could not rename file (" + renamed + ")"); continue; diff --git a/test/tests/zoteroPaneTest.js b/test/tests/zoteroPaneTest.js index 3c144dee7..92f0af59e 100644 --- a/test/tests/zoteroPaneTest.js +++ b/test/tests/zoteroPaneTest.js @@ -244,6 +244,93 @@ describe("ZoteroPane", function() { }) + describe("#renameSelectedAttachmentsFromParents()", function () { + it("should rename a linked file", async function () { + var oldFilename = 'old.png'; + var newFilename = 'Test.png'; + var file = getTestDataDirectory(); + file.append('test.png'); + var tmpDir = await getTempDirectory(); + var oldFile = OS.Path.join(tmpDir, oldFilename); + await OS.File.copy(file.path, oldFile); + + var item = createUnsavedDataObject('item'); + item.setField('title', 'Test'); + await item.saveTx(); + + var attachment = await Zotero.Attachments.linkFromFile({ + file: oldFile, + parentItemID: item.id + }); + await zp.selectItem(attachment.id); + + await assert.eventually.isTrue(zp.renameSelectedAttachmentsFromParents()); + assert.equal(attachment.attachmentFilename, newFilename); + var path = await attachment.getFilePathAsync(); + assert.equal(OS.Path.basename(path), newFilename) + await OS.File.exists(path); + }); + + it("should use unique name for linked file if target name is taken", async function () { + var oldFilename = 'old.png'; + var newFilename = 'Test.png'; + var uniqueFilename = 'Test 2.png'; + var file = getTestDataDirectory(); + file.append('test.png'); + var tmpDir = await getTempDirectory(); + var oldFile = OS.Path.join(tmpDir, oldFilename); + await OS.File.copy(file.path, oldFile); + // Create file with target filename + await Zotero.File.putContentsAsync(OS.Path.join(tmpDir, newFilename), ''); + + var item = createUnsavedDataObject('item'); + item.setField('title', 'Test'); + await item.saveTx(); + + var attachment = await Zotero.Attachments.linkFromFile({ + file: oldFile, + parentItemID: item.id + }); + await zp.selectItem(attachment.id); + + await assert.eventually.isTrue(zp.renameSelectedAttachmentsFromParents()); + assert.equal(attachment.attachmentFilename, uniqueFilename); + var path = await attachment.getFilePathAsync(); + assert.equal(OS.Path.basename(path), uniqueFilename) + await OS.File.exists(path); + }); + + it("should use unique name for linked file without extension if target name is taken", async function () { + var oldFilename = 'old'; + var newFilename = 'Test'; + var uniqueFilename = 'Test 2'; + var file = getTestDataDirectory(); + file.append('test.png'); + var tmpDir = await getTempDirectory(); + var oldFile = OS.Path.join(tmpDir, oldFilename); + await OS.File.copy(file.path, oldFile); + // Create file with target filename + await Zotero.File.putContentsAsync(OS.Path.join(tmpDir, newFilename), ''); + + var item = createUnsavedDataObject('item'); + item.setField('title', 'Test'); + await item.saveTx(); + + var attachment = await Zotero.Attachments.linkFromFile({ + file: oldFile, + parentItemID: item.id + }); + await zp.selectItem(attachment.id); + + await assert.eventually.isTrue(zp.renameSelectedAttachmentsFromParents()); + assert.equal(attachment.attachmentFilename, uniqueFilename); + var path = await attachment.getFilePathAsync(); + assert.equal(OS.Path.basename(path), uniqueFilename) + await OS.File.exists(path); + }); + }); + + describe("#duplicateSelectedItem()", function () { it("should add reverse relations", async function () { await selectLibrary(win);