From 1fb070067596728835094752083fa1f9f033d7a5 Mon Sep 17 00:00:00 2001 From: abraunegg Date: Fri, 13 Mar 2026 07:41:07 +1100 Subject: [PATCH] Update PR: Add code changes for SL-0018 testing * Add code changes tested locally to test TC-0002 to validate SL-0018 --- src/clientSideFiltering.d | 12 +- src/sync.d | 256 +++++++++++++++++++++++++++++--------- src/util.d | 6 + 3 files changed, 215 insertions(+), 59 deletions(-) diff --git a/src/clientSideFiltering.d b/src/clientSideFiltering.d index c36ce923..4e06145a 100644 --- a/src/clientSideFiltering.d +++ b/src/clientSideFiltering.d @@ -240,6 +240,10 @@ class ClientSideFiltering { // Match against 'sync_list' only bool isPathExcludedViaSyncList(string path) { // Are there 'sync_list' rules to process? + + //addLogEntry("isPathExcludedViaSyncList Input Path = " ~ path); + + if (count(syncListRules) > 0) { // Perform 'sync_list' rule testing on the given path return isPathExcluded(path); @@ -766,10 +770,16 @@ class ClientSideFiltering { } // If any of these exclude match items is true, then finalResult has to be flagged as true - if ((exclude) || (excludeExactMatch) || (excludeParentMatched) || (excludeAnywhereMatched) || (excludeWildcardMatched)) { + //if ((exclude) || (excludeExactMatch) || (excludeParentMatched) || (excludeAnywhereMatched) || (excludeWildcardMatched)) { + // finalResult = true; + //} + + // Only force exclusion if an exclusion rule actually matched this path + if (excludeExactMatch || excludeParentMatched || excludeAnywhereMatched || excludeWildcardMatched) { finalResult = true; } + // Final Result if (finalResult) { if (debugLogging) {addLogEntry("Evaluation against 'sync_list' final result: EXCLUDED as no rule included path", ["debug"]);} diff --git a/src/sync.d b/src/sync.d index c0f943a5..2441fbcd 100644 --- a/src/sync.d +++ b/src/sync.d @@ -118,6 +118,8 @@ class SyncEngine { string[] fileUploadFailures; // List of path names changed online, but not changed locally when using --dry-run string[] pathsRenamed; + // List of path names retained when using --download-only --cleanup-local-files + using a 'sync_list' + string[] pathsRetained; // List of paths that were a POSIX case-insensitive match, thus could not be created online string[] posixViolationPaths; // List of local paths, that, when using the OneDrive Business Shared Folders feature, then disabling it, folder still exists locally and online @@ -1047,6 +1049,7 @@ class SyncEngine { interruptedDownloadFiles = []; pathsToCreateOnline = []; databaseItemsToDeleteOnline = []; + pathsRetained = []; // Perform Garbage Collection on this destroyed curl engine GC.collect(); @@ -7865,121 +7868,188 @@ class SyncEngine { if (debugLogging) {addLogEntry("Adding path to create online (directory inclusion): " ~ path, ["debug"]);} addPathToCreateOnline(path); } else { + // we need to clean up this directory - addLogEntry("Removing local directory as --download-only & --cleanup-local-files configured"); + addLogEntry("Attempting removal of local directory as --download-only & --cleanup-local-files configured"); + // Remove any children of this path if they still exist // Resolve 'Directory not empty' error when deleting local files try { - auto directoryEntries = dirEntries(path, SpanMode.depth, false); - bool pathShouldBeRemoved; + //auto directoryEntries = dirEntries(path, SpanMode.depth, false); + // the cleanup code should only operate on the immediate children of the current directory + auto directoryEntries = dirEntries(path, SpanMode.shallow); + foreach (DirEntry child; directoryEntries) { - // set for error logging - currentPath = child.name; - - // reset pathShouldBeRemoved - pathShouldBeRemoved = true; - - // Issue #3655: Deletion of local data where 'sync_list' is expecting this to be kept - // If we are in a --download-only --cleanup-local-files + using a 'sync_list' the expectation here is that matches to 'sync_list' inclusion are kept - // If we get to this point, we have already validated '--download-only --cleanup-local-files' so this is now just about 'sync_list' inclusion - - // Is 'sync_list' configured? - if (syncListConfigured) { - // Should this path be removed? - // selectiveSync.isPathExcludedViaSyncList() returns 'true' if the path is excluded, 'false' if the path is to be included - pathShouldBeRemoved = selectiveSync.isPathExcludedViaSyncList(child.name); + // Normalise the child path once and use it consistently everywhere + string normalisedChildPath = ensureStartsWithDotSlash(buildNormalizedPath(child.name)); + + // Default action is to remove unless a retention condition is met + bool pathShouldBeRemoved = true; + + // 1. If this path was already retained earlier, never delete it + if (canFind(pathsRetained, normalisedChildPath)) { + pathShouldBeRemoved = false; + addLogEntry("Path already present in pathsRetained - retain path: " ~ normalisedChildPath); } - + + // 2. If not already retained, evaluate via sync_list + if (pathShouldBeRemoved && syncListConfigured) { + // selectiveSync.isPathExcludedViaSyncList() returns: + // true = excluded by sync_list + // false = included / must be retained + if (!selectiveSync.isPathExcludedViaSyncList(child.name)) { + pathShouldBeRemoved = false; + addLogEntry("Path retained due to 'sync_list' inclusion: " ~ normalisedChildPath); + } + } + + // 3. Final authoritative defensive check: + // if the path exists in the item database, it must be retained + //if (pathShouldBeRemoved && pathFoundInDatabase(normalisedChildPath)) { + // pathShouldBeRemoved = false; + // addLogEntry("Path found in database - retain path: " ~ normalisedChildPath); + //} + // What action should be taken? if (pathShouldBeRemoved) { // Path should be removed - // what sort of child is this? if (isDir(child.name)) { - addLogEntry("Removing local directory: " ~ child.name); + addLogEntry("Attempting removal of local directory: " ~ normalisedChildPath); } else { - addLogEntry("Removing local file: " ~ child.name); + addLogEntry("Attempting removal of local file: " ~ normalisedChildPath); } - + // Are we in a --dry-run scenario? if (!dryRun) { // No --dry-run ... process local delete - if (exists(child)) { + if (exists(child.name)) { try { - attrIsDir(child.linkAttributes) ? rmdir(child.name) : safeRemove(child.name); + if (attrIsDir(child.linkAttributes)) { + rmdir(child.name); + // Log removal + addLogEntry("Removed local directory: " ~ normalisedChildPath); + + + + } else { + safeRemove(child.name); + // Log removal + addLogEntry("Removed local file: " ~ normalisedChildPath); + + } } catch (FileException e) { - // display the error message - displayFileSystemErrorMessage(e.msg, thisFunctionName, currentPath); + displayFileSystemErrorMessage(e.msg, thisFunctionName, normalisedChildPath); } } } } else { // Path should be retained - // what sort of child is this? if (isDir(child.name)) { addLogEntry("Local directory should be retained due to 'sync_list' inclusion: " ~ child.name); } else { addLogEntry("Local file should be retained due to 'sync_list' inclusion: " ~ child.name); } + + // Add this path to the retention list if not already present + if (!canFind(pathsRetained, normalisedChildPath)) { + pathsRetained ~= normalisedChildPath; + } + + // Child retained, do not perform any further delete logic for this child + continue; } - - - - - } + // Clear directoryEntries object.destroy(directoryEntries); - + + // Determine whether the parent path itself should be removed bool parentalPathShouldBeRemoved = true; - - // Is 'sync_list' configured? + string normalisedParentPath = ensureStartsWithDotSlash(buildNormalizedPath(path)); + string parentPrefix = normalisedParentPath ~ "/"; + + // 1. sync_list evaluation for the parent path itself if (syncListConfigured) { - parentalPathShouldBeRemoved = selectiveSync.isPathExcludedViaSyncList(path); + // selectiveSync.isPathExcludedViaSyncList() returns: + // true = excluded by sync_list + // false = included / must be retained + if (!selectiveSync.isPathExcludedViaSyncList(path)) { + parentalPathShouldBeRemoved = false; + addLogEntry("Parent path retained due to 'sync_list' inclusion: " ~ path); + } } - + + // 2. If parent path exists in the database, it must be retained + if (parentalPathShouldBeRemoved && pathFoundInDatabase(normalisedParentPath)) { + parentalPathShouldBeRemoved = false; + addLogEntry("Parent path found in database - retain path: " ~ normalisedParentPath); + } + + // 3. If any retained path is this parent or is beneath this parent, retain the parent + if (parentalPathShouldBeRemoved) { + foreach (retainedPath; pathsRetained) { + if ((retainedPath == normalisedParentPath) || retainedPath.startsWith(parentPrefix)) { + parentalPathShouldBeRemoved = false; + addLogEntry("Parent path retained because child path is retained: " ~ retainedPath); + break; + } + } + } + // What action should be taken? if (parentalPathShouldBeRemoved) { - // Remove the parental path now that it is empty of children - addLogEntry("Removing local directory: " ~ path); + addLogEntry("Attempting removal of local directory: " ~ path); + // are we in a --dry-run scenario? if (!dryRun) { // No --dry-run ... process local delete if (exists(path)) { - try { rmdirRecurse(path); + addLogEntry("Removed local directory: " ~ path); + } catch (FileException e) { - // display the error message displayFileSystemErrorMessage(e.msg, thisFunctionName, path); } - } } } else { // Path needs to be retained - addLogEntry("Local directory should be retained due to 'sync_list' inclusion: " ~ path); - - - + addLogEntry("Local parent directory should be retained due to 'sync_list' inclusion: " ~ path); + + // Add the parent path to the retention list if not already present + if (!canFind(pathsRetained, normalisedParentPath)) { + pathsRetained ~= normalisedParentPath; + } } - - + } catch (FileException e) { // display the error message displayFileSystemErrorMessage(e.msg, thisFunctionName, currentPath); - + // Display function processing time if configured to do so if (appConfig.getValueBool("display_processing_time") && debugLogging) { // Combine module name & running Function displayFunctionProcessingTime(thisFunctionName, functionStartTime, Clock.currTime(), logKey); } - + // return as there was an error return; } + + + + + + + + } + + // Cleanup pathsRetained + //pathsRetained = []; } // Do we actually traverse this path? @@ -8061,14 +8131,71 @@ class SyncEngine { } } else { - // we need to clean up this file - addLogEntry("Removing local file as --download-only & --cleanup-local-files configured"); - // are we in a --dry-run scenario? - addLogEntry("Removing local file: " ~ path); - if (!dryRun) { - // No --dry-run ... process local file delete - safeRemove(path); + + // Normalise the file path once and use it consistently everywhere + string normalisedFilePath = ensureStartsWithDotSlash(buildNormalizedPath(path)); + + // Default action is to remove unless a retention condition is met + bool pathShouldBeRemoved = true; + + // 1. If this path was already retained earlier, never delete it + if (canFind(pathsRetained, normalisedFilePath)) { + pathShouldBeRemoved = false; + addLogEntry("Path already present in pathsRetained - retain path: " ~ normalisedFilePath); } + + // 2. If not already retained, evaluate via sync_list + if (pathShouldBeRemoved && syncListConfigured) { + // selectiveSync.isPathExcludedViaSyncList() returns: + // true = excluded by sync_list + // false = included / must be retained + if (!selectiveSync.isPathExcludedViaSyncList(path)) { + pathShouldBeRemoved = false; + addLogEntry("Path retained due to 'sync_list' inclusion: " ~ normalisedFilePath); + } + } + + // 3. Final authoritative defensive check: + // if the path exists in the item database, it must be retained + //if (pathShouldBeRemoved && pathFoundInDatabase(normalisedFilePath)) { + // pathShouldBeRemoved = false; + // addLogEntry("Path found in database - retain path: " ~ normalisedFilePath); + //} + + + // What action should be taken? + if (pathShouldBeRemoved) { + + // we need to clean up this file + addLogEntry("Attempting removal of local file as --download-only & --cleanup-local-files configured"); + // are we in a --dry-run scenario? + addLogEntry("Attempting removal of local file: " ~ normalisedFilePath); + if (!dryRun) { + // No --dry-run ... process local file delete + safeRemove(path); + } + + + } else { + // Path should be retained + addLogEntry("Local file should be retained due to 'sync_list' inclusion: " ~ normalisedFilePath); + + // Add this path to the retention list if not already present + if (!canFind(pathsRetained, normalisedFilePath)) { + pathsRetained ~= normalisedFilePath; + } + + + + } + + + + + + + + } } } else { @@ -8252,6 +8379,17 @@ class SyncEngine { displayFunctionProcessingStart(thisFunctionName, logKey); } + // Normalise input IF required + if (!startsWith(searchPath, "./")) { + + if (searchPath != ".") { + + addLogEntry("searchPath does not start with './' ... searchPath needs normalising"); + searchPath = ensureStartsWithDotSlash(buildNormalizedPath(searchPath)); + } + + } + // Check if this path in the database Item databaseItem; if (debugLogging) {addLogEntry("Search DB for this path: " ~ searchPath, ["debug"]);} @@ -8266,6 +8404,7 @@ class SyncEngine { displayFunctionProcessingTime(thisFunctionName, functionStartTime, Clock.currTime(), logKey); } + if (debugLogging) {addLogEntry("Path found in database - early exit", ["debug"]);} return true; // Early exit on finding the path in the DB } } @@ -8276,6 +8415,7 @@ class SyncEngine { displayFunctionProcessingTime(thisFunctionName, functionStartTime, Clock.currTime(), logKey); } + if (debugLogging) {addLogEntry("Path not found in database after exhausing all driveId entries: " ~ searchPath, ["debug"]);} return false; // Return false if path is not found in any drive } diff --git a/src/util.d b/src/util.d index 2e653a83..37159c8c 100644 --- a/src/util.d +++ b/src/util.d @@ -2373,3 +2373,9 @@ string regexEscape(string s) { void markLocalWrite() { lastLocalWrite = MonoTime.currTime(); } + +bool hasPrefix(string[] pathsRetained, string prefix) { + return pathsRetained.any!(p => p.startsWith(prefix)); +} + +