From e0fb3e2226ee5e738abc984207751ee948ff630c Mon Sep 17 00:00:00 2001 From: Emmanuel Leblond Date: Fri, 29 Nov 2024 13:48:26 +0100 Subject: [PATCH] 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/ --- .../unit/invite/bootstrap_organization.rs | 2 +- .../client/tests/unit/invite/claimer.rs | 2 +- .../unit/workspace/folder_transactions.rs | 2 +- .../tests/unit/workspace/merge_folder.rs | 8 +- .../client/tests/unit/workspace/utils.rs | 2 +- .../unit/workspace_inbound_sync_monitor.rs | 2 +- .../platform_storage/src/native/workspace.rs | 2 +- .../platform_storage/tests/unit/workspace.rs | 12 +- libparsec/crates/types/src/error.rs | 15 -- .../crates/types/src/prevent_sync_pattern.rs | 127 +++++++-------- .../types/tests/unit/local_folder_manifest.rs | 25 ++- .../types/tests/unit/prevent_sync_pattern.rs | 151 +++++++++++------- libparsec/src/config.rs | 14 +- 13 files changed, 183 insertions(+), 181 deletions(-) diff --git a/libparsec/crates/client/tests/unit/invite/bootstrap_organization.rs b/libparsec/crates/client/tests/unit/invite/bootstrap_organization.rs index 0821852220f..6f1a4fcae45 100644 --- a/libparsec/crates/client/tests/unit/invite/bootstrap_organization.rs +++ b/libparsec/crates/client/tests/unit/invite/bootstrap_organization.rs @@ -27,7 +27,7 @@ fn make_config(env: &TestbedEnv) -> Arc { 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(), }) } diff --git a/libparsec/crates/client/tests/unit/invite/claimer.rs b/libparsec/crates/client/tests/unit/invite/claimer.rs index 9afd8f08cef..947e3b07ac4 100644 --- a/libparsec/crates/client/tests/unit/invite/claimer.rs +++ b/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"); diff --git a/libparsec/crates/client/tests/unit/workspace/folder_transactions.rs b/libparsec/crates/client/tests/unit/workspace/folder_transactions.rs index 07bc031b3fe..fe95ae2f118 100644 --- a/libparsec/crates/client/tests/unit/workspace/folder_transactions.rs +++ b/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 diff --git a/libparsec/crates/client/tests/unit/workspace/merge_folder.rs b/libparsec/crates/client/tests/unit/workspace/merge_folder.rs index 16c3e4ae926..a9f15736dab 100644 --- a/libparsec/crates/client/tests/unit/workspace/merge_folder.rs +++ b/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 diff --git a/libparsec/crates/client/tests/unit/workspace/utils.rs b/libparsec/crates/client/tests/unit/workspace/utils.rs index 0e817c16a7c..16fb722fbc1 100644 --- a/libparsec/crates/client/tests/unit/workspace/utils.rs +++ b/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 } diff --git a/libparsec/crates/client/tests/unit/workspace_inbound_sync_monitor.rs b/libparsec/crates/client/tests/unit/workspace_inbound_sync_monitor.rs index 9b56f3a0583..3de6eaa0617 100644 --- a/libparsec/crates/client/tests/unit/workspace_inbound_sync_monitor.rs +++ b/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( diff --git a/libparsec/crates/platform_storage/src/native/workspace.rs b/libparsec/crates/platform_storage/src/native/workspace.rs index ecda1fe14b1..934812e72fe 100644 --- a/libparsec/crates/platform_storage/src/native/workspace.rs +++ b/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) }) diff --git a/libparsec/crates/platform_storage/tests/unit/workspace.rs b/libparsec/crates/platform_storage/tests/unit/workspace.rs index 55b9d70765d..f8975c29b6c 100644 --- a/libparsec/crates/platform_storage/tests/unit/workspace.rs +++ b/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(®ex).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(®ex) diff --git a/libparsec/crates/types/src/error.rs b/libparsec/crates/types/src/error.rs index 67e5378ac07..08cd7e3f22a 100644 --- a/libparsec/crates/types/src/error.rs +++ b/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 = Result; - #[derive(Error, Debug, Clone, PartialEq, Eq)] pub enum DataError { #[error("Invalid encryption")] diff --git a/libparsec/crates/types/src/prevent_sync_pattern.rs b/libparsec/crates/types/src/prevent_sync_pattern.rs index 3e9dfd4c1a3..4396c6aa417 100644 --- a/libparsec/crates/types/src/prevent_sync_pattern.rs +++ b/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 = Result; #[derive(Clone, Debug)] pub struct PreventSyncPattern(pub Vec); @@ -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 { - 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, R: BufRead>( - path: P, - reader: R, - ) -> PreventSyncPatternResult { - 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 { + 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::>>() - .map(Self) + .collect::>()?; + 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 { - Ok(Self( - raw_regexes - .iter() - .map(|l| { - regex::Regex::new(l).map_err(|err| PreventSyncPatternError::ParseError { err }) - }) - .collect::, PreventSyncPatternError>>()?, - )) + /// Create a prevent sync pattern from a regex (regular expression, e.g. `^.*\.(txt|md)$`) + pub fn from_regex(regex: &str) -> PreventSyncPatternResult { + 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 { - 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 { + regex_from_glob_pattern(pattern).map(|re| Self(vec![re])) } - pub fn from_regex_str(regex_str: &str) -> PreventSyncPatternResult { - Self::from_raw_regexes(&[regex_str]) + pub fn from_multiple_globs<'a>( + patterns: impl Iterator, + ) -> PreventSyncPatternResult { + let regexes = patterns + .map(regex_from_glob_pattern) + .collect::>()?; + 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 { - 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 { + fnmatch_regex::glob_to_regex(pattern).map_err(PreventSyncPatternError::BadGlob) } impl PartialEq for PreventSyncPattern { diff --git a/libparsec/crates/types/tests/unit/local_folder_manifest.rs b/libparsec/crates/types/tests/unit/local_folder_manifest.rs index 41f1e0dc59a..3b8ca1ad932 100644 --- a/libparsec/crates/types/tests/unit/local_folder_manifest.rs +++ b/libparsec/crates/types/tests/unit/local_folder_manifest.rs @@ -301,7 +301,7 @@ fn from_remote( let lfm = LocalFolderManifest::from_remote( fm.clone(), - &PreventSyncPattern::from_regex_str(regex).unwrap(), + &PreventSyncPattern::from_regex(regex).unwrap(), ); p_assert_eq!(lfm.base, fm); @@ -477,7 +477,7 @@ fn from_remote_with_restored_local_confinement_points( let lfm = LocalFolderManifest::from_remote_with_restored_local_confinement_points( fm.clone(), - &PreventSyncPattern::from_regex_str(regex).unwrap(), + &PreventSyncPattern::from_regex(regex).unwrap(), &lfm, timestamp, ); @@ -695,8 +695,7 @@ fn evolve_children_and_mark_updated( #[case] expected_local_confinement_points: HashSet, #[case] expected_need_sync: bool, ) { - let prevent_sync_pattern = - PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_TMP).unwrap(); + let prevent_sync_pattern = PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_TMP).unwrap(); let t1 = "2000-01-01T00:00:00Z".parse().unwrap(); let t2 = "2000-01-02T00:00:00Z".parse().unwrap(); @@ -800,7 +799,7 @@ fn apply_prevent_sync_pattern_nothing_confined() { // New prevent sync pattern doesn't match any entry, so nothing should change let new_prevent_sync_pattern = - PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_TMP).unwrap(); + PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_TMP).unwrap(); let lfm2 = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!(lfm2, lfm); @@ -881,7 +880,7 @@ fn apply_prevent_sync_pattern_stability_with_confined() { // We re-apply the same `\.tmp$` prevent sync pattern, so nothing should change let current_prevent_sync_pattern = - PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_TMP).unwrap(); + PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_TMP).unwrap(); let lfm2 = lfm.apply_prevent_sync_pattern(¤t_prevent_sync_pattern, t3); p_assert_eq!(lfm2, lfm); @@ -930,7 +929,7 @@ fn apply_prevent_sync_pattern_with_non_confined_local_children_matching_future_p // Now we change the prevent sync pattern to something that matches some of the local children let new_prevent_sync_pattern = - PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_TMP).unwrap(); + PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_TMP).unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!(lfm.remote_confinement_points, HashSet::new()); @@ -1015,7 +1014,7 @@ fn apply_prevent_sync_pattern_with_non_confined_remote_children_matching_future_ // Now we change the prevent sync pattern to something that matches some of the remote children let new_prevent_sync_pattern = - PreventSyncPattern::from_regex_str(PREVENT_SYNC_PATTERN_TMP).unwrap(); + PreventSyncPattern::from_regex(PREVENT_SYNC_PATTERN_TMP).unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!( @@ -1079,7 +1078,7 @@ fn apply_prevent_sync_pattern_with_confined_local_children_turning_non_confined( // The new prevent sync pattern doesn't match any entry, hence `fileA.tmp` is // no longer confined, hence manifest's `updated` field should also get updated. - let new_prevent_sync_pattern = PreventSyncPattern::from_regex_str(r"\.tmp~$").unwrap(); + let new_prevent_sync_pattern = PreventSyncPattern::from_regex(r"\.tmp~$").unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!(lfm.remote_confinement_points, HashSet::new()); @@ -1157,7 +1156,7 @@ fn apply_prevent_sync_pattern_with_local_changes_and_confined_remote_children_tu // Manifest is still need sync due to the removal of `file2.txt`, however the // `updated` shouldn't has changed since the change in confinement is on // an entry that is already in remote manifest ! - let new_prevent_sync_pattern = PreventSyncPattern::from_regex_str(r"\.tmp~$").unwrap(); + let new_prevent_sync_pattern = PreventSyncPattern::from_regex(r"\.tmp~$").unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!(lfm.remote_confinement_points, HashSet::new()); @@ -1226,7 +1225,7 @@ fn apply_prevent_sync_pattern_with_only_confined_remote_children_turning_non_con // The new prevent sync pattern doesn't match any entry, hence `file3.tmp` is // no longer confined, but manifest doesn't need to be sync since this // entry is already in remote manifest ! - let new_prevent_sync_pattern = PreventSyncPattern::from_regex_str(r"\.tmp~$").unwrap(); + let new_prevent_sync_pattern = PreventSyncPattern::from_regex(r"\.tmp~$").unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t2); p_assert_eq!(lfm.remote_confinement_points, HashSet::new()); @@ -1314,7 +1313,7 @@ fn apply_prevent_sync_pattern_with_broader_prevent_sync_pattern() { // `.+` is a superset of the previous `\.tmp$` pattern, all entries should // be confined now - let new_prevent_sync_pattern = PreventSyncPattern::from_regex_str(".+").unwrap(); + let new_prevent_sync_pattern = PreventSyncPattern::from_regex(".+").unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!( @@ -1441,7 +1440,7 @@ fn apply_prevent_sync_pattern_on_renamed_entry( }; // Now apply the new prevent sync pattern... - let new_prevent_sync_pattern = PreventSyncPattern::from_regex_str(r"\.tmp~$").unwrap(); + let new_prevent_sync_pattern = PreventSyncPattern::from_regex(r"\.tmp~$").unwrap(); let lfm = lfm.apply_prevent_sync_pattern(&new_prevent_sync_pattern, t3); p_assert_eq!( diff --git a/libparsec/crates/types/tests/unit/prevent_sync_pattern.rs b/libparsec/crates/types/tests/unit/prevent_sync_pattern.rs index 93705fa8159..9886429e0af 100644 --- a/libparsec/crates/types/tests/unit/prevent_sync_pattern.rs +++ b/libparsec/crates/types/tests/unit/prevent_sync_pattern.rs @@ -1,92 +1,123 @@ // Parsec Cloud (https://parsec.cloud) Copyright (c) BUSL-1.1 2016-present Scille SAS -use std::{ - io::{BufReader, Cursor}, - path::Path, -}; - use libparsec_tests_lite::prelude::*; -use crate::prevent_sync_pattern::PreventSyncPattern; +use crate::{prevent_sync_pattern::PreventSyncPattern, PreventSyncPatternError}; #[rstest] -#[case::base("*.rs\n*.py", "base.tmp")] -#[case::trim_whitespace(" *.rs\n *.py ", "trim_whitespace.tmp")] -#[case::empty_lines("*.rs\n\n\n*.py", "empty_lines.tmp")] -#[case::trim_whitespace_and_empty_lines( - " *.rs\n\n \n\n*.py ", - "trim_whitespace_and_empty_lines.tmp" -)] -#[case::ignore_comment( - "# This contains patterns\n## yes\n *.rs\n\n \n\n*.py ", - "ignore_comment.tmp" -)] -fn from_pattern_file_content(#[case] file_content: &str, #[case] filename: &str) { - let reader = Cursor::new(file_content.to_string()); - let regex = PreventSyncPattern::from_glob_reader(filename, BufReader::new(reader)) - .expect("Regex should be valid"); - - assert!(regex.is_match("file.py")); - assert!(regex.is_match("file.rs")); - assert!(!regex.is_match("stories.txt")); +#[case::base("*.rs\n*.py")] +#[case::trim_whitespace(" *.rs\n *.py ")] +#[case::empty_lines("*.rs\n\n\n*.py")] +#[case::trim_whitespace_and_empty_lines(" *.rs\n\n \n\n*.py ")] +#[case::ignore_comment("# This contains patterns\n## yes\n *.rs\n\n \n\n*.py ")] +fn from_glob_ignore_file(#[case] file_content: &str) { + let pattern = PreventSyncPattern::from_glob_ignore_file(file_content).unwrap(); + + assert!(pattern.is_match("file.py")); + assert!(pattern.is_match("file.rs")); + assert!(!pattern.is_match("stories.txt")); } #[test] fn load_default_pattern_file() { - let regex = PreventSyncPattern::from_file(Path::new("src/default_pattern.ignore")) - .expect("Load default pattern file failed"); + let pattern = PreventSyncPattern::default(); - for pattern in &[ + for candidate in &[ "file.swp", "file~", + "~$file.docx", "$RECYCLE.BIN", "desktop.ini", "shortcut.lnk", ] { - assert!(regex.is_match(pattern), "Pattern {} should match", pattern); + assert!(pattern.is_match(candidate), "{:?} should match", candidate); } - for pattern in &["secrets.txt", "a.docx", "picture.png"] { + for candidate in &["secrets.txt", "a.docx", "picture.png"] { assert!( - !regex.is_match(pattern), - "Pattern {} should not match", - pattern + !pattern.is_match(candidate), + "{:?} should not match", + candidate ); } } #[rstest] -#[case("*.rs", "foo.rs", "bar.py", "bar.rs")] -#[case("*bar*", "foobar.rs", "dummy", "foobarrrrr.py")] -#[case("$myf$le.*", "$myf$le.txt", "bar", "$myf$le.rs")] -fn wildcard_pattern( - #[case] pattern_str: &str, - #[case] valid_case: &str, - #[case] bad_base: &str, - #[case] other_case: &str, -) { - let regex = PreventSyncPattern::from_glob_pattern(pattern_str).expect("Should be valid"); - assert!(regex.is_match(valid_case)); - assert!(!regex.is_match(bad_base)); - assert!(regex.is_match(other_case)); +#[case::match_wildcard("*.rs", "foo.rs", true)] +#[case::no_match_wildcard_differs("*.rs", "foo.py", false)] +#[case::match_wildcard_literal("*.rs", "*.rs", true)] +#[case::no_match_sub_extension("*.rs", "foo.rs.py", false)] +#[case::match_sub_extension("*.tar.gz", "foo.tar.gz", true)] +#[case::no_match_missing_wildcard(".py", "foo.py", false)] +#[case::match_with_wildcard_empty("*.rs", ".rs", true)] +#[case::match_multi_wildcard("*bar*", "foobar.rs", true)] +#[case::match_multi_wildcard_empty("*bar*", "bar", true)] +#[case::match_multi_wildcard_suffix("*bar*", "foo.bar", true)] +#[case::no_match_multi_wildcard("*bar*", "dummy", false)] +#[case::match_question("fo?bar", "foobar", true)] +#[case::match_question_literal("fo?bar", "fo?bar", true)] +#[case::no_match_question_missing("fo?bar", "fobar", false)] // cspell:disable-line +#[case::match_caret("^foo.bar^", "^foo.bar^", true)] +#[case::no_match_caret("^foo", "foo", false)] +#[case::match_dollar("$foo.bar$", "$foo.bar$", true)] +#[case::no_match_dollar("foo$", "foo", false)] +#[case::no_match_empty("", "foo", false)] +#[case::match_empty("", "", true)] +fn glob_pattern(#[case] glob: &str, #[case] candidate: &str, #[case] expected_match: bool) { + let pattern = PreventSyncPattern::from_glob(glob).unwrap(); + p_assert_eq!(pattern.is_match(candidate), expected_match); +} + +#[test] +fn from_multiple_globs() { + let pattern = + PreventSyncPattern::from_multiple_globs(["", "foo", "*.bar"].into_iter()).unwrap(); + + assert!(pattern.is_match("")); + assert!(pattern.is_match("foo")); + assert!(pattern.is_match("foo.bar")); + + assert!(!pattern.is_match(".foo")); } #[rstest] -#[case(r"fooo?$", "foo", "fo", "fooo")] -fn regex_pattern( - #[case] regex_str: &str, - #[case] valid_case: &str, - #[case] bad_base: &str, - #[case] other_case: &str, -) { - let regex = PreventSyncPattern::from_regex_str(regex_str).expect("Should be valid"); - assert!(regex.is_match(valid_case)); - assert!(!regex.is_match(bad_base)); - assert!(regex.is_match(other_case)); +#[case::match_simple(r"fooo?$", "foo", true)] +#[case::match_complex(r"^spam.*fooo?$", "spambarfoo", true)] // cspell:disable-line +#[case::no_match_simple(r"fooo?$", "fo", false)] +fn regex_pattern(#[case] regex: &str, #[case] candidate: &str, #[case] expected_match: bool) { + let pattern = PreventSyncPattern::from_regex(regex).unwrap(); + p_assert_eq!(pattern.is_match(candidate), expected_match); +} + +#[test] +fn bad_regex() { + p_assert_matches!( + PreventSyncPattern::from_regex(r"fooo][?"), + Err(PreventSyncPatternError::BadRegex { .. }) + ); +} + +#[test] +fn bad_glob() { + p_assert_matches!( + PreventSyncPattern::from_glob(r"fooo][?"), + Err(PreventSyncPatternError::BadGlob { .. }) + ); +} + +#[test] +fn bad_multiple_globs() { + p_assert_matches!( + PreventSyncPattern::from_multiple_globs(["", "fooo][?"].into_iter()), + Err(PreventSyncPatternError::BadGlob { .. }) + ); } #[test] -fn bad_regex_str_creation() { - let r = crate::prevent_sync_pattern::PreventSyncPattern::from_regex_str(r"fooo][?"); - assert!(r.is_err()); +fn display() { + let pattern = PreventSyncPattern::from_multiple_globs(["foo.*", "bar?"].into_iter()).unwrap(); + p_assert_eq!( + format!("{:?}", pattern), + r#"PreventSyncPattern([Regex("^foo\\..*$"), Regex("^bar[^/]$")])"# + ); } diff --git a/libparsec/src/config.rs b/libparsec/src/config.rs index b9ba7cf806e..6f44dcf1912 100644 --- a/libparsec/src/config.rs +++ b/libparsec/src/config.rs @@ -55,11 +55,17 @@ impl From for libparsec_client::ClientConfig { proxy: ProxyConfig::default(), with_monitors: config.with_monitors, prevent_sync_pattern: match config.prevent_sync_pattern { - Some(pattern) => PreventSyncPattern::from_glob_reader( - "client_prevent_sync_pattern", - std::io::Cursor::new(pattern), + Some(custom_glob_ignore) => PreventSyncPattern::from_glob_ignore_file( + &custom_glob_ignore, ) - .expect("Cannot process provided prevent sync pattern file"), + .unwrap_or_else(|err| { + // Fall back to default pattern if the custom pattern is invalid + log::warn!( + "Invalid custom prevent sync pattern, falling back to default: {}", + err + ); + PreventSyncPattern::default() + }), None => PreventSyncPattern::default(), }, }