Improve PreventSyncPattern type

- Remove useless $ escape code (already handled by `fnmatch_regex`)
- Fix panic in libparsec bindings when config contains invalid pattern
- Remove parsing from file to simplify API
- More consistent naming in methods
- Add more tests \o/

libparsec/crates/client/tests/unit/invite/bootstrap_organization.rs

@@ -27,7 +27,7 @@ fn make_config(env: &TestbedEnv) -> Arc<ClientConfig> {
         proxy: ProxyConfig::default(),
         mountpoint_mount_strategy: MountpointMountStrategy::Disabled,
         with_monitors: false,
-        prevent_sync_pattern: PreventSyncPattern::from_regex_str(r"\.tmp$").unwrap(),
+        prevent_sync_pattern: PreventSyncPattern::from_regex(r"\.tmp$").unwrap(),
     })
 }
 

libparsec/crates/client/tests/unit/invite/claimer.rs

@@ -30,7 +30,7 @@ async fn claimer(tmp_path: TmpPath, env: &TestbedEnv) {
         proxy: ProxyConfig::default(),
         mountpoint_mount_strategy: MountpointMountStrategy::Disabled,
         with_monitors: false,
-        prevent_sync_pattern: PreventSyncPattern::from_regex_str(r"\.tmp$").unwrap(),
+        prevent_sync_pattern: PreventSyncPattern::from_regex(r"\.tmp$").unwrap(),
     });
 
     let alice = env.local_device("alice@dev1");

libparsec/crates/client/tests/unit/workspace/folder_transactions.rs

