commit 0f743e55c7c3b398ca4e33f871c19c1440482e30
parent c784db88a88c74cab9b50f30baa487f6f111b107
Author: Dan Stillman <dstillman@zotero.org>
Date: Wed, 1 Nov 2017 01:02:23 -0400
Fix "Rename File from Parent Metadata" if target filename exists
Add a unique numeric suffix to the filename, before any extension
Diffstat:
3 files changed, 141 insertions(+), 14 deletions(-)
diff --git 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
@@ -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
@@ -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);
