Skip to content

Commit

Permalink
Allow to keep files locally when they match a new prevent sync pattern
Browse files Browse the repository at this point in the history
  • Loading branch information
@vxgmichel
vxgmichel committed Oct 12, 2024
1 parent 863f524 commit d7359bb
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 20 deletions.
27 changes: 17 additions & 10 deletions libparsec/crates/client/tests/unit/workspace/merge_folder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,10 @@ async fn remote_only_change(
.base
.children
.insert("child.tmp".parse().unwrap(), child_id);
expected
.children
.insert("child.tmp".parse().unwrap(), child_id);
expected.local_confinement_points.insert(child_id);
expected.remote_confinement_points.insert(child_id);
}
// The local manifest has it confinement points build with an outdated prevent
Expand Down Expand Up @@ -2037,8 +2041,6 @@ async fn local_and_remote_changes(
"outdated_psp_remote_child_matches_new_pattern" => {
let child_id = VlobID::from_hex("a1d7229d7e44418a8a4e4fd821003fd3").unwrap();

// At this point, the prevent sync pattern is not `.tmp`, so `child.tmp`
// is just a non confined regular file.
local
.base
.children
Expand All @@ -2054,6 +2056,11 @@ async fn local_and_remote_changes(
.base
.children
.insert("child.tmp".parse().unwrap(), child_id);
expected.children.insert(
"child.tmp".parse().unwrap(),
child_id,
);
expected.local_confinement_points.insert(child_id);
expected.remote_confinement_points.insert(child_id);
expected.need_sync = false;
expected.updated = remote.updated;
Expand Down Expand Up @@ -2151,15 +2158,15 @@ async fn local_and_remote_changes(
.children
.insert("child-remote-rename.tmp".parse().unwrap(), child_id);

// Surprisingly, the local file gets removed although it would have
// been possible to keep it given the id had both a confined name locally
// and remotely. This is just a side-effect of the prevent sync pattern
// being outdated: it is similar to performing the merge with the old
// pattern, and then applying the new one.
expected
.base
.children
.insert("child-remote-rename.tmp".parse().unwrap(), child_id);
expected.children.insert(
"child-local-rename.tmp".parse().unwrap(),
child_id,
);
expected.local_confinement_points.insert(child_id);
expected.remote_confinement_points.insert(child_id);
expected.need_sync = false;
expected.updated = remote.updated;
Expand Down Expand Up @@ -2256,14 +2263,14 @@ async fn local_and_remote_changes(
remote.children.insert("child.tmp".parse().unwrap(), child_id);
remote.author = local_author;

// Here `child.tmp` is removed, which is probably a bug to fix
// TODO: add a test for applying a prevent sync pattern to a synchronized
// entry containing a `.tmp` suffix.

expected.base.author = local_author;
expected
.base
.children
.insert("child.tmp".parse().unwrap(), child_id);
expected.children.insert("child.tmp".parse().unwrap(), child_id);
expected.local_confinement_points.insert(child_id);
expected.remote_confinement_points.insert(child_id);
expected.need_sync = true;
expected.updated = merge_timestamp;
Expand Down
30 changes: 25 additions & 5 deletions libparsec/crates/types/src/local_manifest/folder.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -392,8 +392,19 @@ impl UnconfinedLocalFolderManifest {
speculative: self.speculative,
};

// Check whether there are existing local confinement entries to restore
if !existing_local_manifest.local_confinement_points.is_empty() {
// Check whether there are existing local entries to restore, either because:
// - 1: They were confined entries, in which case they appear in the local confinement points
// of the existing local manifest. Note that may or maybe not be confined in the new
// manifest, depending on the prevent sync pattern. But we have to restore them anyway.
// We should also be careful to not duplicate the entry ID and perform a rename
// if the entry ID is already present in the new manifest.
// - 2: They weren't confined entries, but have been filtered out due to them being present
// in the remote confinement points of the new manifest. Note that we don't want to restore
// them if they don't match the prevent sync pattern, in order to avoid having a local
// non-confined entry with the same id as a remote confined entry.
if !existing_local_manifest.local_confinement_points.is_empty()
|| !new_manifest.remote_confinement_points.is_empty()
{
// Reverse lookup for new entry ids
let new_entry_ids = HashMap::<_, _>::from_iter(
new_manifest
Expand All @@ -405,17 +416,26 @@ impl UnconfinedLocalFolderManifest {
// Build a map of changes to apply
let mut existing_local_confined_entries: HashMap<_, _> = HashMap::new();
for (name, entry_id) in existing_local_manifest.children.iter() {
// Case 1
if existing_local_manifest
.local_confinement_points
.contains(entry_id)
{
// Insert the locally confined entry in the new manifest
existing_local_confined_entries.insert(name.clone(), Some(*entry_id));

// Perform a rename if the entry id is already present in the new manifest
if let Some(&previous_name) = new_entry_ids.get(entry_id) {
existing_local_confined_entries.insert(previous_name.clone(), None);
}

// Insert the locally confined entry in the new manifest
existing_local_confined_entries.insert(name.clone(), Some(*entry_id));
}

// Case 2
if !new_entry_ids.contains_key(entry_id)
&& new_manifest.remote_confinement_points.contains(entry_id)
&& prevent_sync_pattern.is_match(name.as_ref())
{
existing_local_confined_entries.insert(name.clone(), Some(*entry_id));
}
}

Expand Down
24 changes: 19 additions & 5 deletions libparsec/crates/types/tests/unit/local_folder_manifest.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1014,13 +1014,22 @@ fn apply_prevent_sync_pattern_with_non_confined_remote_children_matching_future_
lfm.remote_confinement_points,
HashSet::from_iter([VlobID::from_hex("198762BA0C744DC0B45B2B17678C51CE").unwrap()])
);
p_assert_eq!(lfm.local_confinement_points, HashSet::new());
p_assert_eq!(
lfm.local_confinement_points,
HashSet::from_iter([VlobID::from_hex("198762BA0C744DC0B45B2B17678C51CE").unwrap()])
);
p_assert_eq!(
lfm.children,
HashMap::from_iter([(
"file1.png".parse().unwrap(),
VlobID::from_hex("3DF3AC53967C43D889860AE2F459F42B").unwrap(),
),])
HashMap::from_iter([
(
"file1.png".parse().unwrap(),
VlobID::from_hex("3DF3AC53967C43D889860AE2F459F42B").unwrap(),
),
(
"file3.tmp".parse().unwrap(),
VlobID::from_hex("198762BA0C744DC0B45B2B17678C51CE").unwrap(),
),
])
);
p_assert_eq!(lfm.need_sync, true);
// The last update is actually from t2.
Expand Down Expand Up @@ -1322,11 +1331,16 @@ fn apply_prevent_sync_pattern_with_broader_prevent_sync_pattern() {
HashSet::from_iter([
VlobID::from_hex("B0C37F14927244FA8550EDAECEA09E96").unwrap(),
VlobID::from_hex("936DA01F9ABD4d9d80C702AF85C822A8").unwrap(),
VlobID::from_hex("3DF3AC53967C43D889860AE2F459F42B").unwrap(),
])
);
p_assert_eq!(
lfm.children,
HashMap::from_iter([
(
"file1.png".parse().unwrap(),
VlobID::from_hex("3DF3AC53967C43D889860AE2F459F42B").unwrap(),
),
(
"fileA.mp4".parse().unwrap(),
VlobID::from_hex("936DA01F9ABD4d9d80C702AF85C822A8").unwrap(),
Expand Down

0 comments on commit d7359bb

Please sign in to comment.