All: Fixed issue where synchroniser would try to update a shared fold…

…er that is not longer accessible

packages/app-desktop/gui/MainScreen/commands/leaveSharedFolder.ts

@@ -1,7 +1,6 @@
 import { CommandRuntime, CommandDeclaration, CommandContext } from '@joplin/lib/services/CommandService';
 import { _ } from '@joplin/lib/locale';
 import ShareService from '@joplin/lib/services/share/ShareService';
-import Setting from '@joplin/lib/models/Setting';
 import Logger from '@joplin/lib/Logger';
 
 const logger = Logger.create('leaveSharedFolder');
@@ -26,10 +25,7 @@ export const runtime = (): CommandRuntime => {
 				const share = shares.find(s => s.folder_id === folderId);
 				if (!share) throw new Error(_('Could not verify the share status of this notebook - aborting. Please try again when you are connected to the internet.'));
 
-				const userId = Setting.value('sync.userId');
-				if (share.user.id === userId) throw new Error('Cannot leave own notebook');
-
-				await ShareService.instance().leaveSharedFolder(folderId);
+				await ShareService.instance().leaveSharedFolder(folderId, share.user.id);
 			} catch (error) {
 				logger.error(error);
 				alert(_('Error: %s', error.message));

packages/lib/Logger.ts

@@ -94,8 +94,10 @@ class Logger {
 		};
 	}
 
-	setLevel(level: LogLevel) {
+	public setLevel(level: LogLevel) {
+		const previous = this.level_;
 		this.level_ = level;
+		return previous;
 	}
 
 	level() {

packages/lib/Synchronizer.ts

@@ -424,6 +424,7 @@ export default class Synchronizer {
 		// Before synchronising make sure all share_id properties are set
 		// correctly so as to share/unshare the right items.
 		await Folder.updateAllShareIds(this.resourceService());
+		if (this.shareService_) await this.shareService_.checkShareConsistency();
 
 		const itemUploader = new ItemUploader(this.api(), this.apiCall);
 

packages/lib/models/Folder.ts

@@ -282,7 +282,7 @@ export default class Folder extends BaseItem {
 		return this.db().selectAll(sql, [folderId]);
 	}
 
-	private static async rootSharedFolders(): Promise<FolderEntity[]> {
+	public static async rootSharedFolders(): Promise<FolderEntity[]> {
 		return this.db().selectAll('SELECT id, share_id FROM folders WHERE parent_id = "" AND share_id != ""');
 	}
 

packages/lib/services/share/ShareService.test.ts

@@ -1,7 +1,7 @@
 import Note from '../../models/Note';
 import { encryptionService, msleep, setupDatabaseAndSynchronizer, switchClient } from '../../testing/test-utils';
 import ShareService from './ShareService';
-import reducer from '../../reducer';
+import reducer, { defaultState } from '../../reducer';
 import { createStore } from 'redux';
 import { FolderEntity, NoteEntity } from '../database/types';
 import Folder from '../../models/Folder';
@@ -10,10 +10,15 @@ import { generateKeyPair } from '../e2ee/ppk';
 import MasterKey from '../../models/MasterKey';
 import { MasterKeyEntity } from '../e2ee/types';
 import { updateMasterPassword } from '../e2ee/utils';
+import Logger, { LogLevel } from '../../Logger';
+
+const testReducer = (state: any = defaultState, action: any) => {
+	return reducer(state, action);
+};
 
 function mockService(api: any) {
 	const service = new ShareService();
-	const store = createStore(reducer as any);
+	const store = createStore(testReducer as any);
 	service.initialize(store, encryptionService(), api);
 	return service;
 }
@@ -149,4 +154,26 @@ describe('ShareService', function() {
 		expect(content.ppkId).toBe(recipientPpk.id);
 	});
 
+	it('should leave folders that are no longer with the user', async () => {
+		// `checkShareConsistency` will emit a warning so we need to silent it
+		// in tests.
+		const previousLogLevel = Logger.globalLogger.setLevel(LogLevel.Error);
+
+		const service = testShareFolderService({
+			'GET api/shares': async (_query: Record<string, any>, _body: any): Promise<any> => {
+				return {
+					items: [],
+					has_more: false,
+				};
+			},
+		});
+
+		const folder = await Folder.save({ share_id: 'nolongershared' });
+		await service.checkShareConsistency();
+		expect(await Folder.load(folder.id)).toBeFalsy();
+
+		Logger.globalLogger.setLevel(previousLogLevel);
+	});
+
+
 });

packages/lib/services/share/ShareService.ts

@@ -181,10 +181,53 @@ export default class ShareService {
 	//
 	// We don't delete the children here because that would delete them for the
 	// other share participants too.
-	public async leaveSharedFolder(folderId: string): Promise<void> {
+	//
+	// If `folderShareUserId` is provided, the function will check that the user
+	// does not own the share. It would be an error to leave such a folder
+	// (instead "unshareFolder" should be called).
+	public async leaveSharedFolder(folderId: string, folderShareUserId: string = null): Promise<void> {
+		if (folderShareUserId !== null) {
+			const userId = Setting.value('sync.userId');
+			if (folderShareUserId === userId) throw new Error('Cannot leave own notebook');
+		}
+
 		await Folder.delete(folderId, { deleteChildren: false });
 	}
 
+	// Finds any folder that is associated with a share, but the user no longer
+	// has access to the share, and remove these folders. This check is
+	// necessary otherwise sync will try to update items that are not longer
+	// accessible and will throw the error "Could not find share with ID: xxxx")
+	public async checkShareConsistency() {
+		const rootSharedFolders = await Folder.rootSharedFolders();
+		let hasRefreshedShares = false;
+		let shares = this.shares;
+
+		for (const folder of rootSharedFolders) {
+			let share = shares.find(s => s.id === folder.share_id);
+
+			if (!share && !hasRefreshedShares) {
+				shares = await this.refreshShares();
+				share = shares.find(s => s.id === folder.share_id);
+				hasRefreshedShares = true;
+			}
+
+			if (!share) {
+				// This folder is a associated with a share, but the user no
+				// longer has access to this share. It can happen for two
+				// reasons:
+				//
+				// - It no longer exists
+				// - Or the user rejected that share from a different device,
+				//   and the folder was not deleted as it should have been.
+				//
+				// In that case we need to leave the notebook.
+				logger.warn(`Found a folder that was associated with a share, but the user not longer has access to the share - leaving the folder. Folder: ${folder.title} (${folder.id}). Share: ${folder.share_id}`);
+				await this.leaveSharedFolder(folder.id);
+			}
+		}
+	}
+
 	public async shareNote(noteId: string): Promise<StateShare> {
 		const note = await Note.load(noteId);
 		if (!note) throw new Error(`No such note: ${noteId}`);