You've already forked joplin
							
							
				mirror of
				https://github.com/laurent22/joplin.git
				synced 2025-10-31 00:07:48 +02:00 
			
		
		
		
	Fixed synchronizer conflict handling when deleting folders
This commit is contained in:
		| @@ -322,6 +322,53 @@ describe('Synchronizer', function() { | ||||
| 		done(); | ||||
| 	}); | ||||
|  | ||||
| 	it('should resolve conflict if remote folder has been deleted, but note has been added to folder locally', async (done) => { | ||||
| 		let folder1 = await Folder.save({ title: "folder1" }); | ||||
| 		await synchronizer().start(); | ||||
|  | ||||
| 		await switchClient(2); | ||||
|  | ||||
| 		await synchronizer().start(); | ||||
| 		await Folder.delete(folder1.id); | ||||
| 		await synchronizer().start(); | ||||
|  | ||||
| 		await switchClient(1); | ||||
|  | ||||
| 		let note = await Note.save({ title: "note1", parent_id: folder1.id }); | ||||
| 		await synchronizer().start(); | ||||
| 		let items = await allItems(); | ||||
| 		expect(items.length).toBe(1); | ||||
| 		expect(items[0].title).toBe('note1'); | ||||
| 		expect(items[0].is_conflict).toBe(1); | ||||
| 		 | ||||
| 		done(); | ||||
| 	}); | ||||
|  | ||||
| 	it('should resolve conflict if note has been deleted remotely and locally', async (done) => { | ||||
| 		let folder = await Folder.save({ title: "folder" }); | ||||
| 		let note = await Note.save({ title: "note", parent_id: folder.title }); | ||||
| 		await synchronizer().start(); | ||||
|  | ||||
| 		await switchClient(2); | ||||
|  | ||||
| 		await synchronizer().start(); | ||||
| 		await Note.delete(note.id); | ||||
| 		await synchronizer().start(); | ||||
|  | ||||
| 		await switchClient(1); | ||||
|  | ||||
| 		await Note.delete(note.id); | ||||
| 		await synchronizer().start(); | ||||
|  | ||||
| 		let items = await allItems(); | ||||
| 		expect(items.length).toBe(1); | ||||
| 		expect(items[0].title).toBe('folder'); | ||||
|  | ||||
| 		localItemsSameAsRemote(items, expect); | ||||
| 		 | ||||
| 		done(); | ||||
| 	}); | ||||
|  | ||||
| 	it('should cross delete all folders', async (done) => { | ||||
| 		// If client1 and 2 have two folders, client 1 deletes item 1 and client | ||||
| 		// 2 deletes item 2, they should both end up with no items after sync. | ||||
|   | ||||
| @@ -47,7 +47,7 @@ class LogScreenComponent extends React.Component { | ||||
| 			}; | ||||
| 			return ( | ||||
| 				<View style={{flexDirection: 'row', paddingLeft: 1, paddingRight: 1, paddingTop:0, paddingBottom:0 }}> | ||||
| 					<Text style={style}>{time.unixMsToIsoSec(item.timestamp) + ': ' + item.message}</Text> | ||||
| 					<Text style={style}>{time.formatMsToLocal(item.timestamp, 'MM-DDTHH:mm') + ': ' + item.message}</Text> | ||||
| 				</View> | ||||
| 			); | ||||
| 		} | ||||
|   | ||||
| @@ -1,5 +1,5 @@ | ||||
| import React, { Component } from 'react'; | ||||
| import { View, Button, TextInput, WebView } from 'react-native'; | ||||
| import { View, Button, TextInput, WebView, Text } from 'react-native'; | ||||
| import { connect } from 'react-redux' | ||||
| import { Log } from 'lib/log.js' | ||||
| import { Note } from 'lib/models/note.js' | ||||
| @@ -19,6 +19,8 @@ class NoteScreenComponent extends React.Component { | ||||
| 		this.state = { | ||||
| 			note: Note.new(), | ||||
| 			mode: 'view', | ||||
| 			noteMetadata: '', | ||||
| 			showNoteMetadata: false, | ||||
| 		} | ||||
| 	} | ||||
|  | ||||
| @@ -26,9 +28,11 @@ class NoteScreenComponent extends React.Component { | ||||
| 		if (!this.props.noteId) { | ||||
| 			let note = this.props.itemType == 'todo' ? Note.newTodo(this.props.folderId) : Note.new(this.props.folderId); | ||||
| 			this.setState({ note: note }); | ||||
| 			this.refreshNoteMetadata(); | ||||
| 		} else { | ||||
| 			Note.load(this.props.noteId).then((note) => { | ||||
| 				this.setState({ note: note }); | ||||
| 				this.refreshNoteMetadata(); | ||||
| 			}); | ||||
| 		} | ||||
| 	} | ||||
| @@ -41,6 +45,13 @@ class NoteScreenComponent extends React.Component { | ||||
| 		}); | ||||
| 	} | ||||
|  | ||||
| 	async refreshNoteMetadata(force = null) { | ||||
| 		if (force !== true && !this.state.showNoteMetadata) return; | ||||
|  | ||||
| 		let noteMetadata = await Note.serializeAllProps(this.state.note); | ||||
| 		this.setState({ noteMetadata: noteMetadata }); | ||||
| 	} | ||||
|  | ||||
| 	title_changeText(text) { | ||||
| 		this.noteComponent_change('title', text); | ||||
| 	} | ||||
| @@ -54,6 +65,7 @@ class NoteScreenComponent extends React.Component { | ||||
| 		let note = await Note.save(this.state.note); | ||||
| 		this.setState({ note: note }); | ||||
| 		if (isNew) Note.updateGeolocation(note.id); | ||||
| 		this.refreshNoteMetadata(); | ||||
| 	} | ||||
|  | ||||
| 	deleteNote_onPress(noteId) { | ||||
| @@ -61,12 +73,19 @@ class NoteScreenComponent extends React.Component { | ||||
| 	} | ||||
|  | ||||
| 	attachFile_onPress(noteId) { | ||||
|  | ||||
| 	} | ||||
|  | ||||
| 	showMetadata_onPress() { | ||||
| 		this.setState({ showNoteMetadata: !this.state.showNoteMetadata }); | ||||
| 		this.refreshNoteMetadata(true); | ||||
| 	} | ||||
|  | ||||
| 	menuOptions() { | ||||
| 		return [ | ||||
| 			{ title: _('Attach file'), onPress: () => { this.attachFile_onPress(this.state.note.id); } }, | ||||
| 			{ title: _('Delete note'), onPress: () => { this.deleteNote_onPress(this.state.note.id); } }, | ||||
| 			{ title: _('Toggle metadata'), onPress: () => { this.showMetadata_onPress(); } }, | ||||
| 		]; | ||||
| 	} | ||||
|  | ||||
| @@ -99,15 +118,18 @@ class NoteScreenComponent extends React.Component { | ||||
| 			bodyComponent = <TextInput style={{flex: 1, textAlignVertical: 'top', fontFamily: 'monospace'}} multiline={true} value={note.body} onChangeText={(text) => this.body_changeText(text)} /> | ||||
| 		} | ||||
|  | ||||
| 		console.info(this.state.noteMetadata); | ||||
|  | ||||
| 		return ( | ||||
| 			<View style={{flex: 1}}> | ||||
| 				<ScreenHeader navState={this.props.navigation.state} menuOptions={this.menuOptions()} /> | ||||
| 				<View style={{ flexDirection: 'row' }}> | ||||
| 					{ isTodo && <Checkbox checked={!!Number(note.todo_completed)} /> }<TextInput style={{flex:1}} value={note.title} onChangeText={(text) => this.title_changeText(text)} /> | ||||
| 				</View> | ||||
| 				{ bodyComponent }		 | ||||
| 				{ bodyComponent } | ||||
| 				{ todoComponents } | ||||
| 				<Button title="Save note" onPress={() => this.saveNoteButton_press()} /> | ||||
| 				{ this.state.showNoteMetadata && <Text>{this.state.noteMetadata}</Text> } | ||||
| 			</View> | ||||
| 		); | ||||
| 	} | ||||
|   | ||||
| @@ -28,7 +28,7 @@ class NotesScreenUtils { | ||||
| 		} else { | ||||
| 			this.dispatch({ | ||||
| 				type: 'Navigation/NAVIGATE', | ||||
| 				routeName: 'Loading', | ||||
| 				routeName: 'Welcome', | ||||
| 			}); | ||||
| 		} | ||||
| 	} | ||||
|   | ||||
| @@ -5,7 +5,7 @@ import { Log } from 'lib/log.js' | ||||
| import { ScreenHeader } from 'lib/components/screen-header.js'; | ||||
| import { ActionButton } from 'lib/components/action-button.js'; | ||||
| 
 | ||||
