Desktop, Cli, Mobile, Server: Optimise synchronisation by making delta call return whole items

.eslintignore

@@ -672,6 +672,7 @@ packages/lib/eventManager.js
 packages/lib/file-api-driver-joplinServer.js
 packages/lib/file-api-driver-memory.js
 packages/lib/file-api-driver.test.js
+packages/lib/file-api.test.js
 packages/lib/file-api.js
 packages/lib/fs-driver-base.js
 packages/lib/fs-driver-node.js

.gitignore

@@ -652,6 +652,7 @@ packages/lib/eventManager.js
 packages/lib/file-api-driver-joplinServer.js
 packages/lib/file-api-driver-memory.js
 packages/lib/file-api-driver.test.js
+packages/lib/file-api.test.js
 packages/lib/file-api.js
 packages/lib/fs-driver-base.js
 packages/lib/fs-driver-node.js

packages/lib/Synchronizer.ts

@@ -20,7 +20,7 @@ import JoplinError from './JoplinError';
 import ShareService from './services/share/ShareService';
 import TaskQueue from './TaskQueue';
 import ItemUploader from './services/synchronizer/ItemUploader';
-import { FileApi, RemoteItem } from './file-api';
+import { FileApi, getSupportsDeltaWithItems, PaginatedList, RemoteItem } from './file-api';
 import JoplinDatabase from './JoplinDatabase';
 import { fetchSyncInfo, getActiveMasterKey, localSyncInfo, mergeSyncInfos, saveLocalSyncInfo, setMasterKeyHasBeenUsed, SyncInfo, syncInfoEquals, uploadSyncInfo } from './services/synchronizer/syncInfoUtils';
 import { getMasterPassword, setupAndDisableEncryption, setupAndEnableEncryption } from './services/e2ee/utils';
