Fix renaming behavior for attachment without extension (#4742)

Fix #4739
This commit is contained in:
Tom Najdek 2024-10-15 12:48:04 +02:00 committed by GitHub
parent 7cf9097467
commit 5bd068860c
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
7 changed files with 83 additions and 18 deletions

View file

@ -50,9 +50,8 @@ Zotero_Preferences.FileRenaming = {
if (selectedItem.isRegularItem() && !selectedItem.parentKey) {
return [selectedItem, this.defaultExt, ''];
}
if (selectedItem.isFileAttachment()) {
let path = selectedItem.getFilePath();
let ext = Zotero.File.getExtension(Zotero.File.pathToFile(path));
if (selectedItem.isFileAttachment() && selectedItem.parentKey) {
let ext = Zotero.Attachments.getCorrectFileExtension(selectedItem);
let parentItem = Zotero.Items.getByLibraryAndKey(selectedItem.libraryID, selectedItem.parentKey);
return [parentItem, ext ?? this.defaultExt, selectedItem.getField('title')];
}

View file

@ -2213,7 +2213,7 @@ Zotero.Attachments = new function () {
throw new Error("'item' must be a Zotero.Item");
}
if (typeof options === 'string') {
Zotero.warn("Zotero.Attachments.getFileBaseNameFromItem(item, formatString) is deprecated -- use Zotero.Attachments(item, options)");
Zotero.warn("Zotero.Attachments.getFileBaseNameFromItem(item, formatString) is deprecated -- use Zotero.Attachments.getFileBaseNameFromItem(item, options)");
options = { formatString: options };
}
@ -2463,7 +2463,24 @@ Zotero.Attachments = new function () {
formatted = Zotero.File.getValidFileName(formatted);
return formatted;
};
/**
* @returns {String} Current file extension for the attachment, if it appears to be a valid file extension.
* Otherwise, attempts to guess the file extension from the attachment's content type.
**/
this.getCorrectFileExtension = function (attachment) {
let path = attachment.getFilePath();
if (!path) {
return '';
}
let ext = Zotero.File.getExtension(path);
ext = Zotero.File.isLikeExtension(ext) ? ext : '';
if (ext === '') {
ext = Zotero.MIME.getPrimaryExtension(attachment.attachmentContentType);
Zotero.Debug.log(`Attachment "${path}": Invalid or missing extension. Guessing from content type: ${ext}`);
}
return ext;
};
this.shouldAutoRenameFile = function (isLink) {
if (!Zotero.Prefs.get('autoRenameFiles')) {

View file

@ -83,6 +83,10 @@ Zotero.File = new function(){
var pos = file.leafName.lastIndexOf('.');
return pos==-1 ? '' : file.leafName.substr(pos+1);
}
this.isLikeExtension = function (extension) {
return !!extension.match(/^\w{1,10}$/i);
}
/**

View file

@ -294,8 +294,8 @@ Zotero.RecognizeDocument = new function () {
// Rename attachment file to match new metadata
if (Zotero.Attachments.shouldAutoRenameAttachment(attachment)) {
let ext = Zotero.File.getExtension(path);
let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: originalTitle });
let ext = Zotero.Attachments.getCorrectFileExtension(attachment);
let newName = fileBaseName + (ext ? '.' + ext : '');
let result = await attachment.renameAttachmentFile(newName, false, true);
if (result !== true) {

View file

@ -5624,8 +5624,8 @@ var ZoteroPane = new function()
Zotero.debug('No path for attachment ' + item.key);
continue;
}
let ext = Zotero.File.getExtension(path);
let fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(item.parentItem, { attachmentTitle: item.getField('title') });
let ext = Zotero.Attachments.getCorrectFileExtension(item);
let newName = fileBaseName + (ext ? '.' + ext : '');
let result = await item.renameAttachmentFile(newName, false, true);
if (result !== true) {
@ -5955,14 +5955,9 @@ var ZoteroPane = new function()
let parentItemID = item.parentItemID;
let parentItem = await Zotero.Items.getAsync(parentItemID);
var oldBaseName = item.attachmentFilename.replace(/\.[^.]+$/, '');
var newName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: item.getField('title') });
let extRE = /\.[^\.]+$/;
let origFilename = PathUtils.split(file).pop();
let ext = origFilename.match(extRE);
if (ext) {
newName = newName + ext[0];
}
var fileBaseName = Zotero.Attachments.getFileBaseNameFromItem(parentItem, { attachmentTitle: item.getField('title') });
let ext = Zotero.Attachments.getCorrectFileExtension(item);
let newName = fileBaseName + (ext ? '.' + ext : '');
var renamed = await item.renameAttachmentFile(newName, false, true);
if (renamed !== true) {

View file

@ -323,6 +323,35 @@ describe("Document Recognition", function() {
'test'
);
});
it("should use mime-type-derived extension for files with invalid extensions", async function () {
if (Zotero.automatedTest) {
this.skip(); // TODO: Mock services
}
this.timeout(30000);
let pdfFile = getTestDataDirectory();
pdfFile.append('recognizePDF_test_title.pdf');
let tmpDir = await getTempDirectory();
let tmpFileToImport = OS.Path.join(tmpDir, 'bad name . not an extension');
await OS.File.copy(pdfFile.path, tmpFileToImport);
var attachment = await Zotero.Attachments.importFromFile({ file: tmpFileToImport });
win.ZoteroPane.recognizeSelected();
var addedIDs = await waitForItemEvent("add");
var modifiedIDs = await waitForItemEvent("modify");
assert.lengthOf(addedIDs, 1);
var item = Zotero.Items.get(addedIDs[0]);
assert.lengthOf(modifiedIDs, 2);
await waitForProgressWindow();
// The file should have been renamed
assert.equal(
attachment.attachmentFilename,
Zotero.Attachments.getFileBaseNameFromItem(item) + '.pdf'
);
});
});
describe("Ebooks", function () {

View file

@ -629,8 +629,8 @@ describe("ZoteroPane", function() {
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 newFilename = 'Test.png';
var uniqueFilename = 'Test 2.png';
var file = getTestDataDirectory();
file.append('test.png');
var tmpDir = await getTempDirectory();
@ -652,7 +652,7 @@ describe("ZoteroPane", function() {
await zp.renameSelectedAttachmentsFromParents();
assert.equal(attachment.attachmentFilename, uniqueFilename);
var path = await attachment.getFilePathAsync();
assert.equal(OS.Path.basename(path), uniqueFilename)
assert.equal(OS.Path.basename(path), uniqueFilename);
await OS.File.exists(path);
});
@ -691,6 +691,27 @@ describe("ZoteroPane", function() {
// After a manual rename, the title becomes the default for this type
assert.equal(attachment.getField('title'), Zotero.getString('file-type-image'));
});
it("should restore an extension when renaming a misnamed file", async function () {
let pdfFile = getTestDataDirectory();
pdfFile.append('test.pdf');
let tmpDir = await getTempDirectory();
let tmpFileToImport = OS.Path.join(tmpDir, 'bad name . not an extension');
await OS.File.copy(pdfFile.path, tmpFileToImport);
var item = createUnsavedDataObject('item');
item.setField('title', 'Title');
await item.saveTx();
let attachment = await Zotero.Attachments.importFromFile({
file: tmpFileToImport,
parentItemID: item.id
});
await zp.selectItem(attachment.id);
await zp.renameSelectedAttachmentsFromParents();
assert.equal(attachment.attachmentFilename, 'Title.pdf');
});
});