| class LoadingScreenComponent extends React.Component { | ||||
| class WelcomeScreenComponent extends React.Component { | ||||
| 	 | ||||
| 	static navigationOptions(options) { | ||||
| 		return { header: null }; | ||||
| @@ -31,12 +31,12 @@ class LoadingScreenComponent extends React.Component { | ||||
| 
 | ||||
| } | ||||
| 
 | ||||
| const LoadingScreen = connect( | ||||
| const WelcomeScreen = connect( | ||||
| 	(state) => { | ||||
| 		return { | ||||
| 			loading: state.loading, | ||||
| 		}; | ||||
| 	} | ||||
| )(LoadingScreenComponent) | ||||
| )(WelcomeScreenComponent) | ||||
| 
 | ||||
| export { LoadingScreen }; | ||||
| export { WelcomeScreen }; | ||||
| @@ -213,8 +213,15 @@ class Database { | ||||
| 		} | ||||
|  | ||||
| 		if (typeof where != 'string') { | ||||
| 			params.push(where.id); | ||||
| 			where = 'id=?'; | ||||
| 			let s = []; | ||||
| 			for (let n in where) { | ||||
| 				if (!where.hasOwnProperty(n)) continue; | ||||
| 				params.push(where[n]); | ||||
| 				s.push('`' + n + '`=?'); | ||||
| 			} | ||||
| 			where = s.join(' AND '); | ||||
| 			// params.push(where.id); | ||||
| 			// where = 'id=?'; | ||||
| 		} | ||||
|  | ||||
| 		return { | ||||
|   | ||||
| @@ -177,7 +177,7 @@ class BaseItem extends BaseModel { | ||||
| 	} | ||||
|  | ||||
| 	static serialize_format(propName, propValue) { | ||||
| 		if (['created_time', 'updated_time'].indexOf(propName) >= 0) { | ||||
| 		if (['created_time', 'updated_time', 'sync_time'].indexOf(propName) >= 0) { | ||||
| 			if (!propValue) return ''; | ||||
| 			propValue = moment.unix(propValue / 1000).utc().format('YYYY-MM-DDTHH:mm:ss.SSS') + 'Z'; | ||||
| 		} else if (propValue === null || propValue === undefined) { | ||||
| @@ -207,18 +207,20 @@ class BaseItem extends BaseModel { | ||||
|  | ||||
| 		let output = []; | ||||
|  | ||||
| 		if ('title' in item) { | ||||
| 		if ('title' in item && shownKeys.indexOf('title') >= 0) { | ||||
| 			output.push(item.title); | ||||
| 			output.push(''); | ||||
| 		} | ||||
|  | ||||
| 		if ('body' in item) { | ||||
| 		if ('body' in item && shownKeys.indexOf('body') >= 0) { | ||||
| 			output.push(item.body); | ||||
| 			if (shownKeys.length) output.push(''); | ||||
| 		} | ||||
|  | ||||
| 		for (let i = 0; i < shownKeys.length; i++) { | ||||
| 			let key = shownKeys[i]; | ||||
| 			if (key == 'title' || key == 'body') continue; | ||||
|  | ||||
| 			let value = null; | ||||
| 			if (typeof key === 'function') { | ||||
| 				let r = await key(); | ||||
|   | ||||
| @@ -3,6 +3,7 @@ import { Log } from 'lib/log.js'; | ||||
| import { promiseChain } from 'lib/promise-utils.js'; | ||||
| import { Note } from 'lib/models/note.js'; | ||||
| import { Setting } from 'lib/models/setting.js'; | ||||
| import { Database } from 'lib/database.js'; | ||||
| import { _ } from 'lib/locale.js'; | ||||
| import moment from 'moment'; | ||||
| import { BaseItem } from 'lib/models/base-item.js'; | ||||
| @@ -48,13 +49,23 @@ class Folder extends BaseItem { | ||||
| 		return r ? r.total : 0; | ||||
| 	} | ||||
|  | ||||
| 	static markNotesAsConflict(parentId) { | ||||
| 		let query = Database.updateQuery('notes', { is_conflict: 1 }, { parent_id: parentId }); | ||||
| 		return this.db().exec(query); | ||||
| 	} | ||||
|  | ||||
| 	static async delete(folderId, options = null) { | ||||
| 		if (!options) options = {}; | ||||
| 		if (!('deleteChildren' in options)) options.deleteChildren = true; | ||||
|  | ||||
| 		let folder = await Folder.load(folderId); | ||||
| 		if (!folder) throw new Error('Trying to delete non-existing notebook: ' + folderId); | ||||
| 		 | ||||
| 		let noteIds = await Folder.noteIds(folderId); | ||||
| 		for (let i = 0; i < noteIds.length; i++) { | ||||
| 			await Note.delete(noteIds[i]); | ||||
| 		if (!folder) return; // noop | ||||
|  | ||||
| 		if (options.deleteChildren) {		 | ||||
| 			let noteIds = await Folder.noteIds(folderId); | ||||
| 			for (let i = 0; i < noteIds.length; i++) { | ||||
| 				await Note.delete(noteIds[i]); | ||||
| 			} | ||||
| 		} | ||||
|  | ||||
| 		await super.delete(folderId, options); | ||||
|   | ||||
| @@ -17,12 +17,12 @@ class Note extends BaseItem { | ||||
| 	static async serialize(note, type = null, shownKeys = null) { | ||||
| 		let fieldNames = this.fieldNames(); | ||||
| 		fieldNames.push('type_'); | ||||
| 		lodash.pull(fieldNames, 'is_conflict', 'sync_time', 'body'); // Exclude 'body' since it's going to be added separately at the top of the note | ||||
| 		lodash.pull(fieldNames, 'is_conflict', 'sync_time'); | ||||
| 		return super.serialize(note, 'note', fieldNames); | ||||
| 	} | ||||
|  | ||||
| 	static async serializeForEdit(note) { | ||||
| 		return super.serialize(note, 'note', []); | ||||
| 		return super.serialize(note, 'note', ['title', 'body']); | ||||
| 	} | ||||
|  | ||||
| 	static async unserializeForEdit(content) { | ||||
| @@ -30,6 +30,13 @@ class Note extends BaseItem { | ||||
| 		return super.unserialize(content); | ||||
| 	} | ||||
|  | ||||
| 	static async serializeAllProps(note) { | ||||
| 		let fieldNames = this.fieldNames(); | ||||
| 		fieldNames.push('type_'); | ||||
| 		lodash.pull(fieldNames, 'title', 'body'); | ||||
| 		return super.serialize(note, 'note', fieldNames); | ||||
| 	} | ||||
|  | ||||
| 	static modelType() { | ||||
| 		return BaseModel.TYPE_NOTE; | ||||
| 	} | ||||
|   | ||||
| @@ -224,9 +224,12 @@ class Synchronizer { | ||||
| 						// await this.api().move(tempPath, path); | ||||
|  | ||||
| 						await this.api().put(path, content); | ||||
| 						await this.api().setTimestamp(path, local.updated_time); | ||||
|  | ||||
| 						if (this.randomFailure(options, 0)) return; | ||||
|  | ||||
| 						await this.api().setTimestamp(path, local.updated_time); | ||||
|  | ||||
| 						if (this.randomFailure(options, 1)) return; | ||||
| 						 | ||||
| 						await ItemClass.save({ id: local.id, sync_time: time.unixMs(), type_: local.type_ }, { autoTimestamp: false }); | ||||
|  | ||||
| @@ -251,7 +254,7 @@ class Synchronizer { | ||||
| 						conflictedNote.is_conflict = 1; | ||||
| 						await Note.save(conflictedNote, { autoTimestamp: false }); | ||||
|  | ||||
| 						if (this.randomFailure(options, 1)) return; | ||||
| 						if (this.randomFailure(options, 2)) return; | ||||
|  | ||||
| 						if (remote) { | ||||
| 							let remoteContent = await this.api().get(path); | ||||
| @@ -289,7 +292,7 @@ class Synchronizer { | ||||
| 				let path = BaseItem.systemPath(item.item_id) | ||||
| 				this.logSyncOperation('deleteRemote', null, { id: item.item_id }, 'local has been deleted'); | ||||
| 				await this.api().delete(path); | ||||
| 				if (this.randomFailure(options, 2)) return; | ||||
| 				if (this.randomFailure(options, 3)) return; | ||||
| 				await BaseItem.remoteDeletedItem(item.item_id); | ||||
|  | ||||
| 				report['deleteRemote']++; | ||||
| @@ -395,6 +398,8 @@ class Synchronizer { | ||||
|  | ||||
| 			if (this.randomFailure(options, 4)) return; | ||||
|  | ||||
| 			let localFoldersToDelete = []; | ||||
|  | ||||
| 			if (!this.cancelling()) { | ||||
| 				let items = await BaseItem.syncedItems(); | ||||
| 				for (let i = 0; i < items.length; i++) { | ||||
| @@ -402,6 +407,11 @@ class Synchronizer { | ||||
|  | ||||
| 					let item = items[i]; | ||||
| 					if (remoteIds.indexOf(item.id) < 0) { | ||||
| 						if (item.type_ == Folder.modelType()) { | ||||
| 							localFoldersToDelete.push(item); | ||||
| 							continue; | ||||
| 						} | ||||
|  | ||||
| 						report.localsToDelete++; | ||||
| 						options.onProgress(report); | ||||
| 						this.logSyncOperation('deleteLocal', { id: item.id }, null, 'remote has been deleted'); | ||||
| @@ -413,6 +423,19 @@ class Synchronizer { | ||||
| 					} | ||||
| 				} | ||||
| 			} | ||||
|  | ||||
| 			if (!this.cancelling()) { | ||||
| 				for (let i = 0; i < localFoldersToDelete.length; i++) { | ||||
| 					const folder = localFoldersToDelete[i]; | ||||
| 					const noteIds = await Folder.noteIds(folder.id); | ||||
| 					if (noteIds.length) { // CONFLICT | ||||
| 						await Folder.markNotesAsConflict(folder.id); | ||||
| 						await Folder.delete(folder.id, { deleteChildren: false }); | ||||
| 					} else { | ||||
| 						await Folder.delete(folder.id); | ||||
| 					} | ||||
| 				} | ||||
| 			} | ||||
| 		} catch (error) { | ||||
| 			report.errors.push(error); | ||||
| 			this.logger().error(error); | ||||
|   | ||||
| @@ -26,6 +26,10 @@ let time = { | ||||
| 		return moment.unix(ms / 1000).format('DD/MM/YYYY HH:mm'); | ||||
| 	}, | ||||
|  | ||||
| 	formatMsToLocal(ms, format) { | ||||
| 		return moment.unix(ms / 1000).format(format); | ||||
| 	}, | ||||
|  | ||||
| 	msleep(ms) { | ||||
| 		return new Promise((resolve, reject) => { | ||||
| 			setTimeout(() => { | ||||
|   | ||||
| @@ -23,7 +23,7 @@ import { FolderScreen } from 'lib/components/screens/folder.js' | ||||
| import { FoldersScreen } from 'lib/components/screens/folders.js' | ||||
| import { LogScreen } from 'lib/components/screens/log.js' | ||||
| import { StatusScreen } from 'lib/components/screens/status.js' | ||||
| import { LoadingScreen } from 'lib/components/screens/loading.js' | ||||
| import { WelcomeScreen } from 'lib/components/screens/welcome.js' | ||||
| import { OneDriveLoginScreen } from 'lib/components/screens/onedrive-login.js' | ||||
| import { Setting } from 'lib/models/setting.js' | ||||
| import { Synchronizer } from 'lib/synchronizer.js' | ||||
| @@ -212,7 +212,7 @@ const AppNavigator = StackNavigator({ | ||||
| 	Notes: { screen: NotesScreen }, | ||||
| 	Note: { screen: NoteScreen }, | ||||
| 	Folder: { screen: FolderScreen }, | ||||
| 	Loading: { screen: LoadingScreen }, | ||||
| 	Welcome: { screen: WelcomeScreen }, | ||||
| 	OneDriveLogin: { screen: OneDriveLoginScreen }, | ||||
| 	Log: { screen: LogScreen }, | ||||
| 	Status: { screen: StatusScreen }, | ||||
| @@ -343,7 +343,7 @@ class AppComponent extends React.Component { | ||||
|  | ||||
| defaultState.nav = AppNavigator.router.getStateForAction({ | ||||
| 	type: 'Navigation/NAVIGATE', | ||||
| 	routeName: 'Loading', | ||||
| 	routeName: 'Welcome', | ||||
| 	params: {} | ||||
| }); | ||||
|  | ||||
|   | ||||
		Reference in New Issue
	
	Block a user