diff --git a/kuksa_databroker/databroker/src/glob.rs b/kuksa_databroker/databroker/src/glob.rs index 390d810f..72f5a3ea 100644 --- a/kuksa_databroker/databroker/src/glob.rs +++ b/kuksa_databroker/databroker/src/glob.rs @@ -40,21 +40,21 @@ pub fn to_regex_string(glob: &str) -> String { if glob.starts_with("**.") { regex_string = regex_string.replace("**\\.", ".+\\."); } else if glob.starts_with("*.") { - regex_string = regex_string.replace("*\\.", "[^.]+\\."); + regex_string = regex_string.replace("*\\.", "[^.\\s\\:]+\\."); } if glob.ends_with(".*") { - regex_string = regex_string.replace("\\.*", "\\.[^.]+"); + regex_string = regex_string.replace("\\.*", "\\.[^.\\s\\:]+"); } else if glob.ends_with(".**") { regex_string = regex_string.replace("\\.**", "\\..*"); } if glob.contains(".**.") { - regex_string = regex_string.replace("\\.**", "[\\.[A-Z][a-zA-Z0-9]*]*"); + regex_string = regex_string.replace("\\.**", "[\\.[^.\\s\\:]+]*"); } if glob.contains(".*.") { - regex_string = regex_string.replace("\\.*", "\\.[^.]+"); + regex_string = regex_string.replace("\\.*", "\\.[^.\\s\\:]+"); } } @@ -77,7 +77,7 @@ lazy_static! { ^ # anchor at start (only match full paths) # At least one subpath (consisting of either of three): (?: - [A-Z][a-zA-Z0-9]* # An alphanumeric name (first letter capitalized) + [^\.\s\:]+ # Any character except :, whitespace and . | \* # An asterisk | @@ -88,12 +88,12 @@ lazy_static! { (?: \. # Separator, literal dot (?: - [A-Z][a-zA-Z0-9]* # Alphanumeric name (first letter capitalized) + [^\.\s\:]+ # Any character except :, whitespace and . | \* # An asterisk | \*\* # A double asterisk - ) + )+ )* $ # anchor at end (to match only full paths) ", @@ -110,6 +110,9 @@ mod tests { use super::*; static ALL_SIGNALS: &[&str] = &[ + "Vehicle._kuksa.databroker.GitVersion", + "Vehicle._kuksa.databroker.CargoVersion", + "Vehicle._kuksa.databroker.GitVersion", "Vehicle.ADAS.ABS.IsEnabled", "Vehicle.ADAS.ABS.IsEngaged", "Vehicle.ADAS.ABS.IsError", @@ -1250,7 +1253,7 @@ mod tests { #[test] fn test_regex_wildcard_at_end() { assert!(using_glob("Vehicle.Cabin.Sunroof.*") - .should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.]+$")); + .should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.\s\:]+$")); } #[test] @@ -1267,7 +1270,7 @@ mod tests { #[test] fn test_regex_single_wildcard_in_middle() { assert!(using_glob("Vehicle.Cabin.Sunroof.*.Position") - .should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.]+\.Position$")); + .should_equal_regex_pattern(r"^Vehicle\.Cabin\.Sunroof\.[^.\s\:]+\.Position$")); } #[test] @@ -1357,7 +1360,7 @@ mod tests { #[ignore] #[test] fn test_regex_single_wildcard_at_the_beginning() { - assert!(using_glob("*.Sunroof").should_equal_regex_pattern(r"^[^.]+\.Sunroof$")); + assert!(using_glob("*.Sunroof").should_equal_regex_pattern(r"^[^.\s\:]+\.Sunroof$")); } #[test] @@ -1380,6 +1383,17 @@ mod tests { .should_match_no_signals()); } + #[test] + fn test_matches_underscore_cases() { + assert!(using_glob("Vehicle._kuksa.**") + .with_signals(ALL_SIGNALS) + .should_match_signals(&[ + "Vehicle._kuksa.databroker.GitVersion", + "Vehicle._kuksa.databroker.CargoVersion", + "Vehicle._kuksa.databroker.GitVersion", + ],)); + } + #[test] fn test_is_valid_pattern() { assert!(is_valid_pattern("String.*")); @@ -1397,28 +1411,43 @@ mod tests { assert!(is_valid_pattern( "String.String.String.String.*.String.String" )); - assert!(is_valid_pattern("String.*.String.String")); assert!(is_valid_pattern("String.String.**.String.String")); assert!(is_valid_pattern("String.**.String.String")); assert!(is_valid_pattern("**.String.String.String.**")); - assert!(is_valid_pattern("**.String.String.String.*")); + assert!(is_valid_pattern("**.String.String.9_tring.*")); + assert!(is_valid_pattern("**.string.String.String.*")); + assert!(is_valid_pattern("**._string.String.String.*")); + assert!(is_valid_pattern("String._kuksa.tring.9tring.*")); assert!(is_valid_pattern("**.String")); assert!(is_valid_pattern("*.String.String.String")); assert!(is_valid_pattern("*.String")); + assert!(is_valid_pattern("*.String._")); + + assert!(is_valid_pattern("Vehicle.Con_ñ_de_España,_sí")); + assert!(is_valid_pattern("Vehicle.Do_you_not_like_smörgåstårta")); + assert!(is_valid_pattern("Vehicle.tschö_mit_ö")); + assert!(is_valid_pattern("Vehicle.wie_heißt_das_lied")); + assert!(is_valid_pattern("Vehicle.東京_Москва_r#true")); + assert!(!is_valid_pattern("String.String.String.")); assert!(!is_valid_pattern("String.String.String.String..")); assert!(!is_valid_pattern("String.*.String.String..")); assert!(!is_valid_pattern("*.String.String.String..")); + assert!(!is_valid_pattern("String:String.String")); + assert!(!is_valid_pattern("String.**.St ring.String")); + assert!(!is_valid_pattern("String.**:String. String")); + assert!(!is_valid_pattern("String.**.St. .ring.String")); + assert!(!is_valid_pattern("String.**.St. : .ring.String")); } #[test] fn test_valid_regex_path() { - assert_eq!(to_regex_string("String.*"), "^String\\.[^.]+$"); + assert_eq!(to_regex_string("String.*"), "^String\\.[^.\\s\\:]+$"); assert_eq!(to_regex_string("String.**"), "^String\\..*$"); assert_eq!( to_regex_string("String.String.String.String.*"), - "^String\\.String\\.String\\.String\\.[^.]+$" + "^String\\.String\\.String\\.String\\.[^.\\s\\:]+$" ); assert_eq!( to_regex_string("String.String.String.String.**"), @@ -1434,31 +1463,31 @@ mod tests { ); assert_eq!( to_regex_string("String.String.String.*.String"), - "^String\\.String\\.String\\.[^.]+\\.String$" + "^String\\.String\\.String\\.[^.\\s\\:]+\\.String$" ); assert_eq!( to_regex_string("String.String.String.**.String"), - "^String\\.String\\.String[\\.[A-Z][a-zA-Z0-9]*]*\\.String$" + "^String\\.String\\.String[\\.[^.\\s\\:]+]*\\.String$" ); assert_eq!( to_regex_string("String.String.String.String.String.**.String"), - "^String\\.String\\.String\\.String\\.String[\\.[A-Z][a-zA-Z0-9]*]*\\.String$" + "^String\\.String\\.String\\.String\\.String[\\.[^.\\s\\:]+]*\\.String$" ); assert_eq!( to_regex_string("String.String.String.String.*.String.String"), - "^String\\.String\\.String\\.String\\.[^.]+\\.String\\.String$" + "^String\\.String\\.String\\.String\\.[^.\\s\\:]+\\.String\\.String$" ); assert_eq!( to_regex_string("String.*.String.String"), - "^String\\.[^.]+\\.String\\.String$" + "^String\\.[^.\\s\\:]+\\.String\\.String$" ); assert_eq!( to_regex_string("String.String.**.String.String"), - "^String\\.String[\\.[A-Z][a-zA-Z0-9]*]*\\.String\\.String$" + "^String\\.String[\\.[^.\\s\\:]+]*\\.String\\.String$" ); assert_eq!( to_regex_string("String.**.String.String"), - "^String[\\.[A-Z][a-zA-Z0-9]*]*\\.String\\.String$" + "^String[\\.[^.\\s\\:]+]*\\.String\\.String$" ); assert_eq!( to_regex_string("**.String.String.String.**"), @@ -1466,13 +1495,13 @@ mod tests { ); assert_eq!( to_regex_string("**.String.String.String.*"), - "^.+\\.String\\.String\\.String\\.[^.]+$" + "^.+\\.String\\.String\\.String\\.[^.\\s\\:]+$" ); assert_eq!(to_regex_string("**.String"), "^.+\\.String$"); - assert_eq!(to_regex_string("*.String"), "^[^.]+\\.String$"); + assert_eq!(to_regex_string("*.String"), "^[^.\\s\\:]+\\.String$"); assert_eq!( to_regex_string("*.String.String.String"), - "^[^.]+\\.String\\.String\\.String$" + "^[^.\\s\\:]+\\.String\\.String\\.String$" ); } } diff --git a/kuksa_databroker/databroker/src/main.rs b/kuksa_databroker/databroker/src/main.rs index c44c18f9..b20ef722 100644 --- a/kuksa_databroker/databroker/src/main.rs +++ b/kuksa_databroker/databroker/src/main.rs @@ -29,7 +29,7 @@ use tracing::{debug, error, info}; use clap::{Arg, ArgAction, Command}; -use databroker::{broker, grpc, permissions, vss}; +use databroker::{broker, grpc, permissions, vss, glob}; async fn shutdown_handler() { let mut sigint = @@ -111,51 +111,55 @@ async fn read_metadata_file<'a, 'b>( for (path, entry) in entries { debug!("Adding VSS datapoint type {}", path); - match database - .add_entry( - path.clone(), - entry.data_type, - databroker::broker::ChangeType::OnChange, - entry.entry_type, - entry.description, - entry.allowed, - ) - .await - { - Ok(id) => { - if let Some(default) = entry.default { - let ids = [( - id, - broker::EntryUpdate { - datapoint: Some(broker::Datapoint { - ts: std::time::SystemTime::now(), - value: default, - }), - path: None, - actuator_target: None, - entry_type: None, - data_type: None, - description: None, - allowed: None, - }, - )]; - if let Err(errors) = database.update_entries(ids).await { - // There's only one error (since we're only trying to set one) - if let Some(error) = errors.get(0) { - info!("Failed to set default value for {}: {:?}", path, error.1); + if glob::is_valid_pattern(&path) { + match database + .add_entry( + path.clone(), + entry.data_type, + databroker::broker::ChangeType::OnChange, + entry.entry_type, + entry.description, + entry.allowed, + ) + .await + { + Ok(id) => { + if let Some(default) = entry.default { + let ids = [( + id, + broker::EntryUpdate { + datapoint: Some(broker::Datapoint { + ts: std::time::SystemTime::now(), + value: default, + }), + path: None, + actuator_target: None, + entry_type: None, + data_type: None, + description: None, + allowed: None, + }, + )]; + if let Err(errors) = database.update_entries(ids).await { + // There's only one error (since we're only trying to set one) + if let Some(error) = errors.get(0) { + info!("Failed to set default value for {}: {:?}", path, error.1); + } } } } + Err(RegistrationError::PermissionDenied) => { + error!("Failed to add entry {path}: Permission denied") + } + Err(RegistrationError::PermissionExpired) => { + error!("Failed to add entry {path}: Permission expired") + } + Err(RegistrationError::ValidationError) => { + error!("Failed to add entry {path}: Validation failed") + } } - Err(RegistrationError::PermissionDenied) => { - error!("Failed to add entry {path}: Permission denied") - } - Err(RegistrationError::PermissionExpired) => { - error!("Failed to add entry {path}: Permission expired") - } - Err(RegistrationError::ValidationError) => { - error!("Failed to add entry {path}: Validation failed") - } + } else { + info!("Not valid path pattern for: {}", path); } } Ok(())