All: Fixes #6517: Prevent Joplin from missing changes when syncing with file system or WebDAV (#13054)

packages/lib/file-api.ts

@@ -1,12 +1,14 @@
 import Logger, { LoggerWrapper } from '@joplin/utils/Logger';
 import shim from './shim';
-import BaseItem from './models/BaseItem';
+import BaseItem, { RemoteItemMetadata } from './models/BaseItem';
 import time from './time';
 
 const { isHidden } = require('./path-utils');
 import JoplinError from './JoplinError';
 import { Lock, LockClientType, LockType } from './services/synchronizer/LockHandler';
 import * as ArrayUtils from './ArrayUtils';
+import Setting from './models/Setting';
+import SyncTargetRegistry from './SyncTargetRegistry';
 const { sprintf } = require('sprintf-js');
 const Mutex = require('async-mutex').Mutex;
 
@@ -58,6 +60,21 @@ export const isLocalServer = (url: string) => {
 	return regex.test(url);
 };
 
+// The enhanced basic delta algorithm detects incoming changes based on both timestamp increases and decreases, which resolves issues where an external
+// service is syncing to the sync target directory at the same time as Joplin. Change detection is still limited by the precision of the modified timestamp
+// of the filesystem in use, but at worst this would mean that if 2 Joplin clients synced a conflicting change to the same note within 2 seconds, the incoming
+// change may get ignored (but this is a limitation of the normal basic algorithm as well). However, with the enhanced algorithm, the timing of syncs made by
+// an external sync service are irrelevant, providing the service is set to sync the modified time of files it syncs
+export const enableEnhancedBasicDeltaAlgorithm = () => {
+	if (Setting.value('sync.target') === SyncTargetRegistry.nameToId('filesystem')) {
+		return true;
+	} else if (Setting.value('sync.target') === SyncTargetRegistry.nameToId('webdav')) {
+		return isLocalServer(Setting.value('sync.6.path'));
+	} else {
+		return false;
+	}
+};
+
 // eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
 function requestCanBeRepeated(error: any) {
 	const errorCode = typeof error === 'object' && error.code ? error.code : null;
@@ -107,6 +124,7 @@ async function tryAndRepeat(fn: Function, count: number) {
 
 export interface DeltaOptions {
 	allItemIdsHandler(): Promise<string[]>;
+	allItemMetadataHandler(): Promise<Map<string, RemoteItemMetadata>>;
 	logger?: LoggerWrapper;
 	wipeOutFailSafe: boolean;
 }
@@ -458,7 +476,8 @@ function basicDeltaContextFromOptions_(options: any) {
 // eslint-disable-next-line @typescript-eslint/ban-types, @typescript-eslint/no-explicit-any -- Old code before rule was applied, Old code before rule was applied
 async function basicDelta(path: string, getDirStatFn: Function, options: DeltaOptions) {
 	const outputLimit = 50;
-	const itemIds = await options.allItemIdsHandler();
+	const itemIds: string[] = await options.allItemIdsHandler();
+
 	if (!Array.isArray(itemIds)) throw new Error('Delta API not supported - local IDs must be provided');
 
 	const logger = options && options.logger ? options.logger : new Logger();
@@ -499,6 +518,12 @@ async function basicDelta(path: string, getDirStatFn: Function, options: DeltaOp
 		equal: 0,
 	};
 
+	let remoteItemMetadata: Map<string, RemoteItemMetadata>;
+
+	if (enableEnhancedBasicDeltaAlgorithm()) {
+		remoteItemMetadata = await options.allItemMetadataHandler();
+	}
+
 	// Find out which files have been changed since the last time. Note that we keep
 	// both the timestamp of the most recent change, *and* the items that exactly match
 	// this timestamp. This to handle cases where an item is modified while this delta
@@ -512,33 +537,72 @@ async function basicDelta(path: string, getDirStatFn: Function, options: DeltaOp
 		const stat = newContext.statsCache[i];
 
 		if (stat.isDir) continue;
+		if (!BaseItem.isSystemPath(stat.path)) continue;
 
-		if (stat.updated_time < context.timestamp) {
-			updateReport.older++;
-			continue;
-		}
+		let lastRemoteItemUpdatedTime = 0;
+		const itemId = BaseItem.pathToId(stat.path);
 
-		// Special case for items that exactly match the timestamp
-		if (stat.updated_time === context.timestamp) {
-			if (context.filesAtTimestamp.indexOf(stat.path) >= 0) {
-				updateReport.equal++;
+		if (enableEnhancedBasicDeltaAlgorithm()) {
+			const metadata = remoteItemMetadata.get(itemId);
+
+			if (metadata) {
+				// Check if update is needed
+				lastRemoteItemUpdatedTime = metadata.updated_time;
+
+				if (stat.updated_time === lastRemoteItemUpdatedTime) {
+					// Item has already been synced and is up to date
+					updateReport.equal++;
+					continue;
+				}
+
+				if (stat.updated_time < lastRemoteItemUpdatedTime) {
+					updateReport.older++;
+				}
+
+				if (stat.updated_time > lastRemoteItemUpdatedTime) {
+					updateReport.newer++;
+				}
+			} else {
+				// Item needs to be created locally
+				updateReport.newer++;
+			}
+
+			output.push(stat);
+		} else {
+			if (stat.updated_time < context.timestamp) {
+				updateReport.older++;
 				continue;
 			}
-		}
 
-		if (stat.updated_time > newContext.timestamp) {
-			newContext.timestamp = stat.updated_time;
-			newContext.filesAtTimestamp = [];
-			updateReport.newer++;
-		}
+			// Special case for items that exactly match the timestamp
+			if (stat.updated_time === context.timestamp) {
+				if (context.filesAtTimestamp.indexOf(stat.path) >= 0) {
+					updateReport.equal++;
+					continue;
+				}
+			}
 
-		newContext.filesAtTimestamp.push(stat.path);
-		output.push(stat);
+			if (stat.updated_time > newContext.timestamp) {
+				newContext.timestamp = stat.updated_time;
+				newContext.filesAtTimestamp = [];
+				updateReport.newer++;
+			}
+
+			newContext.filesAtTimestamp.push(stat.path);
+			output.push(stat);
+		}
 
 		if (output.length >= outputLimit) break;
 	}
 
-	logger.info(`BasicDelta: Report: ${JSON.stringify(updateReport)}`);
+	if (enableEnhancedBasicDeltaAlgorithm()) {
+		// context.timestamp and filesAtTimestamp are not required when syncing based on any timestamp changes, but should be updated for backwards compatibility
+		newContext.timestamp = time.unixMs();
+		newContext.filesAtTimestamp = [];
+		logger.info(`BasicDelta (enhanced): Report: ${JSON.stringify(updateReport)}`);
+	} else {
+		logger.info(`BasicDelta: Report: ${JSON.stringify(updateReport)}`);
+	}
 
 	if (!newContext.deletedItemsProcessed) {
 		// Find out which items have been deleted on the sync target by comparing the items