Desktop: Fix infinite loop on startup after quickly moving folders (#12140)

packages/lib/models/Folder.test.ts

@@ -163,6 +163,23 @@ describe('models/Folder', () => {
 		}
 	}));
 
+	it('folder hierarchy cycles should not cause addNoteCounts to loop infinitely', async () => {
+		const f1 = await Folder.save({ title: 'folder1' });
+		const f2 = await Folder.save({ title: 'folder2', parent_id: f1.id });
+		const f3 = await Folder.save({ title: 'folder3', parent_id: f2.id });
+		await Note.save({ title: 'test', parent_id: f1.id });
+
+		// Create a cycle.
+		// Note: This has been observed to happen, likely as a result of a bug in other code.
+		await Folder.save({ id: f1.id, parent_id: f3.id });
+
+		const folders = await Folder.all();
+		// Should not loop indefinitely:
+		await Folder.addNoteCounts(folders);
+		// Note count may be incorrect
+		expect(folders.find(folder => folder.id === f1.id)).toHaveProperty('note_count');
+	});
+
 	it('should not count completed to-dos', (async () => {
 
 		const f1 = await Folder.save({ title: 'folder1' });

packages/lib/models/Folder.ts

@@ -187,6 +187,24 @@ export default class Folder extends BaseItem {
 		};
 	}
 
+	// Checks for invalid state -- whether startId or its parents is part of a cycle
+	// in the folder graph (which should be a tree).
+	private static checkForFolderHierarchyCycle_(
+		idToFolder: Record<string, FolderEntity>,
+		startId: string,
+	) {
+		let folderId = startId;
+		const seenIds = new Set();
+		for (; idToFolder[folderId]; folderId = idToFolder[folderId].parent_id) {
+			if (seenIds.has(folderId)) {
+				return true;
+			}
+			seenIds.add(folderId);
+		}
+
+		return false;
+	}
+
 	// Calculates note counts for all folders and adds the note_count attribute to each folder
 	// Note: this only calculates the overall number of nodes for this folder and all its descendants
 	public static async addNoteCounts(folders: FolderEntity[], includeCompletedTodos = true) {
@@ -226,10 +244,22 @@ export default class Folder extends BaseItem {
 		}
 
 		const noteCounts: NoteCount[] = await this.db().selectAll(sql);
-		// eslint-disable-next-line github/array-foreach -- Old code before rule was applied
-		noteCounts.forEach((noteCount) => {
+		for (const noteCount of noteCounts) {
 			let parentId = noteCount.folder_id;
+
+			let i = 0;
+			let checkedForCycle = false;
 			do {
+				// Handle invalid state, preventing infinite loops -- check whether the current
+				// folder has itself as a parent.
+				if (i++ > 100 && !checkedForCycle) {
+					if (Folder.checkForFolderHierarchyCycle_(foldersById, parentId)) {
+						logger.warn(`Invalid state: Folder ${parentId} has itself as a parent.`);
+						break;
+					}
+					checkedForCycle = true;
+				}
+
 				const folder = foldersById[parentId];
 				if (!folder) break; // https://github.com/laurent22/joplin/issues/2079
 				folder.note_count = (folder.note_count || 0) + noteCount.note_count;
@@ -240,7 +270,7 @@ export default class Folder extends BaseItem {
 
 				parentId = folder.parent_id;
 			} while (parentId);
-		});
+		}
 	}
 
 	// Folders that contain notes that have been modified recently go on top.