Skip to content

fix(event): normalize shorthand event names before persisting notifications#67

Merged
overtrue merged 1 commit intomainfrom
fix/issue-65-event-shorthand
Mar 23, 2026
Merged

fix(event): normalize shorthand event names before persisting notifications#67
overtrue merged 1 commit intomainfrom
fix/issue-65-event-shorthand

Conversation

@overtrue
Copy link
Copy Markdown
Contributor

Related issue

Background

rc event add --event put stored put literally in bucket notification configuration. RustFS matches concrete S3 event patterns (for example s3:ObjectCreated:Put), so the shorthand token never matched and notifications were not emitted.

Root cause

parse_event_list() in crates/cli/src/commands/event.rs passed --event values through without normalization.

Solution

  • Added shorthand normalization in event add parsing:
    • put -> s3:ObjectCreated:*
    • get -> s3:ObjectAccessed:*
    • delete -> s3:ObjectRemoved:*
    • replica -> s3:Replication:*
    • ilm -> s3:ObjectTransition:*
  • Kept existing behavior for fully qualified S3 event names.
  • Preserved sorting and deduplication behavior after normalization.

Tests

  • Added unit test: test_parse_event_list_normalizes_shorthand_names

Validation

  • Docker reproduction (before fix): event list showed -> put.
  • Docker verification (after fix): event list shows -> s3:ObjectCreated:*.

CI / Quality checks

  • cargo fmt --all --check
  • cargo clippy --workspace -- -D warnings
  • cargo test --workspace

Copilot AI review requested due to automatic review settings March 23, 2026 19:09
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes rc event add so shorthand event tokens are normalized to concrete S3 event patterns before being persisted, ensuring RustFS notification matching works as expected (closes #65).

Changes:

  • Normalize --event values in parse_event_list() (e.g., puts3:ObjectCreated:*) while keeping fully-qualified event names unchanged.
  • Preserve existing sorting + deduplication behavior after normalization.
  • Add a unit test covering shorthand normalization and case-insensitivity.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +362 to +368
match value.to_ascii_lowercase().as_str() {
"put" => "s3:ObjectCreated:*".to_string(),
"get" => "s3:ObjectAccessed:*".to_string(),
"delete" => "s3:ObjectRemoved:*".to_string(),
"replica" => "s3:Replication:*".to_string(),
"ilm" => "s3:ObjectTransition:*".to_string(),
_ => value.to_string(),
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

normalize_event_name allocates a new String for every event via to_ascii_lowercase(), even when the value is already a fully-qualified S3 event name. Consider using eq_ignore_ascii_case(...) checks (or matching on value with AsciiExt-style comparisons) to avoid the extra allocation in the hot path, especially when many events are provided.

Suggested change
match value.to_ascii_lowercase().as_str() {
"put" => "s3:ObjectCreated:*".to_string(),
"get" => "s3:ObjectAccessed:*".to_string(),
"delete" => "s3:ObjectRemoved:*".to_string(),
"replica" => "s3:Replication:*".to_string(),
"ilm" => "s3:ObjectTransition:*".to_string(),
_ => value.to_string(),
if value.eq_ignore_ascii_case("put") {
"s3:ObjectCreated:*".to_string()
} else if value.eq_ignore_ascii_case("get") {
"s3:ObjectAccessed:*".to_string()
} else if value.eq_ignore_ascii_case("delete") {
"s3:ObjectRemoved:*".to_string()
} else if value.eq_ignore_ascii_case("replica") {
"s3:Replication:*".to_string()
} else if value.eq_ignore_ascii_case("ilm") {
"s3:ObjectTransition:*".to_string()
} else {
value.to_string()

Copilot uses AI. Check for mistakes.
@overtrue
Copy link
Copy Markdown
Contributor Author

Reviewed the automated summary comment. It matches the implemented scope and does not include additional actionable change requests, so no follow-up code changes are needed at this stage.

@overtrue overtrue merged commit 827c3bc into main Mar 23, 2026
19 checks passed
@overtrue overtrue deleted the fix/issue-65-event-shorthand branch March 23, 2026 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

rc event add --event put registers subscription but events never fire

2 participants