Skip to content

Commit

Permalink
Update PR
Browse files Browse the repository at this point in the history 
* Refactor use of safeBackup passing in bypassDataPreservation to determine if the backup should be taken to ensure consistency in messaging
  • Loading branch information
@abraunegg
abraunegg committed Nov 13, 2024
1 parent fb7dcda commit 74a91f3
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 126 deletions.
119 changes: 38 additions & 81 deletions src/sync.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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?
Expand All @@ -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?
Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);
}
}

Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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);
}
Expand Down
98 changes: 53 additions & 45 deletions src/util.d
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"]);
}
}
}
}
Expand Down

0 comments on commit 74a91f3

Please sign in to comment.