Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure bypass_data_preservation operates as intended #2967

Merged
merged 2 commits into from
Nov 13, 2024
Merged

Ensure bypass_data_preservation operates as intended #2967

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fb7dcda
Ensure bypass_data_preservation operates as intended
abraunegg Nov 13, 2024
74a91f3
Update PR
abraunegg Nov 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 22 additions & 35 deletions src/sync.d
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1424,10 +1424,10 @@ class SyncEngine {
if (debugLogging) {addLogEntry("Flagging to delete item locally: " ~ to!string(onedriveJSONItem), ["debug"]);}
idsToDelete ~= [thisItemDriveId, thisItemId];
} else {
// local data protection is configured, safeBackup the local file, passing in if we are performing a --dry-run or not
// 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, renamedPath);
safeBackup(localPathToDelete, dryRun, bypassDataPreservation, renamedPath);
}
} else {
// Flag to ignore
Expand Down Expand Up @@ -2108,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 @@ -2129,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 @@ -2251,18 +2239,18 @@ class SyncEngine {
} 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
// 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"]);}
// Backup this item, passing in if we are performing a --dry-run or not
// 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);
}
}

Expand Down Expand Up @@ -2457,11 +2445,10 @@ class SyncEngine {
if (!testFileHash(newItemPath, databaseItem)) {
// 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
// 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, renamedPath);
safeBackup(newItemPath, dryRun, bypassDataPreservation, renamedPath);
}
}

Expand Down Expand Up @@ -4553,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 @@ -5926,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 @@ -9452,8 +9439,8 @@ class SyncEngine {
// 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);
// 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
Open in desktop
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
Loading