diff --git a/Cargo.lock b/Cargo.lock index 8bc431b6f..15d7149d2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6596,6 +6596,7 @@ version = "0.39.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "958f21e8e7ceb5a1aa7fa87fab28e7c75976e0bfe7e23ff069e0a260f894067d" dependencies = [ + "encoding_rs", "memchr", "serde", "tokio", diff --git a/crates/notify/Cargo.toml b/crates/notify/Cargo.toml index 5fac173eb..6531cf87d 100644 --- a/crates/notify/Cargo.toml +++ b/crates/notify/Cargo.toml @@ -37,7 +37,6 @@ chrono = { workspace = true, features = ["serde"] } futures = { workspace = true } form_urlencoded = { workspace = true } hashbrown = { workspace = true } -quick-xml = { workspace = true, features = ["serialize", "async-tokio"] } rayon = { workspace = true } rumqttc = { workspace = true } rustc-hash = { workspace = true } @@ -49,6 +48,13 @@ tracing = { workspace = true } url = { workspace = true } wildmatch = { workspace = true, features = ["serde"] } +# quick-xml dependencies for custom S3KeyFilter XML deserialization +# Custom deserializer implemented for S3KeyFilter to handle both XML structures: +# - Direct FilterRule elements under S3Key (AWS S3 format) +# - FilterRuleList wrapper format +# This makes quick-xml 0.39.2 work correctly with Filter elements +quick-xml = { workspace = true, features = ["serialize", "serde-types", "encoding"] } + [dev-dependencies] tokio = { workspace = true, features = ["test-util"] } tracing-subscriber = { workspace = true, features = ["env-filter"] } diff --git a/crates/notify/src/global.rs b/crates/notify/src/global.rs index 5b19ebdfe..4d70a571c 100644 --- a/crates/notify/src/global.rs +++ b/crates/notify/src/global.rs @@ -111,15 +111,11 @@ pub mod notifier_global { suffix: &str, target_ids: &[TargetID], ) -> Result<(), NotificationError> { - // Construct pattern, simple splicing of prefixes and suffixes - let mut pattern = String::new(); - if !prefix.is_empty() { - pattern.push_str(prefix); - } - pattern.push('*'); - if !suffix.is_empty() { - pattern.push_str(suffix); - } + // Construct pattern using proper pattern function + let pattern = crate::rules::pattern::new_pattern( + if prefix.is_empty() { None } else { Some(prefix) }, + if suffix.is_empty() { None } else { Some(suffix) }, + ); // Create BucketNotificationConfig let mut bucket_config = BucketNotificationConfig::new(region); diff --git a/crates/notify/src/rules/config.rs b/crates/notify/src/rules/config.rs index e989074b7..8890399b0 100644 --- a/crates/notify/src/rules/config.rs +++ b/crates/notify/src/rules/config.rs @@ -117,7 +117,7 @@ impl BucketNotificationConfig { // The ARN in queue_conf should now have its region set if it was originally empty. // Ensure TargetID can be cloned or extracted correctly. let target_id = queue_conf.arn.target_id.clone(); - let pattern_str = queue_conf.filter.filter_rule_list.pattern(); + let pattern_str = queue_conf.filter.pattern(); rules_map.add_rule_config(&queue_conf.events, pattern_str, target_id); } diff --git a/crates/notify/src/rules/config_test.rs b/crates/notify/src/rules/config_test.rs new file mode 100644 index 000000000..801b60e2b --- /dev/null +++ b/crates/notify/src/rules/config_test.rs @@ -0,0 +1,440 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Integration tests for BucketNotificationConfig +//! +//! This module contains tests that simulate the exact flow from XML configuration +//! to event matching, including filter rules with prefix and suffix. + +use super::*; +use rustfs_s3_common::EventName; +use rustfs_targets::arn::{ARN, TargetID}; +use std::io::Cursor; + +#[cfg(test)] +mod integration_tests { + use super::*; + + fn create_test_arn() -> ARN { + ARN { + partition: "rustfs".to_string(), + service: "sqs".to_string(), + region: "".to_string(), + target_id: TargetID::new("primary".to_string(), "webhook".to_string()), + } + } + + #[test] + fn test_create_arn() { + let arn = create_test_arn(); + + assert_eq!(arn.partition, "rustfs"); + assert_eq!(arn.service, "sqs"); + assert_eq!(arn.region, ""); + assert_eq!(arn.target_id, TargetID::new("primary".to_string(), "webhook".to_string())); + } + + /// Test exact bug scenario from the bug report + #[test] + fn test_bug_report_exact_scenario_xml() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:* + + + + prefix + uploads/ + + + suffix + .csv + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + // Verify pattern is "uploads/*.csv" + let rules_map = config.get_rules_map(); + + // Debug: Print the patterns stored + println!("\nPatterns stored in rules_map:"); + for (event_name, pattern_rules) in rules_map.inner().iter() { + for pattern in pattern_rules.inner().keys() { + println!(" Event: {:?}, Pattern: '{}'", event_name, pattern); + } + } + + // The event should be registered for ObjectCreatedAll and all its expansions + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedAll)); + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedPut)); + + // Test matching "uploads/test.csv" with ObjectCreatedPut + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.csv"); + + println!("Matching 'uploads/test.csv' with ObjectCreatedPut: {} targets", targets.len()); + for target in &targets { + println!(" Target: {:?}", target); + } + + // Should find the target + assert!(!targets.is_empty(), "Should find at least one target"); + + // Test matching "uploads/subdir/file.csv" - should also match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/subdir/file.csv"); + assert!(!targets.is_empty(), "Nested CSV files should match"); + + // Test matching "uploads/test.txt" - should NOT match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.txt"); + assert!(targets.is_empty(), "TXT files should not match"); + + // Test matching "files/test.csv" - should NOT match (wrong prefix) + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "files/test.csv"); + assert!(targets.is_empty(), "Files in wrong prefix should not match"); + } + + /// Test prefix only filter + #[test] + fn test_prefix_only_filter_xml() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:* + + + + + prefix + images/ + + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // Debug: Print the patterns stored + println!("\nPatterns stored in rules_map:"); + for (event_name, pattern_rules) in rules_map.inner().iter() { + for pattern in pattern_rules.inner().keys() { + println!(" Event: {:?}, Pattern: '{}'", event_name, pattern); + } + } + + // Test matching "images/photo.jpg" + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "images/photo.jpg"); + println!("Matching 'images/photo.jpg': {} targets", targets.len()); + assert!(!targets.is_empty(), "Files in images/ should match"); + + // Test matching "uploads/photo.jpg" - should NOT match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/photo.jpg"); + println!("Matching 'uploads/photo.jpg': {} targets", targets.len()); + assert!(targets.is_empty(), "Files not in images/ should not match"); + } + + /// Test suffix only filter + #[test] + fn test_suffix_only_filter_xml() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:* + + + + + suffix + .pdf + + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // Test matching "document.pdf" - should match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "document.pdf"); + assert!(!targets.is_empty(), "PDF files should match"); + + // Test matching "document.txt" - should NOT match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "document.txt"); + assert!(targets.is_empty(), "Non-PDF files should not match"); + } + + /// Test no filter (match all) + #[test] + fn test_no_filter_xml() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:* + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // All files should match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "anything.csv"); + assert!(!targets.is_empty(), "All files should match"); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.txt"); + assert!(!targets.is_empty(), "All files should match"); + } + + /// Test specific event type instead of compound + #[test] + fn test_specific_event_type_xml() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:Put + + + + prefix + uploads/ + + + suffix + .csv + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // Debug: Print all events that have subscribers + println!("\nAll events with subscribers:"); + for (event_name, _pattern_rules) in rules_map.inner().iter() { + println!(" Event: {:?}", event_name); + } + + // Note: has_subscriber uses bitmask logic, so has_subscriber(&ObjectCreatedAll) returns true + // if any ObjectCreated* event is registered. This is intentional for efficiency. + // Only ObjectCreatedPut was explicitly registered in the XML. + + // Should have subscriber for ObjectCreatedPut + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedPut)); + + // Should NOT have subscriber for ObjectCreatedPost (only Put was configured) + assert!(!rules_map.has_subscriber(&EventName::ObjectCreatedPost)); + + // Test matching with Put event + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.csv"); + assert!(!targets.is_empty(), "Put event should match"); + } + + /// Test URL-encoded object keys + #[test] + fn test_url_encoded_keys() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:* + + + + prefix + uploads/ + + + suffix + .csv + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // Test with URL-encoded space: "uploads/my%20file.csv" + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/my%20file.csv"); + // This may or may not match depending on encoding + println!("URL-encoded key 'uploads/my%20file.csv' matched: {}", !targets.is_empty()); + + // Test with unencoded version + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/my file.csv"); + assert!(!targets.is_empty(), "Unencoded key should match"); + } + + /// Test multiple queue configurations + #[test] + fn test_multiple_queue_configs_xml() { + let xml = r#" + + + csv-queue + arn:rustfs:sqs::primary:webhook-csv + s3:ObjectCreated:* + + + + prefix + uploads/ + + + suffix + .csv + + + + + + jpg-queue + arn:rustfs:sqs::primary:webhook-jpg + s3:ObjectCreated:* + + + + + prefix + images/ + + + suffix + .jpg + + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec![ + "arn:rustfs:sqs:ap-northeast-1:primary:webhook-csv".to_string(), + "arn:rustfs:sqs:ap-northeast-1:primary:webhook-jpg".to_string(), + ]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // Test CSV files + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.csv"); + println!("Targets for 'uploads/test.csv': {:?}", targets); + assert!(!targets.is_empty(), "CSV files should match"); + // Should have csv target (ARN: arn:rustfs:sqs::primary:webhook-csv) + let csv_target = TargetID::new("primary".to_string(), "webhook-csv".to_string()); + assert!(targets.contains(&csv_target), "Should find CSV webhook, got: {:?}", targets); + + // Test JPG files + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "images/photo.jpg"); + assert!(!targets.is_empty(), "JPG files should match"); + // Should have jpg target (ARN: arn:rustfs:sqs:primary:webhook-jpg) + let jpg_target = TargetID::new("primary".to_string(), "webhook-jpg".to_string()); + assert!(targets.contains(&jpg_target), "Should find JPG webhook"); + } + + /// Test compound event type expansion + #[test] + fn test_compound_event_expansion_integration() { + let xml = r#" + + + test-queue + arn:rustfs:sqs:ap-northeast-1:primary:webhook + s3:ObjectCreated:* + + + + + prefix + data/ + + + + + +"#; + + let current_region = "ap-northeast-1"; + let arn_list = vec!["arn:rustfs:sqs:ap-northeast-1:primary:webhook".to_string()]; + + let config = BucketNotificationConfig::from_xml(Cursor::new(xml.as_bytes()), current_region, &arn_list).unwrap(); + + let rules_map = config.get_rules_map(); + + // ObjectCreated:* should be expanded to all ObjectCreated events + let event_types = [ + EventName::ObjectCreatedPut, + EventName::ObjectCreatedPost, + EventName::ObjectCreatedCopy, + EventName::ObjectCreatedCompleteMultipartUpload, + EventName::ObjectCreatedPutRetention, + EventName::ObjectCreatedPutLegalHold, + EventName::ObjectCreatedPutTagging, + EventName::ObjectCreatedDeleteTagging, + ]; + + for event_type in event_types { + assert!(rules_map.has_subscriber(&event_type), "Event {:?} should have subscribers", event_type); + + // All should match "data/file.csv" + let targets = rules_map.match_rules(event_type, "data/file.csv"); + assert!(!targets.is_empty(), "Event {:?} should match", event_type); + } + } +} diff --git a/crates/notify/src/rules/mod.rs b/crates/notify/src/rules/mod.rs index b976ddd9c..28608618a 100644 --- a/crates/notify/src/rules/mod.rs +++ b/crates/notify/src/rules/mod.rs @@ -13,8 +13,14 @@ // limitations under the License. mod config; +#[cfg(test)] +mod config_test; pub mod pattern; mod pattern_rules; +#[cfg(test)] +mod pattern_rules_test; +#[cfg(test)] +mod pattern_test; mod rules_map; mod subscriber_index; mod subscriber_snapshot; diff --git a/crates/notify/src/rules/pattern_rules_test.rs b/crates/notify/src/rules/pattern_rules_test.rs new file mode 100644 index 000000000..07db63a28 --- /dev/null +++ b/crates/notify/src/rules/pattern_rules_test.rs @@ -0,0 +1,481 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for PatternRules and RulesMap +//! +//! This module contains comprehensive tests for the rule matching logic +//! used in bucket notifications, specifically testing the flow from +//! configuration to event matching. + +use super::*; +use rustfs_s3_common::EventName; +use rustfs_targets::arn::TargetID; + +#[cfg(test)] +mod pattern_rules_tests { + use super::*; + + /// Test basic pattern rules matching + #[test] + fn test_pattern_rules_basic_matching() { + let mut rules = PatternRules::new(); + let target_id = TargetID::new("test-id".to_string(), "webhook".to_string()); + + // Add a rule for *.csv files + rules.add("*.csv".to_string(), target_id.clone()); + + // Test matching + assert!(rules.match_simple("test.csv")); + assert!(rules.match_simple("uploads/test.csv")); + assert!(!rules.match_simple("test.txt")); + + // Test match_targets + let targets = rules.match_targets("test.csv"); + assert!(targets.contains(&target_id)); + + let targets = rules.match_targets("test.txt"); + assert!(!targets.contains(&target_id)); + } + + /// Test multiple patterns with different targets + #[test] + fn test_multiple_patterns() { + let mut rules = PatternRules::new(); + let csv_target = TargetID::new("csv-target".to_string(), "webhook".to_string()); + let jpg_target = TargetID::new("jpg-target".to_string(), "webhook".to_string()); + + rules.add("*.csv".to_string(), csv_target.clone()); + rules.add("*.jpg".to_string(), jpg_target.clone()); + + // CSV files match csv_target + let targets = rules.match_targets("test.csv"); + assert!(targets.contains(&csv_target)); + assert!(!targets.contains(&jpg_target)); + + // JPG files match jpg_target + let targets = rules.match_targets("test.jpg"); + assert!(!targets.contains(&csv_target)); + assert!(targets.contains(&jpg_target)); + + // Both patterns match for nested paths + let targets = rules.match_targets("uploads/test.csv"); + assert!(targets.contains(&csv_target)); + + let targets = rules.match_targets("images/photo.jpg"); + assert!(targets.contains(&jpg_target)); + } + + /// Test prefix patterns + #[test] + fn test_prefix_patterns() { + let mut rules = PatternRules::new(); + let target_id = TargetID::new("prefix-target".to_string(), "webhook".to_string()); + + rules.add("uploads/*".to_string(), target_id.clone()); + rules.add("images/*".to_string(), target_id.clone()); + + assert!(rules.match_simple("uploads/test.csv")); + assert!(rules.match_simple("uploads/subdir/test.csv")); + assert!(rules.match_simple("images/photo.jpg")); + assert!(!rules.match_simple("documents/test.txt")); + } + + /// Test prefix + suffix pattern (bug scenario) + #[test] + fn test_prefix_suffix_pattern_bug_scenario() { + let mut rules = PatternRules::new(); + let target_id = TargetID::new("webhook-target".to_string(), "webhook".to_string()); + + // This is the pattern generated from prefix="uploads/", suffix=".csv" + rules.add("uploads/*.csv".to_string(), target_id.clone()); + + // These should match + assert!(rules.match_simple("uploads/test.csv")); + assert!(rules.match_simple("uploads/file1.csv")); + assert!(rules.match_simple("uploads/nested/path/file.csv")); + + // These should not match + assert!(!rules.match_simple("uploads/test.txt")); + assert!(!rules.match_simple("files/test.csv")); + + // Verify match_targets returns correct targets + let targets = rules.match_targets("uploads/test.csv"); + assert_eq!(targets.len(), 1); + assert!(targets.contains(&target_id)); + } + + /// Test match_all pattern + #[test] + fn test_match_all_pattern() { + let mut rules = PatternRules::new(); + let target_id = TargetID::new("all-target".to_string(), "webhook".to_string()); + + rules.add("*".to_string(), target_id.clone()); + + assert!(rules.match_simple("anything")); + assert!(rules.match_simple("uploads/test.csv")); + assert!(rules.match_simple("")); + + let targets = rules.match_targets("any_key"); + assert!(targets.contains(&target_id)); + } + + /// Test empty rules + #[test] + fn test_empty_rules() { + let rules = PatternRules::new(); + + assert!(rules.is_empty()); + assert!(!rules.match_simple("anything")); + + let targets = rules.match_targets("test.csv"); + assert!(targets.is_empty()); + } + + /// Test union operation + #[test] + fn test_union() { + let mut rules1 = PatternRules::new(); + let mut rules2 = PatternRules::new(); + + let target1 = TargetID::new("target1".to_string(), "webhook".to_string()); + let target2 = TargetID::new("target2".to_string(), "webhook".to_string()); + + rules1.add("*.csv".to_string(), target1.clone()); + rules2.add("*.jpg".to_string(), target2.clone()); + + let combined = rules1.union(&rules2); + + assert!(combined.match_simple("test.csv")); + assert!(combined.match_simple("test.jpg")); + assert!(!combined.match_simple("test.png")); + + let targets = combined.match_targets("test.csv"); + assert!(targets.contains(&target1)); + } + + /// Test difference operation + #[test] + fn test_difference() { + let mut rules1 = PatternRules::new(); + let mut rules2 = PatternRules::new(); + + let target1 = TargetID::new("target1".to_string(), "webhook".to_string()); + let target2 = TargetID::new("target2".to_string(), "webhook".to_string()); + + // Add same target to multiple patterns in rules1 + rules1.add("*.csv".to_string(), target1.clone()); + rules1.add("*.jpg".to_string(), target1.clone()); + rules1.add("*.txt".to_string(), target1.clone()); + + // Add different target to .jpg pattern in rules2 + rules2.add("*.jpg".to_string(), target2.clone()); + + let diff = rules1.difference(&rules2); + + // After difference: + // *.csv: target1 remains (pattern doesn't exist in rules2) + // *.jpg: target1 remains (target1 != target2, so it's not removed) + // *.txt: target1 remains (pattern doesn't exist in rules2) + assert!(diff.match_simple("test.csv")); + assert!(diff.match_simple("test.jpg")); + assert!(diff.match_simple("test.txt")); + } + + /// Test difference operation removing same target + #[test] + fn test_difference_same_target() { + let mut rules1 = PatternRules::new(); + let mut rules2 = PatternRules::new(); + + let target1 = TargetID::new("target1".to_string(), "webhook".to_string()); + + // Add same target to multiple patterns in rules1 + rules1.add("*.csv".to_string(), target1.clone()); + rules1.add("*.jpg".to_string(), target1.clone()); + rules1.add("*.txt".to_string(), target1.clone()); + + // Add same target to .jpg pattern in rules2 + rules2.add("*.jpg".to_string(), target1.clone()); + + let diff = rules1.difference(&rules2); + + // After difference: + // *.csv: target1 remains (pattern doesn't exist in rules2) + // *.jpg: target1 is removed (same target in both) + // *.txt: target1 remains (pattern doesn't exist in rules2) + assert!(diff.match_simple("test.csv")); + assert!(!diff.match_simple("test.jpg")); + assert!(diff.match_simple("test.txt")); + } + + /// Test remove_pattern + #[test] + fn test_remove_pattern() { + let mut rules = PatternRules::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + rules.add("*.csv".to_string(), target_id.clone()); + rules.add("*.jpg".to_string(), target_id.clone()); + + assert!(rules.match_simple("test.csv")); + assert!(rules.match_simple("test.jpg")); + + // Remove .csv pattern + assert!(rules.remove_pattern("*.csv")); + + assert!(!rules.match_simple("test.csv")); + assert!(rules.match_simple("test.jpg")); + + // Remove non-existent pattern + assert!(!rules.remove_pattern("*.png")); + } +} + +#[cfg(test)] +mod rules_map_tests { + use super::*; + + /// Test basic rule addition and matching + #[test] + fn test_rules_map_basic() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + // Add rule for ObjectCreatedPut with pattern * + rules_map.add_rule_config(&[EventName::ObjectCreatedPut], "*".to_string(), target_id.clone()); + + // Check has_subscriber + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedPut)); + assert!(!rules_map.has_subscriber(&EventName::ObjectRemovedDelete)); + + // Check match_rules + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "any_key"); + assert!(targets.contains(&target_id)); + + // Check event with different name doesn't match + let targets = rules_map.match_rules(EventName::ObjectRemovedDelete, "any_key"); + assert!(!targets.contains(&target_id)); + } + + /// Test compound event expansion + #[test] + fn test_compound_event_expansion() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + // Add rule for ObjectCreatedAll (compound event) + rules_map.add_rule_config(&[EventName::ObjectCreatedAll], "*.csv".to_string(), target_id.clone()); + + // ObjectCreatedAll should be expanded to all ObjectCreated events + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedAll)); + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedPut)); + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedPost)); + assert!(rules_map.has_subscriber(&EventName::ObjectCreatedCopy)); + + // All should match .csv files + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "test.csv"); + assert!(targets.contains(&target_id)); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPost, "test.csv"); + assert!(targets.contains(&target_id)); + } + + /// Test exact bug scenario + #[test] + fn test_bug_scenario_exact_flow() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("webhook-primary".to_string(), "webhook".to_string()); + + // Configuration: Events=["s3:ObjectCreated:*"], prefix="uploads/", suffix=".csv" + // This generates pattern "uploads/*.csv" + rules_map.add_rule_config(&[EventName::ObjectCreatedAll], "uploads/*.csv".to_string(), target_id.clone()); + + // When ObjectCreatedPut event occurs with key "uploads/test.csv" + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.csv"); + + // Should match + assert!(!targets.is_empty(), "Targets should not be empty for matching event"); + assert!(targets.contains(&target_id), "Should find webhook-primary target"); + + // Test non-matching key + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.txt"); + assert!(targets.is_empty(), "Targets should be empty for non-matching key"); + + // Test matching different suffix + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/subdir/file.csv"); + assert!(!targets.is_empty(), "Nested CSV files should also match"); + assert!(targets.contains(&target_id)); + } + + /// Test multiple patterns for same event + #[test] + fn test_multiple_patterns_same_event() { + let mut rules_map = RulesMap::new(); + let csv_target = TargetID::new("csv-target".to_string(), "webhook".to_string()); + let jpg_target = TargetID::new("jpg-target".to_string(), "webhook".to_string()); + + // Add different patterns for same event + rules_map.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), csv_target.clone()); + rules_map.add_rule_config(&[EventName::ObjectCreatedPut], "*.jpg".to_string(), jpg_target.clone()); + + // Both patterns should work + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "test.csv"); + assert!(targets.contains(&csv_target)); + assert!(!targets.contains(&jpg_target)); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "test.jpg"); + assert!(!targets.contains(&csv_target)); + assert!(targets.contains(&jpg_target)); + } + + /// Test prefix only filter + #[test] + fn test_prefix_only_filter() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + // Configuration: prefix="images/", no suffix + // Pattern should be "images/*" + rules_map.add_rule_config(&[EventName::ObjectCreatedAll], "images/*".to_string(), target_id.clone()); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "images/photo.jpg"); + assert!(targets.contains(&target_id)); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/photo.jpg"); + assert!(!targets.contains(&target_id)); + } + + /// Test suffix only filter + #[test] + fn test_suffix_only_filter() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + // Configuration: no prefix, suffix=".pdf" + // Pattern should be "*.pdf" + rules_map.add_rule_config(&[EventName::ObjectCreatedAll], "*.pdf".to_string(), target_id.clone()); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "document.pdf"); + assert!(targets.contains(&target_id)); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "document.txt"); + assert!(!targets.contains(&target_id)); + } + + /// Test empty pattern (no filter) + #[test] + fn test_no_filter() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + // Configuration: no prefix, no suffix + // Pattern should default to "*" (match all) + rules_map.add_rule_config(&[EventName::ObjectCreatedAll], "".to_string(), target_id.clone()); + + // All keys should match + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "anything.csv"); + assert!(targets.contains(&target_id)); + + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "uploads/test.txt"); + assert!(targets.contains(&target_id)); + } + + /// Test different event types + #[test] + fn test_different_event_types() { + let mut rules_map = RulesMap::new(); + let put_target = TargetID::new("put-target".to_string(), "webhook".to_string()); + let delete_target = TargetID::new("delete-target".to_string(), "webhook".to_string()); + + // Add rules for different event types + rules_map.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), put_target.clone()); + rules_map.add_rule_config(&[EventName::ObjectRemovedDelete], "*.csv".to_string(), delete_target.clone()); + + // Put event should match put_target + let targets = rules_map.match_rules(EventName::ObjectCreatedPut, "test.csv"); + assert!(targets.contains(&put_target)); + assert!(!targets.contains(&delete_target)); + + // Delete event should match delete_target + let targets = rules_map.match_rules(EventName::ObjectRemovedDelete, "test.csv"); + assert!(!targets.contains(&put_target)); + assert!(targets.contains(&delete_target)); + } + + /// Test remove_map + #[test] + fn test_remove_map() { + let mut rules_map1 = RulesMap::new(); + let mut rules_map2 = RulesMap::new(); + + let target1 = TargetID::new("target1".to_string(), "webhook".to_string()); + let target2 = TargetID::new("target2".to_string(), "webhook".to_string()); + + rules_map1.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), target1.clone()); + rules_map1.add_rule_config(&[EventName::ObjectCreatedPost], "*.jpg".to_string(), target1.clone()); + + rules_map2.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), target2.clone()); + + // Remove rules_map2 from rules_map1 + rules_map1.remove_map(&rules_map2); + + // ObjectCreatedPut rule should still exist with target1 (different from target2) + let targets = rules_map1.match_rules(EventName::ObjectCreatedPut, "test.csv"); + assert!(targets.contains(&target1)); + assert!(!targets.contains(&target2)); + + // ObjectCreatedPost rule should still exist + let targets = rules_map1.match_rules(EventName::ObjectCreatedPost, "test.jpg"); + assert!(targets.contains(&target1)); + } + + /// Test remove_map removing same target + #[test] + fn test_remove_map_same_target() { + let mut rules_map1 = RulesMap::new(); + let mut rules_map2 = RulesMap::new(); + + let target1 = TargetID::new("target1".to_string(), "webhook".to_string()); + + rules_map1.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), target1.clone()); + rules_map1.add_rule_config(&[EventName::ObjectCreatedPost], "*.jpg".to_string(), target1.clone()); + + rules_map2.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), target1.clone()); + + // Remove rules_map2 from rules_map1 + rules_map1.remove_map(&rules_map2); + + // ObjectCreatedPut rule should be removed (same target and pattern) + let targets = rules_map1.match_rules(EventName::ObjectCreatedPut, "test.csv"); + assert!(targets.is_empty()); + + // ObjectCreatedPost rule should still exist + let targets = rules_map1.match_rules(EventName::ObjectCreatedPost, "test.jpg"); + assert!(targets.contains(&target1)); + } + + /// Test contains_target_id + #[test] + fn test_contains_target_id() { + let mut rules_map = RulesMap::new(); + let target_id = TargetID::new("test-target".to_string(), "webhook".to_string()); + + assert!(!rules_map.contains_target_id(&target_id)); + + rules_map.add_rule_config(&[EventName::ObjectCreatedPut], "*.csv".to_string(), target_id.clone()); + + assert!(rules_map.contains_target_id(&target_id)); + } +} diff --git a/crates/notify/src/rules/pattern_test.rs b/crates/notify/src/rules/pattern_test.rs new file mode 100644 index 000000000..0e3c3a874 --- /dev/null +++ b/crates/notify/src/rules/pattern_test.rs @@ -0,0 +1,166 @@ +// Copyright 2024 RustFS Team +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Pattern matching tests for notification filter rules +//! +//! This module contains tests for the pattern matching logic used in +//! bucket notification filters, specifically testing prefix and suffix +//! filter rules as used in S3 bucket notifications. + +use super::pattern; + +#[cfg(test)] +mod bug_reproduction_tests { + use super::*; + + /// Test case from bug report: + /// - Prefix: "uploads/" + /// - Suffix: ".csv" + /// - Object key: "uploads/test.csv" + #[test] + fn test_uploads_csv_pattern() { + let pattern = pattern::new_pattern(Some("uploads/"), Some(".csv")); + assert_eq!(pattern, "uploads/*.csv"); + assert!(pattern::match_simple(&pattern, "uploads/test.csv")); + assert!(pattern::match_simple(&pattern, "uploads/subdir/file.csv")); + assert!(!pattern::match_simple(&pattern, "uploads/test.txt")); + } + + /// Test prefix only + #[test] + fn test_prefix_only() { + let pattern = pattern::new_pattern(Some("images/"), None); + assert_eq!(pattern, "images/*"); + assert!(pattern::match_simple(&pattern, "images/photo.jpg")); + assert!(pattern::match_simple(&pattern, "images/subdir/photo.png")); + assert!(!pattern::match_simple(&pattern, "documents/photo.jpg")); + } + + /// Test suffix only + #[test] + fn test_suffix_only() { + let pattern = pattern::new_pattern(None, Some(".jpg")); + assert_eq!(pattern, "*.jpg"); + assert!(pattern::match_simple(&pattern, "photo.jpg")); + assert!(pattern::match_simple(&pattern, "uploads/photo.jpg")); + assert!(pattern::match_simple(&pattern, "a/b/photo.jpg")); + assert!(!pattern::match_simple(&pattern, "photo.png")); + } + + /// Test empty pattern (no filter) + #[test] + fn test_empty_pattern() { + let pattern = pattern::new_pattern(None, None); + assert_eq!(pattern, ""); + // Empty pattern should not match anything + assert!(!pattern::match_simple(&pattern, "anything")); + } + + /// Test complex patterns + #[test] + fn test_complex_patterns() { + // Pattern with both prefix and suffix + let pattern = pattern::new_pattern(Some("data/uploads/"), Some(".csv")); + assert_eq!(pattern, "data/uploads/*.csv"); + assert!(pattern::match_simple(&pattern, "data/uploads/test.csv")); + assert!(!pattern::match_simple(&pattern, "data/test.csv")); + + // Pattern with prefix containing special characters + let pattern = pattern::new_pattern(Some("special-chars_123/"), Some(".txt")); + assert_eq!(pattern, "special-chars_123/*.txt"); + assert!(pattern::match_simple(&pattern, "special-chars_123/file.txt")); + } + + /// Test edge cases + #[test] + fn test_edge_cases() { + // Empty prefix and non-empty suffix + let pattern = pattern::new_pattern(Some(""), Some(".csv")); + assert_eq!(pattern, "*.csv"); + + // Non-empty prefix and empty suffix + let pattern = pattern::new_pattern(Some("uploads/"), Some("")); + assert_eq!(pattern, "uploads/*"); + + // Pattern ending with star in prefix + let pattern = pattern::new_pattern(Some("uploads*"), Some(".csv")); + assert_eq!(pattern, "uploads*.csv"); + + // Pattern starting with star in suffix + let pattern = pattern::new_pattern(Some("uploads/"), Some("*csv")); + assert_eq!(pattern, "uploads/*csv"); + } + + /// Test the exact scenario from bug report + #[test] + fn test_bug_report_exact_scenario() { + // Configuration: prefix="uploads/", suffix=".csv" + let prefix = Some("uploads/"); + let suffix = Some(".csv"); + let object_key = "uploads/test.csv"; + + // Generate pattern from filter rules + let pattern = pattern::new_pattern(prefix, suffix); + println!("Generated pattern: '{}'", pattern); + + // Verify pattern is correct + assert_eq!(pattern, "uploads/*.csv"); + + // Test matching + let matches = pattern::match_simple(&pattern, object_key); + println!("Pattern '{}' matches key '{}': {}", pattern, object_key, matches); + + assert!(matches, "Pattern '{}' should match object key '{}'", pattern, object_key); + + // Test other keys + assert!(pattern::match_simple(&pattern, "uploads/file1.csv")); + assert!(pattern::match_simple(&pattern, "uploads/nested/file.csv")); + assert!(!pattern::match_simple(&pattern, "uploads/file.txt")); + assert!(!pattern::match_simple(&pattern, "files/test.csv")); + } + + /// Test with multiple slashes in path + #[test] + fn test_multiple_slashes() { + let pattern = pattern::new_pattern(Some("a/b/c/"), Some(".csv")); + assert_eq!(pattern, "a/b/c/*.csv"); + assert!(pattern::match_simple(&pattern, "a/b/c/test.csv")); + assert!(!pattern::match_simple(&pattern, "a/b/test.csv")); + } + + /// Test wildcard behavior + #[test] + fn test_wildcard_behavior() { + // Test that * matches any characters including slashes + let pattern = "uploads/*.csv"; + assert!(pattern::match_simple(pattern, "uploads/test.csv")); + assert!(pattern::match_simple(pattern, "uploads/subdir/test.csv")); + assert!(pattern::match_simple(pattern, "uploads/a/b/c/test.csv")); + + // Test that multiple wildcards work + let pattern = "a*b*c.csv"; + assert!(pattern::match_simple(pattern, "a123b456c.csv")); + assert!(pattern::match_simple(pattern, "abc.csv")); + } + + /// Test match_all pattern + #[test] + fn test_match_all_pattern() { + // S3 uses "*" to match all objects + let pattern = "*"; + assert!(pattern::match_simple(pattern, "anything")); + assert!(pattern::match_simple(pattern, "uploads/test.csv")); + assert!(pattern::match_simple(pattern, "")); + } +} diff --git a/crates/notify/src/rules/xml_config.rs b/crates/notify/src/rules/xml_config.rs index 163547c08..14987f50d 100644 --- a/crates/notify/src/rules/xml_config.rs +++ b/crates/notify/src/rules/xml_config.rs @@ -83,17 +83,20 @@ impl FilterRule { } } -#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq)] -pub struct FilterRuleList { +#[derive(Debug, Serialize, Clone, Default, PartialEq, Eq)] +pub struct S3KeyFilter { #[serde(rename = "FilterRule", default, skip_serializing_if = "Vec::is_empty")] - pub rules: Vec, + pub filter_rule_list: Vec, } -impl FilterRuleList { +impl S3KeyFilter { + /// Validate filter rules for duplicates. + /// According to AWS S3 documentation, there can be at most one prefix + /// and one suffix filter rule per queue configuration. pub fn validate(&self) -> Result<(), ParseConfigError> { let mut has_prefix = false; let mut has_suffix = false; - for rule in &self.rules { + for rule in &self.filter_rule_list { rule.validate()?; if rule.name == "prefix" { if has_prefix { @@ -110,11 +113,19 @@ impl FilterRuleList { Ok(()) } + /// Check if filter rule list is empty. + pub fn is_empty(&self) -> bool { + self.filter_rule_list.is_empty() + } + + /// Generate pattern string from filter rules. + /// This method extracts prefix and suffix values from filter rules + /// and generates a wildcard pattern string for matching object keys. pub fn pattern(&self) -> String { let mut prefix_val: Option<&str> = None; let mut suffix_val: Option<&str> = None; - for rule in &self.rules { + for rule in &self.filter_rule_list { if rule.name == "prefix" { prefix_val = Some(&rule.value); } else if rule.name == "suffix" { @@ -123,16 +134,92 @@ impl FilterRuleList { } pattern::new_pattern(prefix_val, suffix_val) } +} - pub fn is_empty(&self) -> bool { - self.rules.is_empty() +#[derive(Debug, serde::Deserialize)] +struct S3KeyContent { + #[serde(rename = "FilterRule", default)] + filter_rule_list: Vec, + #[serde(rename = "FilterRuleList", default)] + filter_rule_list_wrapper: Option, +} + +#[derive(Debug, serde::Deserialize)] +struct S3KeyFilterRuleList { + #[serde(rename = "FilterRule", default)] + filter_rule_list: Vec, +} + +impl S3KeyContent { + /// Get all filter rules from this S3Key content, handling both direct FilterRule + /// and FilterRuleList wrapper structures + fn get_filter_rules(&self) -> Vec { + // If we have a FilterRuleList wrapper, use that + if let Some(wrapper) = &self.filter_rule_list_wrapper + && !wrapper.filter_rule_list.is_empty() + { + return wrapper.filter_rule_list.clone(); + } + // Otherwise use direct FilterRule list + self.filter_rule_list.clone() } } -#[derive(Debug, Serialize, Deserialize, Clone, Default, PartialEq, Eq)] -pub struct S3KeyFilter { - #[serde(rename = "FilterRuleList", default, skip_serializing_if = "FilterRuleList::is_empty")] - pub filter_rule_list: FilterRuleList, +/// Custom deserializer for S3KeyFilter to handle Filter element correctly. +/// AWS S3 XML structure: ... +impl<'de> Deserialize<'de> for S3KeyFilter { + fn deserialize(deserializer: D) -> Result + where + D: serde::Deserializer<'de>, + { + struct S3KeyFilterVisitor { + filter_rules: Vec, + } + + impl S3KeyFilterVisitor { + fn new() -> Self { + Self { + filter_rules: Vec::new(), + } + } + } + + impl<'de> serde::de::Visitor<'de> for S3KeyFilterVisitor { + type Value = S3KeyFilter; + + fn expecting(&self, formatter: &mut std::fmt::Formatter) -> std::fmt::Result { + write!( + formatter, + "an S3Key filter map with an `S3Key` element containing one or more \ + `FilterRule` children (e.g. ...)" + ) + } + + fn visit_map(mut self, mut map: V) -> Result + where + V: serde::de::MapAccess<'de>, + { + while let Some(key) = map.next_key::()? { + match key.as_str() { + "S3Key" => { + // Parse S3Key content which contains FilterRule(s) + let s3key_content: S3KeyContent = map.next_value()?; + self.filter_rules = s3key_content.get_filter_rules(); + } + _ => { + map.next_value::()?; + } + } + } + + Ok(S3KeyFilter { + filter_rule_list: self.filter_rules, + }) + } + } + + deserializer.deserialize_map(S3KeyFilterVisitor::new()) + } } #[derive(Debug, Serialize, Deserialize, Clone, PartialEq, Eq)] @@ -163,7 +250,7 @@ impl QueueConfig { return Err(ParseConfigError::DuplicateEventName(event.to_string())); } } - self.filter.filter_rule_list.validate()?; + self.filter.validate()?; // Validate ARN (similar to Go's Queue.Validate) // The Go code checks targetList.Exists(q.ARN.TargetID) @@ -241,6 +328,7 @@ pub struct NotificationConfiguration { skip_serializing_if = "Vec::is_empty" )] pub lambda_list: Vec, // Modify: Use a new structure + #[serde( rename = "TopicConfiguration", // Tags for each topic configuration item in XML default,