Chore: Sync fuzzer: Add support for adding and removing share participants (#12988)

packages/tools/fuzzer/ActionTracker.ts

@@ -1,11 +1,9 @@
 import { strict as assert } from 'assert';
-import { ActionableClient, FolderData, FolderMetadata, FuzzContext, ItemId, NoteData, TreeItem, isFolder } from './types';
+import { ActionableClient, FolderData, FolderMetadata, FuzzContext, ItemId, NoteData, TreeItem, assertIsFolder, isFolder } from './types';
 import type Client from './Client';
 
 interface ClientData {
 	childIds: ItemId[];
-	// Shared folders belonging to the client
-	sharedFolderIds: ItemId[];
 }
 
 class ActionTracker {
@@ -33,6 +31,17 @@ class ActionTracker {
 					checkItem(childId);
 				}
 
+				// Shared folders
+				assert.ok(item.ownedByEmail, 'all folders should have a "shareOwner" property (even if not shared)');
+				assert.ok(!item.sharedWith.includes(item.ownedByEmail), 'the share owner should not be in an item\'s sharedWith list');
+				if (item.sharedWith.length > 0) {
+					assert.equal(item.parentId, '', 'only toplevel folders should be shared');
+				}
+				for (const sharedWith of item.sharedWith) {
+					assert.ok(this.tree_.has(sharedWith), 'all sharee users should exist');
+				}
+
+				// Uniqueness
 				assert.equal(
 					item.childIds.length,
 					[...new Set(item.childIds)].length,
@@ -60,7 +69,6 @@ class ActionTracker {
 		if (!this.tree_.has(clientId)) {
 			this.tree_.set(clientId, {
 				childIds: [],
-				sharedFolderIds: [],
 			});
 		}
 
@@ -116,7 +124,8 @@ class ActionTracker {
 				return true;
 			};
 
-			const isOwnedByThis = this.tree_.get(clientId).sharedFolderIds.includes(itemId);
+			const item = this.idToItem_.get(itemId);
+			const isOwnedByThis = isFolder(item) && item.ownedByEmail === clientId;
 
 			if (isOwnedByThis) { // Unshare
 				let removed = false;
@@ -125,12 +134,6 @@ class ActionTracker {
 					removed ||= result;
 				}
 
-				const clientData = this.tree_.get(clientId);
-				this.tree_.set(clientId, {
-					...clientData,
-					sharedFolderIds: clientData.sharedFolderIds.filter(id => id !== itemId),
-				});
-
 				// At this point, the item shouldn't be a child of any clients:
 				assert.ok(hasBeenCompletelyRemoved(), 'item should be removed from all clients');
 				assert.ok(removed, 'should be a toplevel item');
@@ -254,7 +257,8 @@ class ActionTracker {
 					...data,
 					parentId: data.parentId ?? '',
 					childIds: getChildIds(data.id),
-					isShareRoot: false,
+					sharedWith: [],
+					ownedByEmail: clientId,
 				});
 				addChild(data.parentId, data.id);
 
@@ -274,19 +278,16 @@ class ActionTracker {
 				return Promise.resolve();
 			},
 			shareFolder: (id: ItemId, shareWith: Client) => {
-				const shareWithChildIds = this.tree_.get(shareWith.email).childIds;
-				if (shareWithChildIds.includes(id)) {
-					throw new Error(`Folder ${id} already shared with ${shareWith.email}`);
-				}
-				assert.ok(this.idToItem_.has(id), 'should exist');
+				const itemToShare = this.idToItem_.get(id);
+				assertIsFolder(itemToShare);
 
-				const sharerClient = this.tree_.get(clientId);
-				if (!sharerClient.sharedFolderIds.includes(id)) {
-					this.tree_.set(clientId, {
-						...sharerClient,
-						sharedFolderIds: [...sharerClient.sharedFolderIds, id],
-					});
-				}
+				const alreadyShared = itemToShare.sharedWith.includes(shareWith.email);
+				assert.ok(!alreadyShared, `Folder ${id} should not yet be shared with ${shareWith.email}`);
+
+				const shareWithChildIds = this.tree_.get(shareWith.email).childIds;
+				assert.ok(
+					!shareWithChildIds.includes(id), `Share recipient (${shareWith.email}) should not have a folder with ID ${id} before receiving the share.`,
+				);
 
 				this.tree_.set(shareWith.email, {
 					...this.tree_.get(shareWith.email),
@@ -294,8 +295,28 @@ class ActionTracker {
 				});
 
 				this.idToItem_.set(id, {
-					...this.idToItem_.get(id),
-					isShareRoot: true,
+					...itemToShare,
+					sharedWith: [...itemToShare.sharedWith, shareWith.email],
+				});
+
+				this.checkRep_();
+				return Promise.resolve();
+			},
+			removeFromShare: (id: ItemId, shareWith: Client) => {
+				const targetItem = this.idToItem_.get(id);
+				assertIsFolder(targetItem);
+
+				assert.ok(targetItem.sharedWith.includes(shareWith.email), `Folder ${id} should be shared with ${shareWith.label}`);
+
+				const otherSubTree = this.tree_.get(shareWith.email);
+				this.tree_.set(shareWith.email, {
+					...otherSubTree,
+					childIds: otherSubTree.childIds.filter(childId => childId !== id),
+				});
+
+				this.idToItem_.set(id, {
+					...targetItem,
+					sharedWith: targetItem.sharedWith.filter(shareeEmail => shareeEmail !== shareWith.email),
 				});
 
 				this.checkRep_();
@@ -313,7 +334,7 @@ class ActionTracker {
 				}
 
 				if (isFolder(item)) {
-					assert.equal(item.isShareRoot, false, 'cannot move toplevel shared folders without first unsharing');
+					assert.deepEqual(item.sharedWith, [], 'cannot move toplevel shared folders without first unsharing');
 				}
 
 				removeChild(item.parentId, itemId);

packages/tools/fuzzer/Client.ts

@@ -513,35 +513,55 @@ class Client implements ActionableClient {
 	public async shareFolder(id: string, shareWith: Client) {
 		await this.tracker_.shareFolder(id, shareWith);
 
-		logger.info('Share', id, 'with', shareWith.label);
-		await this.execCliCommand_('share', 'add', id, shareWith.email);
+		const getPendingInvitations = async (target: Client) => {
+			const shareWithIncoming = JSON.parse((await target.execCliCommand_('share', 'list', '--json')).stdout);
+			return shareWithIncoming.invitations.filter((invitation: unknown) => {
+				if (typeof invitation !== 'object' || !('accepted' in invitation)) {
+					throw new Error('Invalid invitation format');
+				}
+				return !invitation.accepted;
+			});
+		};
 
-		await this.sync();
-		await shareWith.sync();
+		await retryWithCount(async () => {
+			logger.info('Share', id, 'with', shareWith.label);
+			await this.execCliCommand_('share', 'add', id, shareWith.email);
 
-		const shareWithIncoming = JSON.parse((await shareWith.execCliCommand_('share', 'list', '--json')).stdout);
-		const pendingInvitations = shareWithIncoming.invitations.filter((invitation: unknown) => {
-			if (typeof invitation !== 'object' || !('accepted' in invitation)) {
-				throw new Error('Invalid invitation format');
-			}
-			return !invitation.accepted;
-		});
-		assert.deepEqual(pendingInvitations, [
-			{
-				accepted: false,
-				waiting: true,
-				rejected: false,
-				folderId: id,
-				fromUser: {
-					email: this.email,
+			await this.sync();
+			await shareWith.sync();
+
+			const pendingInvitations = await getPendingInvitations(shareWith);
+			assert.deepEqual(pendingInvitations, [
+				{
+					accepted: false,
+					waiting: true,
+					rejected: false,
+					folderId: id,
+					fromUser: {
+						email: this.email,
+					},
 				},
+			], 'there should be a single incoming share from the expected user');
+		}, {
+			count: 2,
+			delayOnFailure: count => count * Second,
+			onFail: (error)=>{
+				logger.warn('Share failed:', error);
 			},
-		], 'there should be a single incoming share from the expected user');
+		});
 
 		await shareWith.execCliCommand_('share', 'accept', id);
 		await shareWith.sync();
 	}
 
+	public async removeFromShare(id: string, other: Client) {
+		await this.tracker_.removeFromShare(id, other);
+
+		logger.info('Remove', other.label, 'from share', id);
+		await this.execCliCommand_('share', 'remove', id, other.email);
+		await other.sync();
+	}
+
 	public async moveItem(itemId: ItemId, newParentId: ItemId) {
 		logger.info('Move', itemId, 'to', newParentId);
 		await this.tracker_.moveItem(itemId, newParentId);

packages/tools/fuzzer/ClientPool.ts

@@ -38,6 +38,10 @@ export default class ClientPool {
 		});
 	}
 
+	public clientsByEmail(email: string) {
+		return this.clients.filter(client => client.email === email);
+	}
+
 	public randomClient(filter: ClientFilter = ()=>true) {
 		const clients = this.clients_.filter(filter);
 		return clients[

packages/tools/fuzzer/sync-fuzzer.ts

@@ -143,17 +143,37 @@ const doRandomAction = async (context: FuzzContext, client: Client, clientPool:
 			return true;
 		},
 		shareFolder: async () => {
+			const other = clientPool.randomClient(c => !c.hasSameAccount(client));
+			if (!other) return false;
+
 			const target = await client.randomFolder({
-				filter: candidate => (
-					!candidate.parentId && !candidate.isShareRoot
-				),
+				filter: candidate => {
+					const isToplevel = !candidate.parentId;
+					const ownedByCurrent = candidate.ownedByEmail === client.email;
+					const alreadyShared = candidate.sharedWith.includes(other.email);
+					return isToplevel && ownedByCurrent && !alreadyShared;
+				},
 			});
 			if (!target) return false;
 
-			const other = clientPool.randomClient(c => !c.hasSameAccount(client));
 			await client.shareFolder(target.id, other);
 			return true;
 		},
+		unshareFolder: async () => {
+			const target = await client.randomFolder({
+				filter: candidate => {
+					return candidate.sharedWith.length > 0 && candidate.ownedByEmail === client.email;
+				},
+			});
+			if (!target) return false;
+
+			const recipientIndex = context.randInt(0, target.sharedWith.length);
+			const recipientEmail = target.sharedWith[recipientIndex];
+			const recipient = clientPool.clientsByEmail(recipientEmail)[0];
+			assert.ok(recipient, `invalid state -- recipient ${recipientEmail} should exist`);
+			await client.removeFromShare(target.id, recipient);
+			return true;
+		},
 		deleteFolder: async () => {
 			const target = await client.randomFolder({});
 			if (!target) return false;
@@ -174,7 +194,7 @@ const doRandomAction = async (context: FuzzContext, client: Client, clientPool:
 		moveFolderTo: async () => {
 			const target = await client.randomFolder({
 				// Don't move shared folders (should not be allowed by the GUI in the main apps).
-				filter: item => !item.isShareRoot,
+				filter: item => item.sharedWith.length === 0,
 			});
 			if (!target) return false;
 

packages/tools/fuzzer/types.ts

@@ -18,7 +18,9 @@ export type FolderMetadata = {
 };
 export type FolderData = FolderMetadata & {
 	childIds: ItemId[];
-	isShareRoot: boolean;
+	sharedWith: string[];
+	// Email of the Joplin Server account that controls the item
+	ownedByEmail: string;
 };
 export type TreeItem = NoteData|FolderData;
 
@@ -26,6 +28,18 @@ export const isFolder = (item: TreeItem): item is FolderData => {
 	return 'childIds' in item;
 };
 
+// Typescript type assertions require type definitions on the left for arrow functions.
+// See https://github.com/microsoft/TypeScript/issues/53450.
+export const assertIsFolder: (item: TreeItem)=> asserts item is FolderData = item => {
+	if (!item) {
+		throw new Error(`Item ${item} is not a folder`);
+	}
+
+	if (!isFolder(item)) {
+		throw new Error(`Expected item with ID ${item?.id} to be a folder.`);
+	}
+};
+
 export interface FuzzContext {
 	serverUrl: string;
 	baseDir: string;
@@ -40,6 +54,7 @@ export interface RandomFolderOptions {
 export interface ActionableClient {
 	createFolder(data: FolderMetadata): Promise<void>;
 	shareFolder(id: ItemId, shareWith: Client): Promise<void>;
+	removeFromShare(id: string, shareWith: Client): Promise<void>;
 	deleteFolder(id: ItemId): Promise<void>;
 	createNote(data: NoteData): Promise<void>;
 	updateNote(data: NoteData): Promise<void>;
@@ -49,7 +64,7 @@ export interface ActionableClient {
 	listNotes(): Promise<NoteData[]>;
 	listFolders(): Promise<FolderMetadata[]>;
 	allFolderDescendants(parentId: ItemId): Promise<ItemId[]>;
-	randomFolder(options: RandomFolderOptions): Promise<FolderMetadata>;
+	randomFolder(options: RandomFolderOptions): Promise<FolderData>;
 	randomNote(): Promise<NoteData>;
 }
 

packages/tools/fuzzer/utils/retryWithCount.ts

@@ -6,7 +6,7 @@ const logger = Logger.create('retryWithCount');
 interface Options {
 	count: number;
 	delayOnFailure?: (retryCount: number)=> number;
-	onFail: (error: Error)=> Promise<void>;
+	onFail: (error: Error)=> void|Promise<void>;
 }
 
 const retryWithCount = async (task: ()=> Promise<void>, { count, delayOnFailure, onFail }: Options) => {