All: Fixed: Prevent app from trying to upload resource it has not downloaded yet

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);
+	}));
+
 });

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);
+		if (!resource) {
-		const localState = await Resource.localState(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();
 				}

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 localState = await Resource.localState(local.id);
-								const remoteContentPath = this.resourceDirName_ + "/" + local.id;
+							if (localState.fetch_status !== Resource.FETCH_STATUS_DONE) {
-								const result = await Resource.fullPathForSyncUpload(local);
+								action = null;
-								local = result.resource;
+							} else {
-								const localResourceContentPath = result.path;
+								try {
-								await this.api().put(remoteContentPath, null, { path: localResourceContentPath, source: "file" });
+									const remoteContentPath = this.resourceDirName_ + "/" + local.id;
-							} catch (error) {
+									const result = await Resource.fullPathForSyncUpload(local);
-								if (error && ["rejectedByTarget", "fileNotFound"].indexOf(error.code) >= 0) {
+									local = result.resource;
-									await handleCannotSyncItem(ItemClass, syncTargetId, local, error.message);
+									const localResourceContentPath = result.path;
-									action = null;
+									await this.api().put(remoteContentPath, null, { path: localResourceContentPath, source: "file" });
-								} else {
+								} catch (error) {
-									throw 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;