Fix relinking of imported attachment with external file

This commit is contained in:
Dan Stillman 2016-10-22 15:02:15 -04:00
parent c1a3a8411f
commit 79baac3158
5 changed files with 86 additions and 40 deletions

View file

@ -797,6 +797,7 @@ Zotero.Attachments = new function(){
var dir = this.getStorageDirectory(item);
yield _moveOrphanedDirectory(dir);
if (!dir.exists()) {
Zotero.debug("Creating directory " + dir.path);
dir.create(Components.interfaces.nsIFile.DIRECTORY_TYPE, 0755);
}
return dir;

View file

@ -2513,43 +2513,53 @@ Zotero.Item.prototype.relinkAttachmentFile = Zotero.Promise.coroutine(function*
if (fileName.endsWith(".lnk")) {
throw new Error("Cannot relink to Windows shortcut");
}
var newPath;
var newName = Zotero.File.getValidFileName(fileName);
if (!newName) {
throw new Error("No valid characters in filename after filtering");
}
var newPath = OS.Path.join(OS.Path.dirname(path), newName);
try {
// If selected file isn't in the attachment's storage directory,
// copy it in and use that one instead
var storageDir = Zotero.Attachments.getStorageDirectory(this).path;
if (this.isImportedAttachment() && OS.Path.dirname(path) != storageDir) {
// If file with same name already exists in the storage directory,
// move it out of the way
let deleteBackup = false;;
if (yield OS.File.exists(newPath)) {
deleteBackup = true;
yield OS.File.move(newPath, newPath + ".bak");
}
let newFile = Zotero.File.copyToUnique(path, newPath);
newPath = newFile.path;
// Delete backup file
if (deleteBackup) {
yield OS.File.remove(newPath + ".bak");
}
// If selected file isn't in the attachment's storage directory,
// copy it in and use that one instead
var storageDir = Zotero.Attachments.getStorageDirectory(this).path;
if (this.isImportedAttachment() && OS.Path.dirname(path) != storageDir) {
newPath = OS.Path.join(storageDir, newName);
// If file with same name already exists in the storage directory,
// move it out of the way
let backupCreated = false;
if (yield OS.File.exists(newPath)) {
backupCreated = true;
yield OS.File.move(newPath, newPath + ".bak");
}
// Rename file to filtered name if necessary
else if (fileName != newName) {
Zotero.debug("Renaming file '" + fileName + "' to '" + newName + "'");
OS.File.move(path, newPath, { noOverwrite: true });
// Create storage directory if necessary
else if (!(yield OS.File.exists(storageDir))) {
Zotero.Attachments.createDirectoryForItem(this);
}
let newFile;
try {
newFile = Zotero.File.copyToUnique(path, newPath);
}
catch (e) {
// Restore backup file if copying failed
if (backupCreated) {
yield OS.File.move(newPath + ".bak", newPath);
}
throw e;
}
newPath = newFile.path;
// Delete backup file
if (backupCreated) {
yield OS.File.remove(newPath + ".bak");
}
}
catch (e) {
Zotero.logError(e);
return false;
// Rename file to filtered name if necessary
else if (fileName != newName) {
newPath = OS.Path.join(OS.Path.dirname(path), newName);
Zotero.debug("Renaming file '" + fileName + "' to '" + newName + "'");
OS.File.move(path, newPath, { noOverwrite: true });
}
this.attachmentPath = newPath;
@ -2560,6 +2570,9 @@ Zotero.Item.prototype.relinkAttachmentFile = Zotero.Promise.coroutine(function*
skipEditCheck: skipItemUpdate
});
this._updateAttachmentStates(true);
yield Zotero.Notifier.trigger('refresh', 'item', this.id);
return true;
});

View file

@ -566,13 +566,18 @@ Zotero.Sync.Storage.Local = {
// If library isn't editable but filename was changed, update
// database without updating the item's mod time, which would result
// in a library access error
if (!Zotero.Items.isEditable(item)) {
Zotero.debug("File renamed without library access -- "
+ "updating itemAttachments path", 3);
yield item.relinkAttachmentFile(newPath, true);
try {
if (!Zotero.Items.isEditable(item)) {
Zotero.debug("File renamed without library access -- "
+ "updating itemAttachments path", 3);
yield item.relinkAttachmentFile(newPath, true);
}
else {
yield item.relinkAttachmentFile(newPath);
}
}
else {
yield item.relinkAttachmentFile(newPath);
catch (e) {
Zotero.File.checkFileAccessError(e, path, 'update');
}
path = newPath;

View file

@ -4512,9 +4512,9 @@ var ZoteroPane = new function()
return;
}
var item = yield Zotero.Items.getAsync(itemID);
var item = Zotero.Items.get(itemID);
if (!item) {
throw('Item ' + itemID + ' not found in ZoteroPane_Local.relinkAttachment()');
throw new Error('Item ' + itemID + ' not found in ZoteroPane_Local.relinkAttachment()');
}
while (true) {
@ -4523,8 +4523,11 @@ var ZoteroPane = new function()
.createInstance(nsIFilePicker);
fp.init(window, Zotero.getString('pane.item.attachments.select'), nsIFilePicker.modeOpen);
var file = item.getFile(false, true);
var file = item.getFilePath();
if (!file) {
Zotero.debug("Invalid path", 2);
break;
}
var dir = Zotero.File.getClosestDirectory(file);
if (dir) {
dir.QueryInterface(Components.interfaces.nsILocalFile);
@ -4549,7 +4552,7 @@ var ZoteroPane = new function()
continue;
}
item.relinkAttachmentFile(file);
yield item.relinkAttachmentFile(file.path);
break;
}

View file

@ -799,6 +799,30 @@ describe("Zotero.Item", function () {
})
describe("#relinkAttachmentFile", function () {
it("should copy a file elsewhere into the storage directory", function* () {
var filename = 'test.png';
var file = getTestDataDirectory();
file.append(filename);
var tmpDir = yield getTempDirectory();
var tmpFile = OS.Path.join(tmpDir, filename);
yield OS.File.copy(file.path, tmpFile);
file = OS.Path.join(tmpDir, filename);
var item = yield Zotero.Attachments.importFromFile({ file });
let path = yield item.getFilePathAsync();
yield OS.File.remove(path);
yield OS.File.removeEmptyDir(OS.Path.dirname(path));
assert.isFalse(yield item.fileExists());
yield item.relinkAttachmentFile(file);
assert.isTrue(yield item.fileExists());
assert.isTrue(yield OS.File.exists(tmpFile));
});
});
describe("#setTags", function () {
it("should save an array of tags in API JSON format", function* () {
var tags = [