diff --git a/chrome/content/zotero/xpcom/storage/storageEngine.js b/chrome/content/zotero/xpcom/storage/storageEngine.js index dd9be82b68..191dd1794c 100644 --- a/chrome/content/zotero/xpcom/storage/storageEngine.js +++ b/chrome/content/zotero/xpcom/storage/storageEngine.js @@ -120,14 +120,19 @@ Zotero.Sync.Storage.Engine.prototype.start = Zotero.Promise.coroutine(function* yield this.controller.cacheCredentials(); } - // Get library last-sync time for download-on-sync libraries. var lastSyncTime = null; var downloadAll = this.local.downloadOnSync(libraryID); - if (downloadAll) { - if (!this.library.storageDownloadNeeded) { - this.library.storageVersion = this.library.libraryVersion; - yield this.library.saveTx(); - } + // After all library data is downloaded during a data sync, the library version is updated to match + // the server version. We track a library version for file syncing separately, so that even if Zotero + // is closed or interrupted between a data sync and a file sync, we know that file syncing has to be + // performed for any files marked for download during data sync (based on outdated mtime/md5). Files + // may be missing remotely, though, so it's only necessary to try to download them once every time + // there are remote storage changes, which we indicate with a flag set in syncLocal. + // + // TODO: If files are persistently missing, don't try to download them each time + if (downloadAll && !this.library.storageDownloadNeeded) { + this.library.storageVersion = this.library.libraryVersion; + yield this.library.saveTx(); } // Check for updated files to upload diff --git a/chrome/content/zotero/xpcom/sync/syncLocal.js b/chrome/content/zotero/xpcom/sync/syncLocal.js index 02fa20c93a..b2357242ee 100644 --- a/chrome/content/zotero/xpcom/sync/syncLocal.js +++ b/chrome/content/zotero/xpcom/sync/syncLocal.js @@ -611,6 +611,18 @@ Zotero.Sync.Data.Local = { if (obj) { Zotero.debug("Matching local " + objectType + " exists", 4); + let jsonDataLocal = obj.toJSON(); + + // For items, check if mtime or file hash changed in metadata, + // which would indicate that a remote storage sync took place and + // a download is needed + if (objectType == 'item' && obj.isImportedAttachment()) { + if (jsonDataLocal.mtime != jsonData.mtime + || jsonDataLocal.md5 != jsonData.md5) { + saveOptions.storageDetailsChanged = true; + } + } + // Local object has been modified since last sync if (!obj.synced) { Zotero.debug("Local " + objectType + " " + obj.libraryKey @@ -620,18 +632,6 @@ Zotero.Sync.Data.Local = { objectType, obj.libraryID, obj.key, obj.version ); - let jsonDataLocal = obj.toJSON(); - - // For items, check if mtime or file hash changed in metadata, - // which would indicate that a remote storage sync took place and - // a download is needed - if (objectType == 'item' && obj.isImportedAttachment()) { - if (jsonDataLocal.mtime != jsonData.mtime - || jsonDataLocal.md5 != jsonData.md5) { - saveOptions.storageDetailsChanged = true; - } - } - let result = this._reconcileChanges( objectType, cachedJSON.data, @@ -701,7 +701,7 @@ Zotero.Sync.Data.Local = { else { Zotero.debug(ObjectType + " doesn't exist locally"); - isNewObject = true; + saveOptions.isNewObject = true; // Check if object has been deleted locally let dateDeleted = yield this.getDateDeleted( diff --git a/test/tests/syncLocalTest.js b/test/tests/syncLocalTest.js index 5ee876476c..15a6d6fb88 100644 --- a/test/tests/syncLocalTest.js +++ b/test/tests/syncLocalTest.js @@ -275,8 +275,9 @@ describe("Zotero.Sync.Data.Local", function() { assert.notInclude(versions, key); }); - it("should mark new attachment items for download", function* () { - var libraryID = Zotero.Libraries.userLibraryID; + it("should mark new attachment items and library for download", function* () { + var library = Zotero.Libraries.userLibrary; + var libraryID = library.id; Zotero.Sync.Storage.Local.setModeForLibrary(libraryID, 'zfs'); var key = Zotero.DataObjectUtilities.generateKey(); @@ -299,10 +300,12 @@ describe("Zotero.Sync.Data.Local", function() { ); var item = Zotero.Items.getByLibraryAndKey(libraryID, key); assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD); + assert.isTrue(library.storageDownloadNeeded); }) it("should mark updated attachment items for download", function* () { - var libraryID = Zotero.Libraries.userLibraryID; + var library = Zotero.Libraries.userLibrary; + var libraryID = library.id; Zotero.Sync.Storage.Local.setModeForLibrary(libraryID, 'zfs'); var item = yield importFileAttachment('test.png'); @@ -327,6 +330,7 @@ describe("Zotero.Sync.Data.Local", function() { ); assert.equal(item.attachmentSyncState, Zotero.Sync.Storage.Local.SYNC_STATE_TO_DOWNLOAD); + assert.isTrue(library.storageDownloadNeeded); }) it("should ignore attachment metadata when resolving metadata conflict", function* () {