From ec59f43a6864f063e41a17b70cc3868600b7d85a Mon Sep 17 00:00:00 2001 From: Dan Stillman Date: Mon, 14 Apr 2025 04:12:06 -0400 Subject: [PATCH] Mark locally missing files marked for upload for download instead Z7 removed an `OS.File.open()` in `_checkForUpdatedFiles()` that would throw on missing files and cause them to be marked for download in a `catch`. This likely caused the fix for #1753 not to work in Z7. --- .../zotero/xpcom/storage/storageLocal.js | 20 +++++++------- test/tests/storageLocalTest.js | 27 ++++++++++++++++++- 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/chrome/content/zotero/xpcom/storage/storageLocal.js b/chrome/content/zotero/xpcom/storage/storageLocal.js index a33c9be365..43e39cd477 100644 --- a/chrome/content/zotero/xpcom/storage/storageLocal.js +++ b/chrome/content/zotero/xpcom/storage/storageLocal.js @@ -330,21 +330,21 @@ Zotero.Sync.Storage.Local = { var fileName = PathUtils.filename(path); try { - // If file is already marked for upload, skip check. Even if the file was changed - // both locally and remotely, conflicts are checked at upload time, so we don't need - // to worry about it here. - // - // This is after open() so that a missing file is properly marked for download. - if (item.attachmentSyncState == this.SYNC_STATE_TO_UPLOAD) { - Zotero.debug("File is already marked for upload"); - return false; - } - let { lastModified: fmtime } = yield IOUtils.stat(path); //Zotero.debug("Memory usage: " + memmgr.resident); //Zotero.debug("File modification time for item " + lk + " is " + fmtime); + // If file is already marked for upload, skip check. Even if the file was changed + // both locally and remotely, conflicts are checked at upload time, so we don't need + // to worry about it here. + // + // This is after stat() so that a missing file is properly marked for download. + if (item.attachmentSyncState == this.SYNC_STATE_TO_UPLOAD) { + Zotero.debug("File is already marked for upload"); + return false; + } + if (fmtime < 0) { Zotero.debug("File mod time " + fmtime + " is less than 0 -- interpreting as 0", 2); fmtime = 0; diff --git a/test/tests/storageLocalTest.js b/test/tests/storageLocalTest.js index f24d977cd3..0ac034951e 100644 --- a/test/tests/storageLocalTest.js +++ b/test/tests/storageLocalTest.js @@ -90,7 +90,32 @@ describe("Zotero.Sync.Storage.Local", function () { assert.isFalse(changed); assert.equal(syncState, Zotero.Sync.Storage.Local.SYNC_STATE_IN_SYNC); assert.equal(syncedModTime, newModTime); - }) + }); + + it("should flag a missing local file for download if already marked for upload", async function () { + // Create attachment + let item = await importFileAttachment('test.png'); + var hash = await item.attachmentHash; + var mtime = await item.attachmentModificationTime; + + // Mark as synced, so it will be checked + item.attachmentSyncedModificationTime = mtime; + item.attachmentSyncedHash = hash; + item.attachmentSyncState = "to_upload"; + await item.saveTx({ skipAll: true }); + + // Delete local file + await IOUtils.remove(item.getFilePath()); + + // File should be marked for download and not returned + var libraryID = Zotero.Libraries.userLibraryID; + var changed = await Zotero.Sync.Storage.Local.checkForUpdatedFiles(libraryID, [item.id]); + + await item.eraseTx(); + + assert.isTrue(changed); + assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD); + }); }) describe("#updateSyncStates()", function () {