refactor(backup): delta contributions function no longer takes reversed input

backup/src/delta.rs

@@ -1,11 +1,11 @@
-use std::{borrow::Borrow, fmt};
+use std::{borrow::Borrow, fmt, path::PathBuf};
 
 use serde::{Deserialize, Serialize};
 
 use super::State;
 
 /// Represents the changes relative to the previous backup
-#[derive(Debug, Serialize, Deserialize, Clone, Default)]
+#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq)]
 pub struct Delta {
     /// What files were added/modified in each part of the tarball.
     pub added: State,
@@ -19,7 +19,6 @@ pub struct Delta {
 impl Delta {
     /// Returns whether the delta is empty by checking whether both its added and removed state
     /// return true for their `is_empty`.
-    #[allow(dead_code)]
     pub fn is_empty(&self) -> bool {
         self.added.is_empty() && self.removed.is_empty()
     }
@@ -111,20 +110,19 @@ impl Delta {
         out
     }
 
-    /// Given a chain of deltas, ordered from last to first, calculate the "contribution" for each
-    /// state.
+    /// Given a chain of deltas, calculate the "contribution" for each state.
     ///
-    /// The contribution of a delta in a given chain is defined as the parts of the state produced
-    /// by this chain that are actually provided by this delta. This comes down to calculating the
-    /// strict difference of this delta and all of its successive deltas.
+    /// For each delta, its contribution is the part of its added and removed files that isn't
+    /// overwritten by any of its following deltas.
     pub fn contributions<I>(deltas: I) -> Vec<State>
     where
         I: IntoIterator,
+        I::IntoIter: DoubleEndedIterator,
         I::Item: Borrow<Delta>,
     {
         let mut contributions: Vec<State> = Vec::new();
 
-        let mut deltas = deltas.into_iter();
+        let mut deltas = deltas.into_iter().rev();
 
         if let Some(first_delta) = deltas.next() {
             // From last to first, we calculate the strict difference of the delta with the union of all its
@@ -139,10 +137,47 @@ impl Delta {
             }
         }
 
-        // contributions.reverse();
-
+        contributions.reverse();
         contributions
     }
+
+    /// Append the given files to the directory's list of added files
+    pub fn append_added<I>(&mut self, dir: impl Into<PathBuf>, files: I)
+    where
+        I: IntoIterator,
+        I::Item: Into<PathBuf>,
+    {
+        self.added.append_dir(dir, files);
+    }
+
+    /// Wrapper around the `append_added` method for a builder-style construction of delta's
+    pub fn with_added<I>(mut self, dir: impl Into<PathBuf>, files: I) -> Self
+    where
+        I: IntoIterator,
+        I::Item: Into<PathBuf>,
+    {
+        self.append_added(dir, files);
+        self
+    }
+
+    /// Append the given files to the directory's list of removed files
+    pub fn append_removed<I>(&mut self, dir: impl Into<PathBuf>, files: I)
+    where
+        I: IntoIterator,
+        I::Item: Into<PathBuf>,
+    {
+        self.removed.append_dir(dir, files);
+    }
+
+    /// Wrapper around the `append_removed` method for a builder-style construction of delta's
+    pub fn with_removed<I>(mut self, dir: impl Into<PathBuf>, files: I) -> Self
+    where
+        I: IntoIterator,
+        I::Item: Into<PathBuf>,
+    {
+        self.append_removed(dir, files);
+        self
+    }
 }
 
 impl fmt::Display for Delta {
@@ -153,3 +188,108 @@ impl fmt::Display for Delta {
         write!(f, "+{}-{}", added_count, removed_count)
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_union_disjunct_dirs() {
+        let a = Delta::default()
+            .with_added("dir_added_1", ["file1", "file2"])
+            .with_removed("dir_removed_1", ["file1", "file2"]);
+        let b = Delta::default()
+            .with_added("dir_added_3", ["file1", "file2"])
+            .with_removed("dir_removed_3", ["file1", "file2"]);
+
+        let expected = Delta::default()
+            .with_added("dir_added_1", ["file1", "file2"])
+            .with_added("dir_added_3", ["file1", "file2"])
+            .with_removed("dir_removed_1", ["file1", "file2"])
+            .with_removed("dir_removed_3", ["file1", "file2"]);
+
+        assert_eq!(expected, a.union(&b));
+        assert_eq!(expected, b.union(&a));
+    }
+
+    #[test]
+    fn test_union_disjunct_files() {
+        let a = Delta::default()
+            .with_added("dir_added_1", ["file1", "file2"])
+            .with_removed("dir_removed_1", ["file1", "file2"]);
+        let b = Delta::default()
+            .with_added("dir_added_1", ["file3", "file4"])
+            .with_removed("dir_removed_1", ["file3", "file4"]);
+
+        let expected = Delta::default()
+            .with_added("dir_added_1", ["file1", "file2", "file3", "file4"])
+            .with_removed("dir_removed_1", ["file1", "file2", "file3", "file4"]);
+
+        assert_eq!(expected, a.union(&b));
+        assert_eq!(expected, b.union(&a));
+    }
+
+    #[test]
+    fn test_union_full_revert() {
+        let a = Delta::default().with_added("dir_1", ["file1", "file2"]);
+        let b = Delta::default().with_removed("dir_1", ["file1", "file2"]);
+
+        let expected = Delta::default().with_removed("dir_1", ["file1", "file2"]);
+        assert_eq!(expected, a.union(&b));
+
+        let expected = Delta::default().with_added("dir_1", ["file1", "file2"]);
+        assert_eq!(expected, b.union(&a));
+    }
+
+    #[test]
+    fn test_difference() {
+        let a = Delta::default()
+            .with_added("dir1", ["file1", "file2"])
+            .with_removed("dir1", ["file3", "file4"]);
+        let b = Delta::default()
+            .with_added("dir1", ["file1"])
+            .with_removed("dir1", ["file3"]);
+        let expected = Delta::default()
+            .with_added("dir1", ["file2"])
+            .with_removed("dir1", ["file4"]);
+
+        assert_eq!(a.difference(&b), expected);
+        assert_eq!(b.difference(&a), Delta::default());
+    }
+
+    #[test]
+    fn test_strict_difference() {
+        let a = Delta::default()
+            .with_added("dir1", ["file1", "file2"])
+            .with_removed("dir1", ["file3", "file4"]);
+        let b = Delta::default()
+            .with_added("dir1", ["file1", "file4"])
+            .with_removed("dir1", ["file3"]);
+        let expected = Delta::default().with_added("dir1", ["file2"]);
+
+        assert_eq!(a.strict_difference(&b), expected);
+        assert_eq!(b.strict_difference(&a), Delta::default());
+    }
+
+    #[test]
+    fn test_contributions() {
+        let deltas = [
+            Delta::default().with_added("dir1", ["file4"]),
+            Delta::default().with_added("dir1", ["file1", "file2"]),
+            Delta::default()
+                .with_added("dir1", ["file1"])
+                .with_added("dir2", ["file3"]),
+            Delta::default()
+                .with_added("dir1", ["file2"])
+                .with_removed("dir2", ["file3"]),
+        ];
+        let expected = [
+            State::default().with_dir("dir1", ["file4"]),
+            State::default(),
+            State::default().with_dir("dir1", ["file1"]),
+            State::default().with_dir("dir1", ["file2"]),
+        ];
+
+        assert_eq!(Delta::contributions(deltas), expected);
+    }
+}

backup/src/manager/mod.rs

@@ -229,9 +229,8 @@ where
                     .map(|_| ()),
                     // Incremental backups are exported one by one according to their contribution
                     BackupType::Incremental => {
-                        let contributions = Delta::contributions(
-                            chain.iter().take(index + 1).map(|b| &b.delta).rev(),
-                        );
+                        let contributions =
+                            Delta::contributions(chain.iter().take(index + 1).map(|b| &b.delta));
 
                         let tar_gz = OpenOptions::new()
                             .write(true)
@@ -245,7 +244,6 @@ where
                         // overwritten by their successors anyways.
                         for (contribution, backup) in contributions
                             .iter()
-                            .rev()
                             .zip(chain.iter().take(index + 1))
                             .filter(|(contribution, _)| !contribution.is_empty())
                         {

backup/src/state.rs

@@ -11,7 +11,7 @@ use crate::Delta;
 
 /// Struct that represents a current state for a backup. This struct acts as a smart pointer around
 /// a HashMap.
-#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Default)]
+#[derive(Debug, Clone, Serialize, Deserialize, Default)]
 pub struct State(HashMap<PathBuf, HashSet<PathBuf>>);
 
 impl State {
@@ -49,8 +49,52 @@ impl State {
     pub fn is_empty(&self) -> bool {
         self.0.values().all(|s| s.is_empty())
     }
+
+    pub fn append_dir<I>(&mut self, dir: impl Into<PathBuf>, files: I)
+    where
+        I: IntoIterator,
+        I::Item: Into<PathBuf>,
+    {
+        let dir = dir.into();
+        let files = files.into_iter().map(Into::into);
+
+        if let Some(dir_files) = self.0.get_mut(&dir) {
+            dir_files.extend(files);
+        } else {
+            self.0.insert(dir, files.collect());
+        }
+    }
+
+    pub fn with_dir<I>(mut self, dir: impl Into<PathBuf>, files: I) -> Self
+    where
+        I: IntoIterator,
+        I::Item: Into<PathBuf>,
+    {
+        self.append_dir(dir, files);
+        self
+    }
 }
 
+impl PartialEq for State {
+    fn eq(&self, other: &Self) -> bool {
+        let self_non_empty = self.0.values().filter(|files| !files.is_empty()).count();
+        let other_non_empty = other.0.values().filter(|files| !files.is_empty()).count();
+
+        if self_non_empty != other_non_empty {
+            return false;
+        }
+
+        // If both states have the same number of non-empty directories, then comparing each
+        // directory of one with the other will only be true if their list of non-empty directories
+        // is identical.
+        self.0
+            .iter()
+            .all(|(dir, files)| files.is_empty() || other.0.get(dir).map_or(false, |v| v == files))
+    }
+}
+
+impl Eq for State {}
+
 impl<T> From<T> for State
 where
     T: IntoIterator,
@@ -86,3 +130,33 @@ impl DerefMut for State {
         &mut self.0
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_eq() {
+        let a = State::default().with_dir("dir1", ["file1", "file2"]);
+        let b = State::default().with_dir("dir1", ["file1", "file2"]);
+
+        assert_eq!(a, b);
+
+        let b = b.with_dir("dir2", ["file3"]);
+
+        assert_ne!(a, b);
+    }
+
+    #[test]
+    fn test_eq_empty_dirs() {
+        let a = State::default().with_dir("dir1", ["file1", "file2"]);
+        let b = State::default()
+            .with_dir("dir1", ["file1", "file2"])
+            .with_dir("dir2", Vec::<PathBuf>::new());
+
+        assert_eq!(a, b);
+
+        let b = b.with_dir("dir2", ["file3"]);
+        assert_ne!(a, b);
+    }
+}