Handle resource conflicts

CliClient/tests/synchronizer.js

@@ -1007,7 +1007,7 @@ describe('synchronizer', function() {
 		let resource1_2 = (await Resource.all())[0];
 		const modFile = `${__dirname}/tmp/test_mod.txt`;
 		await shim.fsDriver().writeFile(modFile, '1234 MOD', 'utf8');
-		await shim.updateResourceBlob(resource1_2.id, modFile);
+		await Resource.updateResourceBlobContent(resource1_2.id, modFile);
 		const originalSize = resource1_2.size;
 		resource1_2 = (await Resource.all())[0];
 		const newSize = resource1_2.size;
@@ -1022,9 +1022,67 @@ describe('synchronizer', function() {
 		await resourceFetcher().waitForAllFinished();
 		const resource1_1 = (await Resource.all())[0];
 		expect(resource1_1.size).toBe(newSize);
-		const resource1_1Path = Resource.fullPath(resource1_1);
-		const newContent = await shim.fsDriver().readFile(resource1_1Path, 'utf8');
-		expect(newContent).toBe('1234 MOD');
+		expect(await Resource.resourceBlobContent(resource1_1.id, 'utf8')).toBe('1234 MOD');
+	}));
+
+	it('should handle resource conflicts', asyncTest(async () => {
+		{
+			const tempFile = `${__dirname}/tmp/test.txt`;
+			await shim.fsDriver().writeFile(tempFile, '1234', 'utf8');
+			const folder1 = await Folder.save({ title: 'folder1' });
+			const note1 = await Note.save({ title: 'ma note', parent_id: folder1.id });
+			await shim.attachFileToNote(note1, tempFile);
+			await synchronizer().start();
+		}
+
+		await switchClient(2);
+
+		{
+			await synchronizer().start();
+			await resourceFetcher().start();
+			await resourceFetcher().waitForAllFinished();
+			const resource = (await Resource.all())[0];
+			const modFile2 = `${__dirname}/tmp/test_mod_2.txt`;
+			await shim.fsDriver().writeFile(modFile2, '1234 MOD 2', 'utf8');
+			await Resource.updateResourceBlobContent(resource.id, modFile2);
+			await synchronizer().start();
+		}
+
+		await switchClient(1);
+
+		{
+			// Going to modify a resource without syncing first, which will cause a conflict
+			const resource = (await Resource.all())[0];
+			const modFile1 = `${__dirname}/tmp/test_mod_1.txt`;
+			await shim.fsDriver().writeFile(modFile1, '1234 MOD 1', 'utf8');
+			await Resource.updateResourceBlobContent(resource.id, modFile1);
+			await synchronizer().start(); // CONFLICT
+
+			// If we try to read the resource content now, it should throw because the local
+			// content has been moved to the conflict notebook, and the new local content
+			// has not been downloaded yet.
+			await checkThrowAsync(async () => await Resource.resourceBlobContent(resource.id));
+
+			// Now download resources, and our local content would have been overwritten by
+			// the content from client 2
+			await resourceFetcher().start();
+			await resourceFetcher().waitForAllFinished();
+			const localContent =  await Resource.resourceBlobContent(resource.id, 'utf8');
+			expect(localContent).toBe('1234 MOD 2');
+
+			// Check that the Conflict note has been generated, with the conflict resource
+			// attached to it, and check that it has the original content.
+			const allNotes = await Note.all();
+			expect(allNotes.length).toBe(2);
+			const conflictNote = allNotes.find((v) => {
+				return !!v.is_conflict;
+			});
+			expect(!!conflictNote).toBe(true);
+			const resourceIds = await Note.linkedResourceIds(conflictNote.body);
+			expect(resourceIds.length).toBe(1);
+			const conflictContent =  await Resource.resourceBlobContent(resourceIds[0], 'utf8');
+			expect(conflictContent).toBe('1234 MOD 1');
+		}
 	}));
 
 	it('should upload decrypted items to sync target after encryption disabled', asyncTest(async () => {

README.md

@@ -69,6 +69,7 @@ The Web Clipper is a browser extension that allows you to save web pages and scr
 	- [Joplin Forum](https://discourse.joplinapp.org)
 	- [Markdown Guide](https://github.com/laurent22/joplin/blob/master/readme/markdown.md)
 	- [How to enable end-to-end encryption](https://github.com/laurent22/joplin/blob/master/readme/e2ee.md)
+	- [What is a conflict?](https://github.com/laurent22/joplin/blob/master/readme/conflict.md)
 	- [End-to-end encryption spec](https://github.com/laurent22/joplin/blob/master/readme/spec.md)
 	- [How to enable debug mode](https://github.com/laurent22/joplin/blob/master/readme/debugging.md)
 	- [API documentation](https://github.com/laurent22/joplin/blob/master/readme/api.md)

ReactNativeClient/lib/models/Resource.js

@@ -122,6 +122,11 @@ class Resource extends BaseItem {
 		return 'ok';
 	}
 
+	static async requireIsReady(resource) {
+		const readyStatus = await Resource.readyStatus(resource);
+		if (readyStatus !== 'ok') throw new Error(`Resource is not ready. Status: ${readyStatus}`);
+	}
+
 	// For resources, we need to decrypt the item (metadata) and the resource binary blob.
 	static async decrypt(item) {
 		// The item might already be decrypted but not the blob (for instance if it crashes while
@@ -236,7 +241,7 @@ class Resource extends BaseItem {
 		return url.substr(2);
 	}
 
-	static localState(resourceOrId) {
+	static async localState(resourceOrId) {
 		return ResourceLocalState.byResourceId(typeof resourceOrId === 'object' ? resourceOrId.id : resourceOrId);
 	}
 
@@ -315,6 +320,25 @@ class Resource extends BaseItem {
 		throw new Error(`Invalid status: ${status}`);
 	}
 
+	static async updateResourceBlobContent(resourceId, newBlobFilePath) {
+		const resource = await Resource.load(resourceId);
+		await this.requireIsReady(resource);
+
+		const fileStat = await this.fsDriver().stat(newBlobFilePath);
+		await this.fsDriver().copy(newBlobFilePath, Resource.fullPath(resource));
+
+		await Resource.save({
+			id: resource.id,
+			size: fileStat.size,
+		});
+	}
+
+	static async resourceBlobContent(resourceId, encoding = 'Buffer') {
+		const resource = await Resource.load(resourceId);
+		await this.requireIsReady(resource);
+		return await this.fsDriver().readFile(Resource.fullPath(resource), encoding);
+	}
+
 }
 
 Resource.IMAGE_MAX_DIMENSION = 1920;

ReactNativeClient/lib/services/ResourceEditWatcher.ts

@@ -77,7 +77,7 @@ export default class ResourceEditWatcher {
 		const makeSaveAction = (resourceId:string, path:string) => {
 			return async () => {
 				this.logger().info(`ResourceEditWatcher: Saving resource ${resourceId}`);
-				await shim.updateResourceBlob(resourceId, path);
+				await Resource.updateResourceBlob(resourceId, path);
 				this.eventEmitter_.emit('resourceChange', { id: resourceId });
 			};
 		};

ReactNativeClient/lib/shim-init-node.js

@@ -205,18 +205,26 @@ function shimInit() {
 		return Resource.save(resource, { isNew: true });
 	};
 
-	shim.updateResourceBlob = async function(resourceId, newBlobFilePath) {
+	shim.duplicateResource = async function(resourceId) {
 		const resource = await Resource.load(resourceId);
-		const readyStatus = await Resource.readyStatus(resourceId);
-		if (readyStatus !== 'ok') throw new Error(`Cannot set resource blob because resource is not ready. Status: ${readyStatus}`);
+		const localState = await Resource.localState(resource);
 
-		const fileStat = await shim.fsDriver().stat(newBlobFilePath);
-		await shim.fsDriver().copy(newBlobFilePath, Resource.fullPath(resource));
+		let newResource = { ...resource };
+		delete newResource.id;
+		newResource = await Resource.save(newResource);
 
-		await Resource.save({
-			id: resource.id,
-			size: fileStat.size,
-		});
+		const newLocalState = { ...localState };
+		newLocalState.resource_id = newResource.id;
+		delete newLocalState.id;
+
+		await Resource.setLocalState(newResource, newLocalState);
+
+		const sourcePath = Resource.fullPath(resource);
+		if (await shim.fsDriver().exists(sourcePath)) {
+			await shim.fsDriver().copy(sourcePath, Resource.fullPath(newResource));
+		}
+
+		return newResource;
 	};
 
 	shim.attachFileToNoteBody = async function(noteBody, filePath, position = null, options = null) {

ReactNativeClient/lib/synchronizer.js

@@ -372,6 +372,12 @@ class Synchronizer {
 						let reason = '';
 						let remoteContent = null;
 
+						const getConflictType = (conflictedItem) => {
+							if (conflictedItem.type_ === BaseModel.TYPE_NOTE) return 'noteConflict';
+							if (conflictedItem.type_ === BaseModel.TYPE_RESOURCE) return 'resourceConflict';
+							return 'itemConflict';
+						};
+
 						if (!remote) {
 							if (!local.sync_time) {
 								action = 'createRemote';
@@ -379,7 +385,7 @@ class Synchronizer {
 							} else {
 								// Note or item was modified after having been deleted remotely
 								// "itemConflict" is for all the items except the notes, which are dealt with in a special way
-								action = local.type_ == BaseModel.TYPE_NOTE ? 'noteConflict' : 'itemConflict';
+								action = getConflictType(local);
 								reason = 'remote has been deleted, but local has changes';
 							}
 						} else {
@@ -416,7 +422,7 @@ class Synchronizer {
 								// Since, in this loop, we are only dealing with items that require sync, if the
 								// remote has been modified after the sync time, it means both items have been
 								// modified and so there's a conflict.
-								action = local.type_ == BaseModel.TYPE_NOTE ? 'noteConflict' : 'itemConflict';
+								action = getConflictType(local);
 								reason = 'both remote and local have changes';
 							} else {
 								action = 'updateRemote';
@@ -528,8 +534,29 @@ class Synchronizer {
 								conflictedNote.is_conflict = 1;
 								await Note.save(conflictedNote, { autoTimestamp: false, changeSource: ItemChange.SOURCE_SYNC });
 							}
-
+						} else if (action == 'resourceConflict') {
 							// ------------------------------------------------------------------------------
+							// Unlike notes we always handle the conflict for resources
+							// ------------------------------------------------------------------------------
+
+							const conflictResource = await shim.duplicateResource(local.id);
+
+							await Note.save({
+								title: _('Attachment conflict: "%s"', local.title),
+								body: _('There was a [conflict](%s) on the attachment below.\n\n%s', 'https://joplinapp.org/conflict', Resource.markdownTag(conflictResource)),
+								is_conflict: 1,
+							}, { changeSource: ItemChange.SOURCE_SYNC });
+
+							// The local content we have is no longer valid and should be re-downloaded
+							await Resource.setLocalState(local.id, {
+								fetch_status: Resource.FETCH_STATUS_IDLE,
+							});
+						}
+
+						if (['noteConflict', 'resourceConflict'].includes(action)) {
+							// ------------------------------------------------------------------------------
+							// For note and resource conflicts, the creation of the conflict item is done
+							// differently. However the way the local content is handled is the same.
 							// Either copy the remote content to local or, if the remote content has
 							// been deleted, delete the local content.
 							// ------------------------------------------------------------------------------

readme/conflict.md

@@ -0,0 +1,17 @@
+# What is a conflict?
+
+A conflict happens when one note or one attachment is modified in two different places, and then synchronised. In that case, it not possible to determine which version of the note or attachment you want to keep, and thus a conflict is generated.
+
+# What happens in case of a conflict?
+
+When Joplin detects a conflict, the local note is copied to the Conflict notebook so as to avoid any data loss. Then the remote note is downloaded. You can then inspect the notes in the Conflict notebook, compare it with your other version, and copy any change that might have been overwritten.
+
+# How to avoid conflicts?
+
+Conflicts are always annoying to deal with so it is best to avoid them as much as possible.
+
+For this, the best way is to synchronise as often as possible, so that you are always working with the latest versions of your notes.
+
+Joplin attempts to do this by uploading your latest changes within a few seconds. However, downloading changes is done at fixed intervals, every few minutes (as defined in the Config screen) and this is where conflicts may happen. It can also happen if one of your device did not have an internet connection for some times, and then synchronises. A bad internet connection can also hinder synchronisation because it will interrupt the process, which may have to restarted from the beginning to ensure consistency.
+
+So if you have not opened your application in a while, manually sync it and wait for it to complete, that way you are sure that whatever change you make will be on the latest version of the note.