Chore: Sync fuzzer: Fix incorrect expected state after removing the last user from a share (#13061)

packages/tools/fuzzer/ActionTracker.ts

@@ -1,12 +1,15 @@
 import { strict as assert } from 'assert';
 import { ActionableClient, FolderData, FuzzContext, ItemId, NoteData, ShareOptions, TreeItem, assertIsFolder, isFolder } from './types';
-import type Client from './Client';
 import FolderRecord from './model/FolderRecord';
 
 interface ClientData {
 	childIds: ItemId[];
 }
 
+interface ClientInfo {
+	email: string;
+}
+
 class ActionTracker {
 	private idToItem_: Map<ItemId, TreeItem> = new Map();
 	private tree_: Map<string, ClientData> = new Map();
@@ -87,7 +90,7 @@ class ActionTracker {
 		}
 	}
 
-	public track(client: { email: string }) {
+	public track(client: ClientInfo) {
 		const clientId = client.email;
 		// If the client's remote account already exists, continue using it:
 		if (!this.tree_.has(clientId)) {
@@ -271,6 +274,23 @@ class ActionTracker {
 			}
 		};
 
+		const removeFromShare = (id: ItemId, shareWith: ClientInfo) => {
+			const targetItem = this.idToItem_.get(id);
+			assertIsFolder(targetItem);
+
+			assert.ok(targetItem.isSharedWith(shareWith.email), `Folder ${id} should be shared with ${shareWith.email}`);
+
+			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.withRemovedFromShare(shareWith.email));
+
+			this.checkRep_();
+		};
+
 		const tracker: ActionableClient = {
 			createNote: (data: NoteData) => {
 				assertWriteable(data.parentId);
@@ -311,6 +331,7 @@ class ActionTracker {
 					childIds: getChildIds(data.id),
 					sharedWith: [],
 					ownedByEmail: clientId,
+					isShared: false,
 				}));
 				addChild(data.parentId, data.id);
 
@@ -330,7 +351,7 @@ class ActionTracker {
 				this.checkRep_();
 				return Promise.resolve();
 			},
-			shareFolder: (id: ItemId, shareWith: Client, options: ShareOptions) => {
+			shareFolder: (id: ItemId, shareWith: ClientInfo, options: ShareOptions) => {
 				const itemToShare = this.idToItem_.get(id);
 				assertIsFolder(itemToShare);
 
@@ -354,19 +375,16 @@ class ActionTracker {
 				this.checkRep_();
 				return Promise.resolve();
 			},
-			removeFromShare: (id: ItemId, shareWith: Client) => {
+			removeFromShare: (id, client) => Promise.resolve(removeFromShare(id, client)),
+			deleteAssociatedShare: (id: ItemId) => {
 				const targetItem = this.idToItem_.get(id);
 				assertIsFolder(targetItem);
 
-				assert.ok(targetItem.isSharedWith(shareWith.email), `Folder ${id} should be shared with ${shareWith.label}`);
+				for (const recipient of targetItem.shareRecipients) {
+					removeFromShare(id, { email: recipient });
+				}
 
-				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.withUnshared(shareWith.email));
+				this.idToItem_.set(id, targetItem.withUnshared());
 
 				this.checkRep_();
 				return Promise.resolve();

packages/tools/fuzzer/Client.ts

@@ -567,6 +567,12 @@ class Client implements ActionableClient {
 		await other.sync();
 	}
 
+	public async deleteAssociatedShare(id: string) {
+		await this.tracker_.deleteAssociatedShare(id);
+		logger.info('Unshare', id, '(from', this.label, ')');
+		await this.execCliCommand_('share', 'delete', '-f', id);
+	}
+
 	public async moveItem(itemId: ItemId, newParentId: ItemId) {
 		logger.info('Move', itemId, 'to', newParentId);
 		await this.tracker_.moveItem(itemId, newParentId);

packages/tools/fuzzer/model/FolderRecord.ts

@@ -9,6 +9,7 @@ export type ShareRecord = {
 interface InitializationOptions extends FolderData {
 	childIds: ItemId[];
 	sharedWith: ShareRecord[];
+	isShared: boolean;
 	// Email of the Joplin Server account that controls the item
 	ownedByEmail: string;
 }
@@ -23,6 +24,10 @@ export default class FolderRecord implements FolderData {
 	public readonly title: string;
 	public readonly ownedByEmail: string;
 	public readonly childIds: ItemId[];
+
+	// Only valid for root folders. Note that a separate isShared_ is needed
+	// because Joplin folders can be 'shared' even if there are no share recipients.
+	private readonly isShared_: boolean;
 	private readonly sharedWith_: ShareRecord[];
 
 	public constructor(options: InitializationOptions) {
@@ -30,6 +35,7 @@ export default class FolderRecord implements FolderData {
 		this.id = options.id;
 		this.title = options.title;
 		this.childIds = options.childIds;
+		this.isShared_ = options.isShared ?? options.sharedWith.length > 0;
 		this.ownedByEmail = options.ownedByEmail;
 		this.sharedWith_ = options.sharedWith;
 
@@ -51,6 +57,7 @@ export default class FolderRecord implements FolderData {
 			parentId: this.parentId,
 			id: this.id,
 			title: this.title,
+			isShared: this.isShared_,
 			ownedByEmail: this.ownedByEmail,
 			childIds: [...this.childIds],
 			sharedWith: [...this.sharedWith_],
@@ -58,7 +65,7 @@ export default class FolderRecord implements FolderData {
 	}
 
 	public get isRootSharedItem() {
-		return this.sharedWith_.length > 0;
+		return this.isShared_;
 	}
 
 	public isSharedWith(email: string) {
@@ -124,6 +131,7 @@ export default class FolderRecord implements FolderData {
 
 		return new FolderRecord({
 			...this.metadata_,
+			isShared: true,
 			sharedWith: [
 				...this.sharedWith_.filter(record => record.email !== recipientEmail),
 				{ email: recipientEmail, readOnly },
@@ -131,7 +139,7 @@ export default class FolderRecord implements FolderData {
 		});
 	}
 
-	public withUnshared(recipientEmail: string) {
+	public withRemovedFromShare(recipientEmail: string) {
 		if (!this.isSharedWith(recipientEmail)) {
 			return this;
 		}
@@ -141,4 +149,16 @@ export default class FolderRecord implements FolderData {
 			sharedWith: this.sharedWith_.filter(record => record.email !== recipientEmail),
 		});
 	}
+
+	public withUnshared() {
+		if (!this.isShared_) {
+			return this;
+		}
+
+		return new FolderRecord({
+			...this.metadata_,
+			sharedWith: [],
+			isShared: false,
+		});
+	}
 }

packages/tools/fuzzer/sync-fuzzer.ts

@@ -172,11 +172,15 @@ const doRandomAction = async (context: FuzzContext, client: Client, clientPool:
 			});
 			if (!target) return false;
 
-			const recipientIndex = context.randInt(0, target.shareRecipients.length);
-			const recipientEmail = target.shareRecipients[recipientIndex];
-			const recipient = clientPool.clientsByEmail(recipientEmail)[0];
-			assert.ok(recipient, `invalid state -- recipient ${recipientEmail} should exist`);
-			await client.removeFromShare(target.id, recipient);
+			const recipientIndex = context.randInt(-1, target.shareRecipients.length);
+			if (recipientIndex === -1) { // Completely remove the share
+				await client.deleteAssociatedShare(target.id);
+			} else {
+				const recipientEmail = target.shareRecipients[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 () => {

packages/tools/fuzzer/types.ts

@@ -67,6 +67,7 @@ export interface ActionableClient {
 	createFolder(data: FolderData): Promise<void>;
 	shareFolder(id: ItemId, shareWith: Client, options: ShareOptions): Promise<void>;
 	removeFromShare(id: string, shareWith: Client): Promise<void>;
+	deleteAssociatedShare(id: string): Promise<void>;
 	deleteFolder(id: ItemId): Promise<void>;
 	createNote(data: NoteData): Promise<void>;
 	updateNote(data: NoteData): Promise<void>;