diff --git a/CliClient/tests/synchronizer.js b/CliClient/tests/synchronizer.js index 2e6c670ffc..655e435171 100644 --- a/CliClient/tests/synchronizer.js +++ b/CliClient/tests/synchronizer.js @@ -53,6 +53,10 @@ async function remoteNotesFoldersResources() { return remoteItemsByTypes([BaseModel.TYPE_NOTE, BaseModel.TYPE_FOLDER, BaseModel.TYPE_RESOURCE]); } +async function remoteResources() { + return remoteItemsByTypes([BaseModel.TYPE_RESOURCE]); +} + async function localNotesFoldersSameAsRemote(locals, expect) { let error = null; try { @@ -861,7 +865,7 @@ describe('Synchronizer', function() { expect(ls.fetch_status).toBe(Resource.FETCH_STATUS_IDLE); const fetcher = new ResourceFetcher(() => { return synchronizer().api() }); - fetcher.queueDownload(resource1_2.id); + fetcher.queueDownload_(resource1_2.id); await fetcher.waitForAllFinished(); resource1_2 = await Resource.load(resource1.id); @@ -1320,4 +1324,31 @@ describe('Synchronizer', function() { expect(syncItems[1].sync_disabled).toBe(1); })); + it("should not upload a resource if it has not been fetched yet", asyncTest(async () => { + // In some rare cases, the synchronizer might try to upload a resource even though it + // doesn't have the resource file. It can happen in this situation: + // - C1 create resource + // - C1 sync + // - C2 sync + // - C2 resource metadata is received but ResourceFetcher hasn't downloaded the file yet + // - C2 enables E2EE - all the items are marked for forced sync + // - C2 sync + // The synchronizer will try to upload the resource, even though it doesn't have the file, + // so we need to make sure it doesn't. But also that once it gets the file, the resource + // does get uploaded. + + const note1 = await Note.save({ title: 'note' }); + await shim.attachFileToNote(note1, __dirname + '/../tests/support/photo.jpg'); + const resource = (await Resource.all())[0]; + await Resource.setLocalState(resource.id, { fetch_status: Resource.FETCH_STATUS_IDLE }); + await synchronizer().start(); + + expect((await remoteResources()).length).toBe(0); + + await Resource.setLocalState(resource.id, { fetch_status: Resource.FETCH_STATUS_DONE }); + await synchronizer().start(); + + expect((await remoteResources()).length).toBe(1); + })); + }); diff --git a/ReactNativeClient/lib/services/ResourceFetcher.js b/ReactNativeClient/lib/services/ResourceFetcher.js index f53e28c620..021cb71b87 100644 --- a/ReactNativeClient/lib/services/ResourceFetcher.js +++ b/ReactNativeClient/lib/services/ResourceFetcher.js @@ -99,12 +99,15 @@ class ResourceFetcher extends BaseService { if (this.fetchingItems_[resourceId]) return; this.fetchingItems_[resourceId] = true; + const resource = await Resource.load(resourceId); + const localState = await Resource.localState(resource); + const completeDownload = async (emitDownloadComplete = true, localResourceContentPath = '') => { // 2019-05-12: This is only necessary to set the file size of the resources that come via // sync. The other ones have been done using migrations/20.js. This code can be removed // after a few months. - if (resource.size < 0 && localResourceContentPath && !resource.encryption_blob_encrypted) { + if (resource && resource.size < 0 && localResourceContentPath && !resource.encryption_blob_encrypted) { await ResourceService.autoSetFileSizes(); } @@ -119,8 +122,11 @@ class ResourceFetcher extends BaseService { this.updateReport(); } - const resource = await Resource.load(resourceId); - const localState = await Resource.localState(resource); + if (!resource) { + this.logger().info('ResourceFetcher: Attempting to download a resource that does not exist (has been deleted?): ' + resourceId); + await completeDownload(false); + return; + } // Shouldn't happen, but just to be safe don't re-download the // resource if it's already been downloaded. @@ -166,7 +172,7 @@ class ResourceFetcher extends BaseService { async waitForAllFinished() { return new Promise((resolve, reject) => { const iid = setInterval(() => { - if (!this.queue_.length && !Object.getOwnPropertyNames(this.fetchingItems_).length) { + if (!this.updateReportIID_ && !this.scheduleQueueProcessIID_ && !this.addingResources_ && !this.queue_.length && !Object.getOwnPropertyNames(this.fetchingItems_).length) { clearInterval(iid); resolve(); } diff --git a/ReactNativeClient/lib/synchronizer.js b/ReactNativeClient/lib/synchronizer.js index 2919f369db..e6321818ae 100644 --- a/ReactNativeClient/lib/synchronizer.js +++ b/ReactNativeClient/lib/synchronizer.js @@ -231,6 +231,11 @@ class Synchronizer { if (syncSteps.indexOf("update_remote") >= 0) { let donePaths = []; + + const completeItemProcessing = (path) => { + donePaths.push(path); + } + while (true) { if (this.cancelling()) break; @@ -290,6 +295,7 @@ class Synchronizer { if (error.code === 'rejectedByTarget') { this.progressReport_.errors.push(error); this.logger().warn('Rejected by target: ' + path + ': ' + error.message); + completeItemProcessing(path); continue; } else { throw error; @@ -313,18 +319,23 @@ class Synchronizer { this.logSyncOperation(action, local, remote, reason); if (local.type_ == BaseModel.TYPE_RESOURCE && (action == "createRemote" || action === "updateRemote" || (action == "itemConflict" && remote))) { - try { - const remoteContentPath = this.resourceDirName_ + "/" + local.id; - const result = await Resource.fullPathForSyncUpload(local); - local = result.resource; - const localResourceContentPath = result.path; - await this.api().put(remoteContentPath, null, { path: localResourceContentPath, source: "file" }); - } catch (error) { - if (error && ["rejectedByTarget", "fileNotFound"].indexOf(error.code) >= 0) { - await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message); - action = null; - } else { - throw error; + const localState = await Resource.localState(local.id); + if (localState.fetch_status !== Resource.FETCH_STATUS_DONE) { + action = null; + } else { + try { + const remoteContentPath = this.resourceDirName_ + "/" + local.id; + const result = await Resource.fullPathForSyncUpload(local); + local = result.resource; + const localResourceContentPath = result.path; + await this.api().put(remoteContentPath, null, { path: localResourceContentPath, source: "file" }); + } catch (error) { + if (error && ["rejectedByTarget", "fileNotFound"].indexOf(error.code) >= 0) { + await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message); + action = null; + } else { + throw error; + } } } } @@ -422,7 +433,7 @@ class Synchronizer { } } - donePaths.push(path); + completeItemProcessing(path); } if (!result.hasMore) break;