@@ -389,9 +389,6 @@ export default class Synchronizer {
 		this.syncTargetIsLocked_ = false;
 		this.cancelling_ = false;
 
-		// const masterKeysBefore = await MasterKey.count();
-		// let hasAutoEnabledEncryption = false;
-
 		const synchronizationId = time.unixMs().toString();
 
 		const outputContext = { ...lastContext };
@@ -401,7 +398,7 @@ export default class Synchronizer {
 		this.dispatch({ type: 'SYNC_STARTED' });
 		eventManager.emit(EventName.SyncStart);
 
-		this.logSyncOperation('starting', null, null, `Starting synchronisation to target ${syncTargetId}... supportsAccurateTimestamp = ${this.api().supportsAccurateTimestamp}; supportsMultiPut = ${this.api().supportsMultiPut}; supportsDeltaWithItems = ${this.api().supportsDeltaWithItems} [${synchronizationId}]`);
+		this.logSyncOperation('starting', null, null, `Starting synchronisation to target ${syncTargetId}... supportsAccurateTimestamp = ${this.api().supportsAccurateTimestamp}; supportsMultiPut = ${this.api().supportsMultiPut}} [${synchronizationId}]`);
 
 		const handleCannotSyncItem = async (ItemClass: any, syncTargetId: any, item: any, cannotSyncReason: string, itemLocation: any = null) => {
 			await ItemClass.saveSyncDisabled(syncTargetId, item, cannotSyncReason, itemLocation);
@@ -810,7 +807,7 @@ export default class Synchronizer {
 				while (true) {
 					if (this.cancelling() || hasCancelled) break;
 
-					const listResult: any = await this.apiCall('delta', '', {
+					const listResult: PaginatedList = await this.apiCall('delta', '', {
 						context: context,
 
 						// allItemIdsHandler() provides a way for drivers that don't have a delta API to
@@ -827,7 +824,9 @@ export default class Synchronizer {
 						logger: logger,
 					});
 
-					const remotes: RemoteItem[] = listResult.items;
+					const supportsDeltaWithItems = getSupportsDeltaWithItems(listResult);
+
+					const remotes = listResult.items;
 
 					this.logSyncOperation('fetchingTotal', null, null, 'Fetching delta items from sync target', remotes.length);
 
@@ -843,7 +842,7 @@ export default class Synchronizer {
 							if (local && local.updated_time === remote.jop_updated_time) needsToDownload = false;
 						}
 
-						if (this.api().supportsDeltaWithItems) {
+						if (supportsDeltaWithItems) {
 							needsToDownload = false;
 						}
 
@@ -866,9 +865,7 @@ export default class Synchronizer {
 						if (!BaseItem.isSystemPath(remote.path)) continue; // The delta API might return things like the .sync, .resource or the root folder
 
 						const loadContent = async () => {
-							if (this.api().supportsDeltaWithItems) {
+							if (supportsDeltaWithItems) return remote.jopItem;
-								return remote.jopItem;
-							}
 
 							const task = await this.downloadQueue_.waitForResult(path);
 							if (task.error) throw task.error;

packages/lib/file-api-driver-joplinServer.ts

@@ -45,10 +45,6 @@ export default class FileApiDriverJoplinServer {
 		return true;
 	}
 
-	public get supportsDeltaWithItems() {
-		return true;
-	}
-
 	public requestRepeatCount() {
 		return 3;
 	}

packages/lib/file-api.test.ts

@@ -0,0 +1,73 @@
+import { PaginatedList, RemoteItem, getSupportsDeltaWithItems } from './file-api';
+
+const defaultPaginatedList = (): PaginatedList => {
+	return {
+		items: [],
+		hasMore: false,
+		context: null,
+	};
+};
+
+const defaultItem = (): RemoteItem => {
+	return {
+		id: '',
+	};
+};
+
+describe('file-api', () => {
+
+	test.each([
+		[
+			{
+				...defaultPaginatedList(),
+				items: [],
+			},
+			false,
+		],
+
+		[
+			{
+				...defaultPaginatedList(),
+				items: [
+					{
+						...defaultItem(),
+						path: 'test',
+					},
+				],
+			},
+			false,
+		],
+
+		[
+			{
+				...defaultPaginatedList(),
+				items: [
+					{
+						...defaultItem(),
+						path: 'test',
+						jopItem: null,
+					},
+				],
+			},
+			true,
+		],
+
+		[
+			{
+				...defaultPaginatedList(),
+				items: [
+					{
+						...defaultItem(),
+						path: 'test',
+						jopItem: { something: 'abcd' },
+					},
+				],
+			},
+			true,
+		],
+	])('should tell if the sync target supports delta with items', async (deltaResponse: PaginatedList, expected: boolean) => {
+		const actual = getSupportsDeltaWithItems(deltaResponse);
+		expect(actual).toBe(expected);
+	});
+
+});

packages/lib/file-api.ts

@@ -43,6 +43,14 @@ export interface PaginatedList {
 	context: any;
 }
 
+// Tells whether the delta call is going to include the items themselves or
+// just the metadata (which is the default). If the items are included it
+// means less http request and faster processing.
+export const getSupportsDeltaWithItems = (deltaResponse: PaginatedList) => {
+	if (!deltaResponse.items.length) return false;
+	return 'jopItem' in deltaResponse.items[0];
+};
+
 function requestCanBeRepeated(error: any) {
 	const errorCode = typeof error === 'object' && error.code ? error.code : null;
 
@@ -139,13 +147,6 @@ class FileApi {
 		return !!this.driver().supportsLocks;
 	}
 
-	// Tells whether the delta call is going to include the items themselves or
-	// just the metadata (which is the default). If the items are included it
-	// means less http request and faster processing.
-	public get supportsDeltaWithItems(): boolean {
-		return !!this.driver().supportsDeltaWithItems;
-	}
-
 	private async fetchRemoteDateOffset_() {
 		const tempFile = `${this.tempDirName()}/timeCheck${Math.round(Math.random() * 1000000)}.txt`;
 		const startTime = Date.now();
@@ -361,7 +362,7 @@ class FileApi {
 		return tryAndRepeat(() => this.driver_.clearRoot(this.baseDir()), this.requestRepeatCount());
 	}
 
-	public delta(path: string, options: any = null) {
+	public delta(path: string, options: any = null): Promise<PaginatedList> {
 		logger.debug(`delta ${this.fullPath(path)}`);
 		return tryAndRepeat(() => this.driver_.delta(this.fullPath(path), options), this.requestRepeatCount());
 	}

packages/lib/models/BaseItem.ts

@@ -207,7 +207,7 @@ export default class BaseItem extends BaseModel {
 		return output;
 	}
 
-	public static pathToId(path: string) {
+	public static pathToId(path: string): string {
 		const p = path.split('/');
 		const s = p[p.length - 1].split('.');
 		let name: any = s[0];

packages/server/src/models/ChangeModel.test.ts

@@ -325,4 +325,21 @@ describe('ChangeModel', () => {
 		}
 	});
 
+	test('should not return the whole item if the option is disabled', async () => {
+		const { user } = await createUserAndSession(1, true);
+
+		const changeModel = await models().change();
+		changeModel.deltaIncludesItems_ = false;
+
+		await createItemTree3(user.id, '', '', [
+			{
+				id: '000000000000000000000000000000F1',
+				title: 'Folder 1',
+			},
+		]);
+
+		const result = await changeModel.delta(user.id);
+		expect('jopItem' in result.items[0]).toBe(false);
+	});
+
 });

packages/server/src/models/ChangeModel.ts

@@ -16,7 +16,7 @@ export const defaultChangeTtl = 180 * Day;
 
 export interface DeltaChange extends Change {
 	jop_updated_time?: number;
-	jopItem: any;
+	jopItem?: any;
 }
 
 export type PaginatedDeltaChanges = PaginatedResults<DeltaChange>;
@@ -53,7 +53,7 @@ export function requestDeltaPagination(query: any): ChangePagination {
 
 export default class ChangeModel extends BaseModel<Change> {
 
-	private deltaIncludesItems_: boolean;
+	public deltaIncludesItems_: boolean;
 
 	public constructor(db: DbConnection, modelFactory: NewModelFactoryHandler, config: Config) {
 		super(db, modelFactory, config);
@@ -266,12 +266,14 @@ export default class ChangeModel extends BaseModel<Change> {
 
 		const finalChanges = processedChanges.map(change => {
 			const item = items.find(item => item.id === change.item_id);
-			if (!item) return { ...change, jopItem: null };
+			if (!item) return this.deltaIncludesItems_ ? { ...change, jopItem: null } : { ...change };
 			const deltaChange: DeltaChange = {
 				...change,
 				jop_updated_time: item.jop_updated_time,
-				jopItem: this.deltaIncludesItems_ && item.jop_type ? this.models().item().itemToJoplinItem(item) : null,
 			};
+			if (this.deltaIncludesItems_) {
+				deltaChange.jopItem = item.jop_type ? this.models().item().itemToJoplinItem(item) : null;
+			}
 			return deltaChange;
 		});