Refactoring and testing

.eslintignore

@@ -1345,6 +1345,7 @@ packages/lib/services/synchronizer/Synchronizer.basics.test.js
 packages/lib/services/synchronizer/Synchronizer.conflicts.test.js
 packages/lib/services/synchronizer/Synchronizer.e2ee.test.js
 packages/lib/services/synchronizer/Synchronizer.ppk.test.js
+packages/lib/services/synchronizer/Synchronizer.report.test.js
 packages/lib/services/synchronizer/Synchronizer.resources.test.js
 packages/lib/services/synchronizer/Synchronizer.revisions.test.js
 packages/lib/services/synchronizer/Synchronizer.sharing.test.js

.gitignore

@@ -1321,6 +1321,7 @@ packages/lib/services/synchronizer/Synchronizer.basics.test.js
 packages/lib/services/synchronizer/Synchronizer.conflicts.test.js
 packages/lib/services/synchronizer/Synchronizer.e2ee.test.js
 packages/lib/services/synchronizer/Synchronizer.ppk.test.js
+packages/lib/services/synchronizer/Synchronizer.report.test.js
 packages/lib/services/synchronizer/Synchronizer.resources.test.js
 packages/lib/services/synchronizer/Synchronizer.revisions.test.js
 packages/lib/services/synchronizer/Synchronizer.sharing.test.js

packages/app-mobile/utils/testing/createMockReduxStore.ts

@@ -1,5 +1,4 @@
-import reducer from '@joplin/lib/reducer';
-import { createStore } from 'redux';
+import { createReduxStore } from '@joplin/lib/testing/test-utils';
 import appDefaultState from '../appDefaultState';
 import Setting from '@joplin/lib/models/Setting';
 
@@ -9,11 +8,7 @@ const defaultState = {
 	settings: { theme: Setting.THEME_LIGHT },
 };
 
-const testReducer = (state = defaultState, action: unknown) => {
-	return reducer(state, action);
-};
-
 const createMockReduxStore = () => {
-	return createStore(testReducer);
+	return createReduxStore(defaultState);
 };
 export default createMockReduxStore;

packages/lib/Synchronizer.ts

@@ -30,7 +30,7 @@ import handleConflictAction from './services/synchronizer/utils/handleConflictAc
 import resourceRemotePath from './services/synchronizer/utils/resourceRemotePath';
 import syncDeleteStep from './services/synchronizer/utils/syncDeleteStep';
 import { ErrorCode } from './errors';
-import { SyncAction, SyncReport, SyncReportItemCounts } from './services/synchronizer/utils/types';
+import { SyncAction, SyncReport, ItemCountPerType } from './services/synchronizer/utils/types';
 import checkDisabledSyncItemsNotification from './services/synchronizer/utils/checkDisabledSyncItemsNotification';
 import { substrWithEllipsis } from './string-utils';
 const { sprintf } = require('sprintf-js');
