All: Fixed issue when trying to sync an item associated with a share that no longer exists

packages/lib/JoplinServerApi.ts

@@ -91,7 +91,7 @@ export default class JoplinServerApi {
 		return _('Could not connect to Joplin Server. Please check the Synchronisation options in the config screen. Full error was:\n\n%s', msg);
 	}
 
-	private hidePassword(o: any): any {
+	private hidePasswords(o: any): any {
 		if (typeof o === 'string') {
 			try {
 				const output = JSON.parse(o);
@@ -104,6 +104,7 @@ export default class JoplinServerApi {
 		} else {
 			const output = { ...o };
 			if (output.password) output.password = '******';
+			if (output['X-API-AUTH']) output['X-API-AUTH'] = '******';
 			return output;
 		}
 	}
@@ -114,14 +115,14 @@ export default class JoplinServerApi {
 		output.push('-v');
 		if (options.method) output.push(`-X ${options.method}`);
 		if (options.headers) {
+			const headers = this.hidePasswords(options.headers);
 			for (const n in options.headers) {
 				if (!options.headers.hasOwnProperty(n)) continue;
-				const headerValue = n === 'X-API-AUTH' ? '******' : options.headers[n];
-				output.push(`${'-H ' + '"'}${n}: ${headerValue}"`);
+				output.push(`${'-H ' + '"'}${n}: ${headers[n]}"`);
 			}
 		}
 		if (options.body) {
-			const serialized = typeof options.body !== 'string' ? JSON.stringify(this.hidePassword(options.body)) : this.hidePassword(options.body);
+			const serialized = typeof options.body !== 'string' ? JSON.stringify(this.hidePasswords(options.body)) : this.hidePasswords(options.body);
 			output.push(`${'--data ' + '\''}${serialized}'`);
 		}
 		output.push(`'${url}'`);

packages/lib/models/Folder.sharing.test.ts

@@ -326,37 +326,65 @@ describe('models_Folder.sharing', function() {
 
 		await Folder.updateAllShareIds();
 
-		// await NoteResource.updateResourceShareIds();
-
 		{
 			const resource: ResourceEntity = await Resource.load(resourceId);
 			expect(resource.share_id).toBe('');
 		}
 	});
 
-	// it('should not recursively delete when non-owner deletes a shared folder', async () => {
-	// 	const folder = await createFolderTree('', [
-	// 		{
-	// 			title: 'folder 1',
-	// 			children: [
-	// 				{
-	// 					title: 'note 1',
-	// 				},
-	// 			],
-	// 		},
-	// 	]);
+	it('should unshare items that are no longer part of an existing share', async () => {
+		await createFolderTree('', [
+			{
+				title: 'folder 1',
+				share_id: '1',
+				children: [
+					{
+						title: 'note 1',
+					},
+				],
+			},
+			{
+				title: 'folder 2',
+				share_id: '2',
+				children: [
+					{
+						title: 'note 2',
+					},
+				],
+			},
+		]);
 
-	// 	BaseItem.shareService_ = {
-	// 		isSharedFolderOwner: (_folderId: string) => false,
-	// 	} as any;
+		const resourceService = new ResourceService();
 
-	// 	await Folder.save({ id: folder.id, share_id: 'abcd1234' });
-	// 	await Folder.updateAllShareIds();
+		let note1: NoteEntity = await Note.loadByTitle('note 1');
+		let note2: NoteEntity = await Note.loadByTitle('note 2');
+		note1 = await shim.attachFileToNote(note1, testImagePath);
+		note2 = await shim.attachFileToNote(note2, testImagePath);
+		const resourceId1 = (await Note.linkedResourceIds(note1.body))[0];
+		const resourceId2 = (await Note.linkedResourceIds(note2.body))[0];
 
-	// 	await Folder.delete(folder.id);
+		await resourceService.indexNoteResources();
 
-	// 	expect((await Folder.all()).length).toBe(0);
-	// 	expect((await Note.all()).length).toBe(1);
-	// });
+		await Folder.updateAllShareIds();
+
+		await Folder.updateNoLongerSharedItems(['1']);
+
+		// At this point, all items associated with share 2 should have their
+		// share_id cleared, because the share no longer exists. We also
+		// double-check that share 1 hasn't been cleared.
+		expect((await Note.loadByTitle('note 1')).share_id).toBe('1');
+		expect((await Note.loadByTitle('note 2')).share_id).toBe('');
+		expect((await Folder.loadByTitle('folder 1')).share_id).toBe('1');
+		expect((await Folder.loadByTitle('folder 2')).share_id).toBe('');
+		expect((await Resource.load(resourceId1)).share_id).toBe('1');
+		expect((await Resource.load(resourceId2)).share_id).toBe('');
+
+		// If we pass an empty array, it means there are no active share
+		// anymore, so all share_id should be cleared.
+		await Folder.updateNoLongerSharedItems([]);
+		expect((await Note.loadByTitle('note 1')).share_id).toBe('');
+		expect((await Folder.loadByTitle('folder 1')).share_id).toBe('');
+		expect((await Resource.load(resourceId1)).share_id).toBe('');
+	});
 
 });

packages/lib/models/Folder.ts

@@ -374,6 +374,38 @@ export default class Folder extends BaseItem {
 		await this.updateResourceShareIds();
 	}
 
+	// Clear the "share_id" property for the items that are associated with a
+	// share that no longer exists.
+	public static async updateNoLongerSharedItems(activeShareIds: string[]) {
+		const tableNameToClasses: Record<string, any> = {
+			'folders': Folder,
+			'notes': Note,
+			'resources': Resource,
+		};
+
+		for (const tableName of ['folders', 'notes', 'resources']) {
+			const ItemClass = tableNameToClasses[tableName];
+
+			const query = activeShareIds.length ? `
+				SELECT id FROM ${tableName}
+				WHERE share_id NOT IN ("${activeShareIds.join('","')}")
+			` : `
+				SELECT id FROM ${tableName}
+				WHERE share_id != ''
+			`;
+
+			const rows = await this.db().selectAll(query);
+
+			for (const row of rows) {
+				await ItemClass.save({
+					id: row.id,
+					share_id: '',
+					updated_time: Date.now(),
+				}, { autoTimestamp: false });
+			}
+		}
+	}
+
 	static async allAsTree(folders: FolderEntity[] = null, options: any = null) {
 		const all = folders ? folders : await this.all(options);
 

packages/lib/services/share/ShareService.ts

@@ -1,10 +1,13 @@
 import { Store } from 'redux';
 import JoplinServerApi from '../../JoplinServerApi';
+import Logger from '../../Logger';
 import Folder from '../../models/Folder';
 import Note from '../../models/Note';
 import Setting from '../../models/Setting';
 import { State, stateRootKey, StateShare } from './reducer';
 
+const logger = Logger.create('ShareService');
+
 export default class ShareService {
 
 	private static instance_: ShareService;
@@ -152,6 +155,10 @@ export default class ShareService {
 		return this.shares.filter(s => !!s.note_id).map(s => s.note_id);
 	}
 
+	public get shareInvitations() {
+		return this.state.shareInvitations;
+	}
+
 	public async addShareRecipient(shareId: string, recipientEmail: string) {
 		return this.api().exec('POST', `api/shares/${shareId}/users`, {}, {
 			email: recipientEmail,
@@ -216,11 +223,26 @@ export default class ShareService {
 		});
 	}
 
+	private async updateNoLongerSharedItems() {
+		const shareIds = this.shares.map(share => share.id).concat(this.shareInvitations.map(si => si.share.id));
+		await Folder.updateNoLongerSharedItems(shareIds);
+	}
+
 	public async maintenance() {
 		if (this.enabled) {
-			await this.refreshShareInvitations();
-			await this.refreshShares();
-			Setting.setValue('sync.userId', this.api().userId);
+			let hasError = false;
+			try {
+				await this.refreshShareInvitations();
+				await this.refreshShares();
+				Setting.setValue('sync.userId', this.api().userId);
+			} catch (error) {
+				hasError = true;
+				logger.error('Failed to run maintenance:', error);
+			}
+
+			// If there was no errors, it means we have all the share objects,
+			// so we can run the clean up function.
+			if (!hasError) await this.updateNoLongerSharedItems();
 		}
 	}
 

packages/lib/shim.ts

@@ -1,4 +1,4 @@
-import { ResourceEntity } from './services/database/types';
+import { NoteEntity, ResourceEntity } from './services/database/types';
 
 let isTestingEnv_ = false;
 
@@ -211,7 +211,7 @@ const shim = {
 
 	detectAndSetLocale: null as Function,
 
-	attachFileToNote: async (_note: any, _filePath: string) => {
+	attachFileToNote: async (_note: any, _filePath: string): Promise<NoteEntity> => {
 		throw new Error('Not implemented');
 	},