diff --git a/src/sync.d b/src/sync.d index 2d1061b02..f4975d1e5 100644 --- a/src/sync.d +++ b/src/sync.d @@ -1424,16 +1424,10 @@ class SyncEngine { if (debugLogging) {addLogEntry("Flagging to delete item locally: " ~ to!string(onedriveJSONItem), ["debug"]);} idsToDelete ~= [thisItemDriveId, thisItemId]; } else { - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ localPathToDelete, ["info", "notify"]); - } else { - // local data protection is configured, safeBackup the local file, passing in if we are performing a --dry-run or not - // In case the renamed path is needed - string renamedPath; - safeBackup(localPathToDelete, dryRun, renamedPath); - } + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not + // In case the renamed path is needed + string renamedPath; + safeBackup(localPathToDelete, dryRun, bypassDataPreservation, renamedPath); } } else { // Flag to ignore @@ -2114,16 +2108,10 @@ class SyncEngine { // 3. The local modified time > remote modified time // 4. The id of the item from OneDrive is not in the database - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ newItemPath, ["info", "notify"]); - } else { - // local data protection is configured, safeBackup the local file, passing in if we are performing a --dry-run or not - // In case the renamed path is needed - string renamedPath; - safeBackup(newItemPath, dryRun, renamedPath); - } + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not + // In case the renamed path is needed + string renamedPath; + safeBackup(newItemPath, dryRun, bypassDataPreservation, renamedPath); } } else { // Is the remote newer? @@ -2135,16 +2123,10 @@ class SyncEngine { addLogEntry("itemModifiedTime (OneDrive item): " ~ to!string(itemModifiedTime), ["debug"]); } - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ newItemPath, ["info", "notify"]); - } else { - // local data protection is configured, safeBackup the local file, passing in if we are performing a --dry-run or not - // In case the renamed path is needed - string renamedPath; - safeBackup(newItemPath, dryRun, renamedPath); - } + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not + // In case the renamed path is needed + string renamedPath; + safeBackup(newItemPath, dryRun, bypassDataPreservation, renamedPath); } // Are the timestamps equal? @@ -2255,32 +2237,20 @@ class SyncEngine { // The destination item is in-sync if (verboseLogging) {addLogEntry("Destination is in sync and will be overwritten", ["verbose"]);} } else { - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ changedItemPath, ["info", "notify"]); - } else { - // The destination item is different - if (verboseLogging) {addLogEntry("The destination is occupied with a different item, renaming the conflicting file...", ["verbose"]);} - // Backup this item, passing in if we are performing a --dry-run or not - // In case the renamed path is needed - string renamedPath; - safeBackup(changedItemPath, dryRun, renamedPath); - } - } - } else { - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ changedItemPath, ["info", "notify"]); - } else { - // The to be overwritten item is not already in the itemdb, so it should saved to avoid data loss - if (verboseLogging) {addLogEntry("The destination is occupied by an existing un-synced file, renaming the conflicting file...", ["verbose"]);} - // Backup this item, passing in if we are performing a --dry-run or not + // The destination item is different + if (verboseLogging) {addLogEntry("The destination is occupied with a different item, renaming the conflicting file...", ["verbose"]);} + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not // In case the renamed path is needed string renamedPath; - safeBackup(changedItemPath, dryRun, renamedPath); + safeBackup(changedItemPath, dryRun, bypassDataPreservation, renamedPath); } + } else { + // The to be overwritten item is not already in the itemdb, so it should saved to avoid data loss + if (verboseLogging) {addLogEntry("The destination is occupied by an existing un-synced file, renaming the conflicting file...", ["verbose"]);} + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not + // In case the renamed path is needed + string renamedPath; + safeBackup(changedItemPath, dryRun, bypassDataPreservation, renamedPath); } } @@ -2473,18 +2443,12 @@ class SyncEngine { // Does the DB (what we think is in sync) hash match the existing local file hash? if (!testFileHash(newItemPath, databaseItem)) { - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ newItemPath, ["info", "notify"]); - } else { - // local file is different to what we know to be true - addLogEntry("The local file to replace (" ~ newItemPath ~ ") has been modified locally since the last download. Renaming it to avoid potential local data loss."); - // Perform the local safeBackup of the existing local file, passing in if we are performing a --dry-run or not - // In case the renamed path is needed - string renamedPath; - safeBackup(newItemPath, dryRun, renamedPath); - } + // local file is different to what we know to be true + addLogEntry("The local file to replace (" ~ newItemPath ~ ") has been modified locally since the last download. Renaming it to avoid potential local data loss."); + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not + // In case the renamed path is needed + string renamedPath; + safeBackup(newItemPath, dryRun, bypassDataPreservation, renamedPath); } } @@ -4576,8 +4540,8 @@ class SyncEngine { // Online is newer, rename local, then upload the renamed file // We need to know the renamed path so we can upload it string renamedPath; - // Rename the local path - safeBackup(localFilePath, dryRun, renamedPath); + // Rename the local path - we WANT this to occur regardless of bypassDataPreservation setting + safeBackup(localFilePath, dryRun, false, renamedPath); // Upload renamed local file as a new file uploadNewFile(renamedPath); @@ -5949,8 +5913,8 @@ class SyncEngine { // Online is newer, rename local, then upload the renamed file // We need to know the renamed path so we can upload it string renamedPath; - // Rename the local path - safeBackup(fileToUpload, dryRun, renamedPath); + // Rename the local path - we WANT this to occur regardless of bypassDataPreservation setting + safeBackup(fileToUpload, dryRun, false, renamedPath); // Upload renamed local file as a new file uploadNewFile(renamedPath); // Process the database entry removal for the original file. In a --dry-run scenario, this is being done against a DB copy. @@ -9472,18 +9436,11 @@ class SyncEngine { // Attempt to apply this changed item applyPotentiallyChangedItem(existingDatabaseItem, existingItemPath, downloadSharedFileDbItem, newItemPath, fileToDownload); } else { - // Has the user configured to IGNORE local data protection rules? - if (bypassDataPreservation) { - // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup - addLogEntry("WARNING: Local Data Protection has been disabled. You may experience data loss on this file: " ~ newItemPath, ["info", "notify"]); - } else { - // File exists locally, it is not in sync, there is no record in the DB of this file - // In case the renamed path is needed - string renamedPath; - // Rename the local file - safeBackup(newItemPath, dryRun, renamedPath); - } - + // File exists locally, it is not in sync, there is no record in the DB of this file + // In case the renamed path is needed + string renamedPath; + // If local data protection is configured (bypassDataPreservation = false), safeBackup the local file, passing in if we are performing a --dry-run or not + safeBackup(newItemPath, dryRun, bypassDataPreservation, renamedPath); // Submit this shared file to be processed further for downloading applyPotentiallyNewLocalItem(downloadSharedFileDbItem, fileToDownload, newItemPath); } diff --git a/src/util.d b/src/util.d index 322e25d79..92c6853b5 100644 --- a/src/util.d +++ b/src/util.d @@ -55,55 +55,63 @@ shared static this() { } // Creates a safe backup of the given item, and only performs the function if not in a --dry-run scenario -void safeBackup(const(char)[] path, bool dryRun, out string renamedPath) { - auto ext = extension(path); - auto newPath = path.chomp(ext) ~ "-" ~ deviceName ~ "-safeBackup-"; - int n = 1; - - // Limit to 1000 iterations .. 1000 file backups - while (exists(newPath ~ format("%04d", n) ~ ext) && n < 1000) { - n++; - } - - // Check if unique file name was found - if (exists(newPath ~ format("%04d", n) ~ ext)) { - // On the 1000th backup of this file, this should be triggered - addLogEntry("Failed to backup " ~ to!string(path) ~ ": Unique file name could not be found after 1000 attempts", ["error"]); - return; // Exit function as a unique file name could not be found - } +void safeBackup(const(char)[] path, bool dryRun, bool bypassDataPreservation, out string renamedPath) { + // Do we actually perform a safe backup? + // Has the user configured to IGNORE local data protection rules? + if (bypassDataPreservation) { + // The user has configured to ignore data safety checks and overwrite local data rather than preserve & safeBackup + addLogEntry("WARNING: Local Data Protection has been disabled - not renaming local file. You may experience data loss on this file: " ~ to!string(path), ["info", "notify"]); + } else { + // configure variables + auto ext = extension(path); + auto newPath = path.chomp(ext) ~ "-" ~ deviceName ~ "-safeBackup-"; + int n = 1; + + // Limit to 1000 iterations .. 1000 file backups + while (exists(newPath ~ format("%04d", n) ~ ext) && n < 1000) { + n++; + } - // Configure the new name with zero-padded counter - newPath ~= format("%04d", n) ~ ext; + // Check if unique file name was found + if (exists(newPath ~ format("%04d", n) ~ ext)) { + // On the 1000th backup of this file, this should be triggered + addLogEntry("Failed to backup " ~ to!string(path) ~ ": Unique file name could not be found after 1000 attempts", ["error"]); + return; // Exit function as a unique file name could not be found + } - // Log that we are performing the backup by renaming the file - if (verboseLogging) { - addLogEntry("The local item is out-of-sync with OneDrive, renaming to preserve existing file and prevent local data loss: " ~ to!string(path) ~ " -> " ~ to!string(newPath), ["verbose"]); - } + // Configure the new name with zero-padded counter + newPath ~= format("%04d", n) ~ ext; - if (!dryRun) { - // Not a --dry-run scenario - do the file rename - // - // There are 2 options to rename a file - // rename() - https://dlang.org/library/std/file/rename.html - // std.file.copy() - https://dlang.org/library/std/file/copy.html - // - // rename: - // It is not possible to rename a file across different mount points or drives. On POSIX, the operation is atomic. That means, if to already exists there will be no time period during the operation where to is missing. - // - // std.file.copy - // Copy file from to file to. File timestamps are preserved. File attributes are preserved, if preserve equals Yes.preserveAttributes - // - // Use rename() as Linux is POSIX compliant, we have an atomic operation where at no point in time the 'to' is missing. - try { - rename(path, newPath); - renamedPath = to!string(newPath); - } catch (Exception e) { - // Handle exceptions, e.g., log error - addLogEntry("Renaming of local file failed for " ~ to!string(path) ~ ": " ~ e.msg, ["error"]); + // Log that we are performing the backup by renaming the file + if (verboseLogging) { + addLogEntry("The local item is out-of-sync with OneDrive, renaming to preserve existing file and prevent local data loss: " ~ to!string(path) ~ " -> " ~ to!string(newPath), ["verbose"]); } - } else { - if (debugLogging) { - addLogEntry("DRY-RUN: Skipping renaming local file to preserve existing file and prevent data loss: " ~ to!string(path) ~ " -> " ~ to!string(newPath), ["debug"]); + + if (!dryRun) { + // Not a --dry-run scenario - do the file rename + // + // There are 2 options to rename a file + // rename() - https://dlang.org/library/std/file/rename.html + // std.file.copy() - https://dlang.org/library/std/file/copy.html + // + // rename: + // It is not possible to rename a file across different mount points or drives. On POSIX, the operation is atomic. That means, if to already exists there will be no time period during the operation where to is missing. + // + // std.file.copy + // Copy file from to file to. File timestamps are preserved. File attributes are preserved, if preserve equals Yes.preserveAttributes + // + // Use rename() as Linux is POSIX compliant, we have an atomic operation where at no point in time the 'to' is missing. + try { + rename(path, newPath); + renamedPath = to!string(newPath); + } catch (Exception e) { + // Handle exceptions, e.g., log error + addLogEntry("Renaming of local file failed for " ~ to!string(path) ~ ": " ~ e.msg, ["error"]); + } + } else { + if (debugLogging) { + addLogEntry("DRY-RUN: Skipping renaming local file to preserve existing file and prevent data loss: " ~ to!string(path) ~ " -> " ~ to!string(newPath), ["debug"]); + } } } }