@@ -83,8 +83,7 @@ export default class Synchronizer {
 
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
 	private onProgress_: Function;
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-	private progressReport_: any = {};
+	private progressReport_: SyncReport = {};
 
 	// eslint-disable-next-line @typescript-eslint/ban-types -- Old code before rule was applied
 	public dispatch: Function;
@@ -195,7 +194,7 @@ export default class Synchronizer {
 	}
 
 	public static reportToLines(report: SyncReport) {
-		const formatItemCounts = (counts: SyncReportItemCounts) => {
+		const formatItemCounts = (counts: ItemCountPerType) => {
 			const includedKeyNames: string[] = [];
 			let hasOther = false;
 
@@ -294,14 +293,15 @@ export default class Synchronizer {
 		if (!['fetchingProcessed', 'fetchingTotal'].includes(action)) syncDebugLog.info(line.join(': '));
 
 		// Actions that are categorized by per-item-type
-		const itemActions: string[] = [
-			SyncAction.CreateLocal, SyncAction.CreateRemote, SyncAction.UpdateLocal, SyncAction.UpdateRemote, SyncAction.DeleteLocal, SyncAction.DeleteRemote,
-		];
-		if (itemActions.includes(action)) {
+		const isItemAction = (testAction: string): testAction is SyncAction => {
+			const syncActions: string[] = Object.values(SyncAction);
+			return syncActions.includes(testAction);
+		};
+		if (isItemAction(action)) {
 			this.progressReport_[action] = { ...this.progressReport_[action] };
 			this.progressReport_[action][modelName] ??= 0;
 			this.progressReport_[action][modelName] += actionCount;
-		} else {
+		} else if (action === 'fetchingProcessed' || action === 'fetchingTotal') {
 			this.progressReport_[action] ??= 0;
 			this.progressReport_[action] += actionCount;
 		}
@@ -313,13 +313,14 @@ export default class Synchronizer {
 		// for this but for now this simple fix will do.
 		// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 		const reportCopy: any = {};
-		for (const n in this.progressReport_) reportCopy[n] = this.progressReport_[n];
+		for (const [key, value] of Object.entries(this.progressReport_)) {
+			reportCopy[key] = value;
+		}
 		if (reportCopy.errors) reportCopy.errors = this.progressReport_.errors.slice();
 		this.dispatch({ type: 'SYNC_REPORT_UPDATE', report: reportCopy });
 	}
 
-	// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
-	public async logSyncSummary(report: any) {
+	public async logSyncSummary(report: SyncReport) {
 		logger.info('Operations completed: ');
 		for (const n in report) {
 			if (!report.hasOwnProperty(n)) continue;
@@ -329,7 +330,8 @@ export default class Synchronizer {
 			if (n === 'state') continue;
 			if (n === 'startTime') continue;
 			if (n === 'completedTime') continue;
-			logger.info(`${n}: ${report[n] ? report[n] : '-'}`);
+			const key = n as keyof typeof report;
+			logger.info(`${n}: ${report[key] ? report[key] : '-'}`);
 		}
 		const folderCount = await Folder.count();
 		const noteCount = await Note.count();

packages/lib/services/synchronizer/Synchronizer.report.test.ts

@@ -0,0 +1,65 @@
+import { Store } from 'redux';
+import { State } from '../../reducer';
+import Folder from '../../models/Folder';
+import Note from '../../models/Note';
+import Synchronizer from '../../Synchronizer';
+import { createReduxStore, setupDatabaseAndSynchronizer, switchClient, synchronizer, synchronizerStart } from '../../testing/test-utils';
+
+let appStoreClient2: Store<State>;
+const getClient2SyncReport = () => {
+	return appStoreClient2.getState().syncReport;
+};
+
+describe('Synchronizer.report', () => {
+
+	beforeEach(async () => {
+		await setupDatabaseAndSynchronizer(1);
+		await setupDatabaseAndSynchronizer(2);
+		await switchClient(1);
+
+		appStoreClient2 = createReduxStore();
+		synchronizer(2).dispatch = appStoreClient2.dispatch;
+	});
+
+	test('should list the different kinds of items that were deleted', async () => {
+		const folder = await Folder.save({ title: 'Test folder' });
+		await Note.save({ title: 'Note 1', parent_id: folder.id });
+		const note2 = await Note.save({ title: 'Note 2' });
+
+		// Ensure that client 2 creates the items
+		await synchronizerStart();
+		await switchClient(2);
+		await synchronizerStart();
+
+		await Note.delete(note2.id, { toTrash: false });
+		await synchronizerStart();
+
+		// Deleting a remote item: Should list item types
+		expect(getClient2SyncReport()).toMatchObject({
+			deleteRemote: {
+				Note: 1,
+			},
+		});
+		expect(Synchronizer.reportToLines(getClient2SyncReport())[0]).toBe(
+			'Deleted remote: 1 (notes).',
+		);
+
+		// Delete a remote folder
+		await switchClient(1);
+		await Folder.delete(folder.id);
+		await synchronizerStart();
+		await switchClient(2);
+
+		// Deleting local items: Sync report should include type descriptions:
+		await synchronizerStart();
+		expect(getClient2SyncReport()).toMatchObject({
+			deleteLocal: {
+				Note: 1,
+				Folder: 1,
+			},
+		});
+		expect(Synchronizer.reportToLines(getClient2SyncReport())[0]).toBe(
+			'Deleted local: 2 (notes, notebooks).',
+		);
+	});
+});

packages/lib/services/synchronizer/utils/types.ts

@@ -18,12 +18,19 @@ export enum SyncAction {
 	DeleteLocal = 'deleteLocal',
 }
 
-export type SyncReportItemCounts = Record<string, number>;
-type SyncReportItemSection = Partial<Record<SyncAction, SyncReportItemCounts>>;
+export interface ItemCountPerType {
+	[modelType: string]: number;
+}
+
+type SyncReportItemSection = {
+	[action in SyncAction]?: ItemCountPerType;
+};
 
 export type SyncReport = SyncReportItemSection & {
 	fetchingTotal?: number;
 	fetchingProcessed?: number;
+
+	state?: string;
 	cancelling?: boolean;
 	startTime?: number;
 	completedTime?: number;

packages/lib/testing/test-utils.ts

@@ -67,7 +67,8 @@ import OcrDriverTesseract from '../services/ocr/drivers/OcrDriverTesseract';
 import OcrService from '../services/ocr/OcrService';
 import { createWorker } from 'tesseract.js';
 import { reg } from '../registry';
-import { Store } from 'redux';
+import { createStore, Store } from 'redux';
+import reducer, { defaultState as defaultAppState, State as AppState } from '../reducer';
 
 // Each suite has its own separate data and temp directory so that multiple
 // suites can be run at the same time. suiteName is what is used to
@@ -453,6 +454,14 @@ const createNoteAndResource = async (options: CreateNoteAndResourceOptions = nul
 	return { note, resource };
 };
 
+export const createReduxStore = (defaultState: AppState = defaultAppState) => {
+	const mockReducer = (state: AppState = defaultState, action: unknown) => {
+		return reducer(state, action);
+	};
+
+	return createStore(mockReducer);
+};
+
 // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 async function setupDatabaseAndSynchronizer(id: number, options: any = null) {
 	if (id === null) id = currentClient_;