@@ -466,7 +466,7 @@ async fn bootstrap_env(env: &TestbedEnv, root_level: bool) -> Env {
 
     let (base_path, parent_id) = env
         .customize(|builder| {
-            let prevent_sync_pattern = PreventSyncPattern::from_regex_str(r"\.tmp$").unwrap();
+            let prevent_sync_pattern = PreventSyncPattern::from_regex(r"\.tmp$").unwrap();
             let res = if root_level {
                 // Remove all children from the workspace
                 builder

libparsec/crates/client/tests/unit/workspace/merge_folder.rs

@@ -20,7 +20,7 @@ async fn no_remote_change(
     kind: &str,
     env: &TestbedEnv,
 ) {
-    let prevent_sync_pattern = PreventSyncPattern::from_glob_pattern("*.tmp").unwrap();
+    let prevent_sync_pattern = PreventSyncPattern::from_glob("*.tmp").unwrap();
     let local_author = "alice@dev1".parse().unwrap();
     let timestamp = "2021-01-10T00:00:00Z".parse().unwrap();
     let vlob_id = VlobID::from_hex("87c6b5fd3b454c94bab51d6af1c6930b").unwrap();
@@ -175,7 +175,7 @@ async fn no_remote_change_but_local_uses_outdated_prevent_sync_pattern(
         unknown => panic!("Unknown kind: {}", unknown),
     }
 
-    let new_prevent_sync_pattern = PreventSyncPattern::from_glob_pattern("*.tmp").unwrap();
+    let new_prevent_sync_pattern = PreventSyncPattern::from_glob("*.tmp").unwrap();
     let outcome = merge_local_folder_manifest(
         local_author,
         timestamp,
@@ -263,7 +263,7 @@ async fn remote_only_change(
         speculative: false,
     };
 
-    let prevent_sync_pattern = PreventSyncPattern::from_glob_pattern("*.tmp").unwrap();
+    let prevent_sync_pattern = PreventSyncPattern::from_glob("*.tmp").unwrap();
     let child_id = VlobID::from_hex("1040c4845fd1451b9c243c93991d9a5e").unwrap();
     let confined_id = VlobID::from_hex("9100fa0bfca94e4d96077dd274a243c0").unwrap();
     match kind {
@@ -658,7 +658,7 @@ async fn local_and_remote_changes(
         speculative: false,
     };
 
-    let prevent_sync_pattern = PreventSyncPattern::from_glob_pattern("*.tmp").unwrap();
+    let prevent_sync_pattern = PreventSyncPattern::from_glob("*.tmp").unwrap();
     match kind {
         "only_updated_field_modified" => {
             // Since only `updated` has been modified on local, then

libparsec/crates/client/tests/unit/workspace/utils.rs

@@ -20,7 +20,7 @@ pub(crate) async fn workspace_ops_factory(
         discriminant_dir,
         device,
         realm_id,
-        PreventSyncPattern::from_regex_str(r"\.tmp$").unwrap(),
+        PreventSyncPattern::from_regex(r"\.tmp$").unwrap(),
     )
     .await
 }

libparsec/crates/client/tests/unit/workspace_inbound_sync_monitor.rs

@@ -188,7 +188,7 @@ async fn real_io_provides_a_starting_event(env: &TestbedEnv) {
         workspace_storage_cache_size: WorkspaceStorageCacheSize::Default,
         proxy: ProxyConfig::default(),
         with_monitors: false,
-        prevent_sync_pattern: PreventSyncPattern::from_regex_str(r"\.tmp$").unwrap(),
+        prevent_sync_pattern: PreventSyncPattern::from_regex(r"\.tmp$").unwrap(),
     });
     let event_bus = EventBus::default();
     let cmds = Arc::new(

libparsec/crates/platform_storage/src/native/workspace.rs

@@ -443,7 +443,7 @@ impl PlatformWorkspaceStorage {
         db_get_prevent_sync_pattern(&mut self.conn)
             .await
             .and_then(|(pattern, fully_applied)| {
-                PreventSyncPattern::from_regex_str(&pattern)
+                PreventSyncPattern::from_regex(&pattern)
                     .map(|re| (re, fully_applied))
                     .map_err(anyhow::Error::from)
             })

libparsec/crates/platform_storage/tests/unit/workspace.rs

@@ -1109,7 +1109,7 @@ async fn check_prevent_sync_pattern_initialized_with_empty_pattern(env: &Testbed
 
     p_assert_eq!(
         regex,
-        PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_EMPTY_PATTERN).unwrap()
+        PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_EMPTY_PATTERN).unwrap()
     );
     assert!(!bool);
 }
@@ -1118,8 +1118,7 @@ async fn check_prevent_sync_pattern_initialized_with_empty_pattern(env: &Testbed
 async fn mark_empty_pattern_as_fully_applied(env: &TestbedEnv) {
     let mut workspace = start_workspace(env).await;
 
-    let empty_pattern =
-        PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_EMPTY_PATTERN).unwrap();
+    let empty_pattern = PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_EMPTY_PATTERN).unwrap();
 
     let res = workspace
         .mark_prevent_sync_pattern_fully_applied(&empty_pattern)
@@ -1134,8 +1133,7 @@ async fn check_set_pattern_is_idempotent(env: &TestbedEnv) {
     let mut workspace = start_workspace(env).await;
 
     // 1st, mark the empty pattern as fully applied.
-    let empty_pattern =
-        PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_EMPTY_PATTERN).unwrap();
+    let empty_pattern = PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_EMPTY_PATTERN).unwrap();
 
     let res = workspace
         .mark_prevent_sync_pattern_fully_applied(&empty_pattern)
@@ -1156,7 +1154,7 @@ async fn check_set_pattern_is_idempotent(env: &TestbedEnv) {
 async fn set_prevent_sync_pattern(env: &TestbedEnv) {
     let mut workspace = start_workspace(env).await;
 
-    let regex = PreventSyncPattern::from_regex_str(r".*\.tmp$").unwrap();
+    let regex = PreventSyncPattern::from_regex(r".*\.tmp$").unwrap();
 
     let res = workspace.set_prevent_sync_pattern(&regex).await.unwrap();
 
@@ -1171,7 +1169,7 @@ async fn set_prevent_sync_pattern(env: &TestbedEnv) {
 async fn nop_mark_prevent_sync_pattern_with_different_pat(env: &TestbedEnv) {
     let mut workspace = start_workspace(env).await;
 
-    let regex = PreventSyncPattern::from_regex_str(r".*\.tmp$").unwrap();
+    let regex = PreventSyncPattern::from_regex(r".*\.tmp$").unwrap();
 
     let res = workspace
         .mark_prevent_sync_pattern_fully_applied(&regex)

libparsec/crates/types/src/error.rs

@@ -8,21 +8,6 @@ use libparsec_crypto::CryptoError;
 
 use crate::{DateTime, DeviceFileType, DeviceID, HumanHandle, UserID, VlobID};
 
-#[derive(Error, Debug)]
-pub enum PreventSyncPatternError {
-    #[error("Regex parsing err: {err}")]
-    ParseError { err: regex::Error },
-    #[error("Failed to convert glob pattern into regex: {err}")]
-    GlobPatternError { err: fnmatch_regex::error::Error },
-    #[error("IO error on pattern file `{file_path}`: {err}")]
-    PatternFileIOError {
-        file_path: std::path::PathBuf,
-        err: std::io::Error,
-    },
-}
-
-pub type PreventSyncPatternResult<T> = Result<T, PreventSyncPatternError>;
-
 #[derive(Error, Debug, Clone, PartialEq, Eq)]
 pub enum DataError {
     #[error("Invalid encryption")]

libparsec/crates/types/src/prevent_sync_pattern.rs

@@ -1,14 +1,17 @@
 // Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS
 
-use std::{
-    collections::HashSet,
-    fmt::Display,
-    fs,
-    io::{BufRead, BufReader},
-    path::Path,
-};
+use std::{collections::HashSet, fmt::Display};
+use thiserror::Error;
+
+#[derive(Error, Debug)]
+pub enum PreventSyncPatternError {
+    #[error("Regex parsing error: {0}")]
+    BadRegex(regex::Error),
+    #[error("Glob parsing error: {0}")]
+    BadGlob(fnmatch_regex::error::Error),
+}
 
-use crate::{PreventSyncPatternError, PreventSyncPatternResult};
+pub type PreventSyncPatternResult<T> = Result<T, PreventSyncPatternError>;
 
 #[derive(Clone, Debug)]
 pub struct PreventSyncPattern(pub Vec<regex::Regex>);
@@ -17,95 +20,75 @@ const DEFAULT_PREVENT_SYNC_PATTERN: &str = std::include_str!("default_pattern.ig
 
 impl Default for PreventSyncPattern {
     fn default() -> Self {
-        Self::from_glob_reader(
-            stringify!(DEFAULT_PREVENT_SYNC_PATTERN),
-            std::io::Cursor::new(DEFAULT_PREVENT_SYNC_PATTERN),
-        )
-        .expect("Cannot parse default prevent sync pattern")
+        Self::from_glob_ignore_file(DEFAULT_PREVENT_SYNC_PATTERN)
+            .expect("Cannot parse default prevent sync pattern")
     }
 }
 
-/// The fnmatch_regex crate does not escape the character '$'. It is a problem
-/// for us. Since we need to match strings like `$RECYCLE.BIN` we need to escape
-/// it anyway.
-fn escape_globing_pattern(string: &str) -> String {
-    str::replace(string, "$", "\\$")
-}
-
 impl PreventSyncPattern {
-    /// Returns a regex which is built from a file that contains shell like patterns
-    pub fn from_file(file_path: &Path) -> PreventSyncPatternResult<Self> {
-        let reader = fs::File::open(file_path).map(BufReader::new).map_err(|e| {
-            PreventSyncPatternError::PatternFileIOError {
-                file_path: file_path.to_path_buf(),
-                err: e,
-            }
-        })?;
-
-        Self::from_glob_reader(file_path, reader)
-    }
-
-    pub fn from_glob_reader<P: AsRef<Path>, R: BufRead>(
-        path: P,
-        reader: R,
-    ) -> PreventSyncPatternResult<Self> {
-        reader
+    /// Glob ignore file format is simply:
+    /// - Each line contains either a glob pattern of a comment
+    /// - Each line is first trimmed of leading/trailing whitespaces
+    /// - Comment lines start with a `#` character
+    ///
+    /// Example:
+    /// ```raw
+    /// # Ignore C stuff
+    /// *.{so,o}
+    ///
+    /// # Ignore Python stuff
+    /// *.pyc
+    /// ```
+    ///
+    /// (see `default_pattern.ignore` for a more complex example)
+    pub fn from_glob_ignore_file(file_content: &str) -> PreventSyncPatternResult<Self> {
+        let regexes = file_content
             .lines()
-            .filter_map(|line| match line {
-                Ok(line) => {
-                    let l = line.trim();
-
-                    if l != "\n" && !l.starts_with('#') {
-                        Some(from_glob_pattern(l))
-                    } else {
-                        None
-                    }
+            .filter_map(|line| {
+                let line = line.trim();
+                if line.is_empty() || line.starts_with('#') {
+                    None
+                } else {
+                    Some(regex_from_glob_pattern(line))
                 }
-                Err(e) => Some(Err(PreventSyncPatternError::PatternFileIOError {
-                    file_path: path.as_ref().to_path_buf(),
-                    err: e,
-                })),
             })
-            .collect::<PreventSyncPatternResult<Vec<regex::Regex>>>()
-            .map(Self)
+            .collect::<Result<_, _>>()?;
+        Ok(Self(regexes))
     }
 
-    /// Returns a regex which is an union of all regexes from `raw_regexes` slice parameter
-    pub fn from_raw_regexes(raw_regexes: &[&str]) -> PreventSyncPatternResult<Self> {
-        Ok(Self(
-            raw_regexes
-                .iter()
-                .map(|l| {
-                    regex::Regex::new(l).map_err(|err| PreventSyncPatternError::ParseError { err })
-                })
-                .collect::<Result<Vec<regex::Regex>, PreventSyncPatternError>>()?,
-        ))
+    /// Create a prevent sync pattern from a regex (regular expression, e.g. `^.*\.(txt|md)$`)
+    pub fn from_regex(regex: &str) -> PreventSyncPatternResult<Self> {
+        let regex = regex::Regex::new(regex).map_err(PreventSyncPatternError::BadRegex)?;
+        Ok(Self(vec![regex]))
     }
 
-    /// Returns a regex from a glob pattern
-    pub fn from_glob_pattern(pattern: &str) -> PreventSyncPatternResult<Self> {
-        from_glob_pattern(pattern).map(|re| Self(vec![re]))
+    /// Create a prevent sync pattern from a glob pattern (e.g. `*.{txt,md}`)
+    pub fn from_glob(pattern: &str) -> PreventSyncPatternResult<Self> {
+        regex_from_glob_pattern(pattern).map(|re| Self(vec![re]))
     }
 
-    pub fn from_regex_str(regex_str: &str) -> PreventSyncPatternResult<Self> {
-        Self::from_raw_regexes(&[regex_str])
+    pub fn from_multiple_globs<'a>(
+        patterns: impl Iterator<Item = &'a str>,
+    ) -> PreventSyncPatternResult<Self> {
+        let regexes = patterns
+            .map(regex_from_glob_pattern)
+            .collect::<Result<_, _>>()?;
+        Ok(Self(regexes))
     }
 
     pub fn is_match(&self, string: &str) -> bool {
         self.0.iter().any(|r| r.is_match(string))
     }
 
-    /// Create an empty regex, that regex will never match anything
+    /// Create an empty prevent sync pattern that will never match anything
     pub const fn empty() -> Self {
         Self(Vec::new())
     }
 }
 
 /// Parse a glob pattern like `*.rs` and convert it to an regex.
-fn from_glob_pattern(pattern: &str) -> PreventSyncPatternResult<regex::Regex> {
-    let escaped_str = escape_globing_pattern(pattern);
-    fnmatch_regex::glob_to_regex(&escaped_str)
-        .map_err(|err| PreventSyncPatternError::GlobPatternError { err })
+fn regex_from_glob_pattern(pattern: &str) -> PreventSyncPatternResult<regex::Regex> {
+    fnmatch_regex::glob_to_regex(pattern).map_err(PreventSyncPatternError::BadGlob)
 }
 
 impl PartialEq for PreventSyncPattern {