From 91c099e35ff91b0903047e59eb0ad024a321bb6d Mon Sep 17 00:00:00 2001 From: weisd Date: Mon, 9 Jun 2025 11:29:23 +0800 Subject: [PATCH] add Error test, fix clippy --- cli/rustfs-gui/src/utils/helper.rs | 8 +- .../src/generated/proto_gen/node_service.rs | 6 +- common/protos/src/lib.rs | 2 +- crates/config/src/config.rs | 2 +- crates/event-notifier/examples/simple.rs | 2 +- crates/event-notifier/examples/webhook.rs | 2 +- crates/event-notifier/src/event.rs | 2 +- crates/event-notifier/src/global.rs | 4 +- crates/event-notifier/src/lib.rs | 2 +- crates/event-notifier/src/notifier.rs | 2 +- crates/event-notifier/src/store.rs | 2 +- crates/filemeta/benches/xl_meta_bench.rs | 4 +- crates/filemeta/src/error.rs | 375 ++++++++++++++++++ crates/filemeta/src/filemeta_inline.rs | 6 +- crates/filemeta/src/metacache.rs | 4 +- crates/obs/examples/server.rs | 2 +- crates/obs/src/global.rs | 4 +- crates/obs/src/logger.rs | 2 +- crates/obs/src/sinks/webhook.rs | 2 +- crates/obs/src/system/collector.rs | 6 +- crates/obs/src/system/gpu.rs | 6 +- crates/obs/src/worker.rs | 2 +- crates/rio/src/bitrot.rs | 2 +- crates/rio/src/compress_reader.rs | 2 +- crates/rio/src/hash_reader.rs | 2 +- crates/utils/src/certs.rs | 20 +- crates/utils/src/hash.rs | 2 +- crates/utils/src/os/linux.rs | 8 +- crates/utils/src/os/unix.rs | 6 +- crates/zip/src/lib.rs | 8 +- crypto/src/jwt/decode.rs | 2 +- crypto/src/jwt/encode.rs | 2 +- e2e_test/src/reliant/lock.rs | 22 +- ecstore/src/bucket/error.rs | 2 +- ecstore/src/bucket/metadata_sys.rs | 12 +- ecstore/src/config/com.rs | 6 +- ecstore/src/config/mod.rs | 8 +- ecstore/src/config/storageclass.rs | 18 +- ecstore/src/disk/error.rs | 140 +++++++ ecstore/src/erasure_coding/encode.rs | 22 +- ecstore/src/erasure_coding/erasure.rs | 5 +- ecstore/src/error.rs | 195 ++++++++- ecstore/src/heal/background_heal_ops.rs | 14 +- ecstore/src/heal/data_scanner.rs | 21 +- ecstore/src/heal/data_scanner_metric.rs | 2 +- ecstore/src/heal/data_usage_cache.rs | 2 +- ecstore/src/heal/heal_commands.rs | 2 +- ecstore/src/heal/heal_ops.rs | 12 +- ecstore/src/heal/mrf.rs | 2 +- ecstore/src/notification_sys.rs | 4 +- ecstore/src/peer_rest_client.rs | 2 +- ecstore/src/set_disk.rs | 10 +- ecstore/src/sets.rs | 9 +- ecstore/src/store_api.rs | 10 +- ecstore/src/store_init.rs | 7 +- ecstore/src/utils/wildcard.rs | 2 +- iam/src/error.rs | 194 +++++++++ iam/src/manager.rs | 21 +- iam/src/store.rs | 4 +- iam/src/store/object.rs | 8 +- iam/src/sys.rs | 18 +- iam/src/utils.rs | 2 +- policy/src/auth/credentials.rs | 6 +- policy/src/error.rs | 201 ++++++++++ policy/src/policy/action.rs | 2 +- policy/src/policy/function.rs | 4 +- policy/src/policy/function/addr.rs | 2 +- policy/src/policy/function/bool_null.rs | 2 +- policy/src/policy/function/condition.rs | 8 +- policy/src/policy/function/date.rs | 6 +- policy/src/policy/function/func.rs | 2 +- policy/src/policy/function/number.rs | 2 +- policy/src/policy/function/string.rs | 2 +- policy/src/policy/policy.rs | 4 +- policy/src/policy/principal.rs | 2 +- policy/src/policy/resource.rs | 2 +- policy/src/policy/statement.rs | 4 +- policy/src/policy/utils/path.rs | 6 +- policy/src/utils.rs | 2 +- policy/tests/policy_is_allowed.rs | 4 +- rustfs/src/admin/handlers/group.rs | 2 +- rustfs/src/admin/handlers/policys.rs | 2 +- rustfs/src/admin/handlers/service_account.rs | 2 +- rustfs/src/admin/handlers/sts.rs | 3 +- rustfs/src/admin/handlers/trace.rs | 4 +- rustfs/src/admin/handlers/user.rs | 4 +- rustfs/src/auth.rs | 6 +- rustfs/src/console.rs | 4 +- rustfs/src/error.rs | 235 +++++++++++ rustfs/src/server/mod.rs | 4 +- rustfs/src/server/service_state.rs | 6 +- s3select/api/src/query/dispatcher.rs | 2 +- s3select/api/src/query/execution.rs | 4 +- s3select/api/src/query/session.rs | 6 +- s3select/api/src/server/dbms.rs | 4 +- .../query/src/data_source/table_source.rs | 2 +- s3select/query/src/dispatcher/manager.rs | 6 +- s3select/query/src/execution/factory.rs | 2 +- .../query/src/execution/scheduler/local.rs | 2 +- s3select/query/src/instance.rs | 4 +- s3select/query/src/metadata/mod.rs | 2 +- s3select/query/src/sql/analyzer.rs | 2 +- s3select/query/src/sql/logical/optimizer.rs | 16 +- s3select/query/src/sql/optimizer.rs | 4 +- s3select/query/src/sql/parser.rs | 2 +- s3select/query/src/sql/physical/optimizer.rs | 2 +- s3select/query/src/sql/physical/planner.rs | 4 +- s3select/query/src/sql/planner.rs | 2 +- 108 files changed, 1594 insertions(+), 282 deletions(-) diff --git a/cli/rustfs-gui/src/utils/helper.rs b/cli/rustfs-gui/src/utils/helper.rs index 5a55b8ae..28bc14b7 100644 --- a/cli/rustfs-gui/src/utils/helper.rs +++ b/cli/rustfs-gui/src/utils/helper.rs @@ -11,7 +11,7 @@ use tokio::fs; use tokio::fs::File; use tokio::io::AsyncWriteExt; use tokio::net::TcpStream; -use tokio::sync::{mpsc, Mutex}; +use tokio::sync::{Mutex, mpsc}; #[derive(RustEmbed)] #[folder = "$CARGO_MANIFEST_DIR/embedded-rustfs/"] @@ -746,10 +746,10 @@ mod tests { assert_eq!(ServiceManager::extract_port("host:0"), Some(0)); assert_eq!(ServiceManager::extract_port("host:65535"), Some(65535)); assert_eq!(ServiceManager::extract_port("host:65536"), None); // Out of range - // IPv6-like address - extract_port takes the second part after split(':') - // For "::1:8080", split(':') gives ["", "", "1", "8080"], nth(1) gives "" + // IPv6-like address - extract_port takes the second part after split(':') + // For "::1:8080", split(':') gives ["", "", "1", "8080"], nth(1) gives "" assert_eq!(ServiceManager::extract_port("::1:8080"), None); // Second part is empty - // For "[::1]:8080", split(':') gives ["[", "", "1]", "8080"], nth(1) gives "" + // For "[::1]:8080", split(':') gives ["[", "", "1]", "8080"], nth(1) gives "" assert_eq!(ServiceManager::extract_port("[::1]:8080"), None); // Second part is empty } diff --git a/common/protos/src/generated/proto_gen/node_service.rs b/common/protos/src/generated/proto_gen/node_service.rs index 8a0c4b82..f7790549 100644 --- a/common/protos/src/generated/proto_gen/node_service.rs +++ b/common/protos/src/generated/proto_gen/node_service.rs @@ -1091,9 +1091,9 @@ pub mod node_service_client { F: tonic::service::Interceptor, T::ResponseBody: Default, T: tonic::codegen::Service< - http::Request, - Response = http::Response<>::ResponseBody>, - >, + http::Request, + Response = http::Response<>::ResponseBody>, + >, >>::Error: Into + std::marker::Send + std::marker::Sync, { diff --git a/common/protos/src/lib.rs b/common/protos/src/lib.rs index e1b86f2d..4d6acb4a 100644 --- a/common/protos/src/lib.rs +++ b/common/protos/src/lib.rs @@ -7,10 +7,10 @@ use common::globals::GLOBAL_Conn_Map; pub use generated::*; use proto_gen::node_service::node_service_client::NodeServiceClient; use tonic::{ + Request, Status, metadata::MetadataValue, service::interceptor::InterceptedService, transport::{Channel, Endpoint}, - Request, Status, }; // Default 100 MB diff --git a/crates/config/src/config.rs b/crates/config/src/config.rs index b9d1f31e..40dd8fc6 100644 --- a/crates/config/src/config.rs +++ b/crates/config/src/config.rs @@ -1,5 +1,5 @@ -use crate::event::config::NotifierConfig; use crate::ObservabilityConfig; +use crate::event::config::NotifierConfig; /// RustFs configuration pub struct RustFsConfig { diff --git a/crates/event-notifier/examples/simple.rs b/crates/event-notifier/examples/simple.rs index 27d422b0..eb2213db 100644 --- a/crates/event-notifier/examples/simple.rs +++ b/crates/event-notifier/examples/simple.rs @@ -1,5 +1,5 @@ -use rustfs_event_notifier::create_adapters; use rustfs_event_notifier::NotifierSystem; +use rustfs_event_notifier::create_adapters; use rustfs_event_notifier::{AdapterConfig, NotifierConfig, WebhookConfig}; use rustfs_event_notifier::{Bucket, Event, Identity, Metadata, Name, Object, Source}; use std::collections::HashMap; diff --git a/crates/event-notifier/examples/webhook.rs b/crates/event-notifier/examples/webhook.rs index 4cdf02c6..a91b8afd 100644 --- a/crates/event-notifier/examples/webhook.rs +++ b/crates/event-notifier/examples/webhook.rs @@ -1,4 +1,4 @@ -use axum::{extract::Json, http::StatusCode, routing::post, Router}; +use axum::{Router, extract::Json, http::StatusCode, routing::post}; use serde_json::Value; use std::time::{SystemTime, UNIX_EPOCH}; diff --git a/crates/event-notifier/src/event.rs b/crates/event-notifier/src/event.rs index 1bf100d7..16eabccc 100644 --- a/crates/event-notifier/src/event.rs +++ b/crates/event-notifier/src/event.rs @@ -1,7 +1,7 @@ use crate::Error; use serde::{Deserialize, Serialize}; use serde_with::{DeserializeFromStr, SerializeDisplay}; -use smallvec::{smallvec, SmallVec}; +use smallvec::{SmallVec, smallvec}; use std::borrow::Cow; use std::collections::HashMap; use std::time::{SystemTime, UNIX_EPOCH}; diff --git a/crates/event-notifier/src/global.rs b/crates/event-notifier/src/global.rs index 49716e74..c9995c38 100644 --- a/crates/event-notifier/src/global.rs +++ b/crates/event-notifier/src/global.rs @@ -1,5 +1,5 @@ -use crate::{create_adapters, Error, Event, NotifierConfig, NotifierSystem}; -use std::sync::{atomic, Arc}; +use crate::{Error, Event, NotifierConfig, NotifierSystem, create_adapters}; +use std::sync::{Arc, atomic}; use tokio::sync::{Mutex, OnceCell}; use tracing::instrument; diff --git a/crates/event-notifier/src/lib.rs b/crates/event-notifier/src/lib.rs index fe2e5e3d..e840aa7a 100644 --- a/crates/event-notifier/src/lib.rs +++ b/crates/event-notifier/src/lib.rs @@ -7,6 +7,7 @@ mod global; mod notifier; mod store; +pub use adapter::ChannelAdapter; pub use adapter::create_adapters; #[cfg(all(feature = "kafka", target_os = "linux"))] pub use adapter::kafka::KafkaAdapter; @@ -14,7 +15,6 @@ pub use adapter::kafka::KafkaAdapter; pub use adapter::mqtt::MqttAdapter; #[cfg(feature = "webhook")] pub use adapter::webhook::WebhookAdapter; -pub use adapter::ChannelAdapter; pub use bus::event_bus; #[cfg(all(feature = "kafka", target_os = "linux"))] pub use config::KafkaConfig; diff --git a/crates/event-notifier/src/notifier.rs b/crates/event-notifier/src/notifier.rs index 5ab17d37..21f6ac58 100644 --- a/crates/event-notifier/src/notifier.rs +++ b/crates/event-notifier/src/notifier.rs @@ -1,4 +1,4 @@ -use crate::{event_bus, ChannelAdapter, Error, Event, EventStore, NotifierConfig}; +use crate::{ChannelAdapter, Error, Event, EventStore, NotifierConfig, event_bus}; use std::sync::Arc; use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; diff --git a/crates/event-notifier/src/store.rs b/crates/event-notifier/src/store.rs index 24911615..debc8f83 100644 --- a/crates/event-notifier/src/store.rs +++ b/crates/event-notifier/src/store.rs @@ -2,7 +2,7 @@ use crate::Error; use crate::Log; use std::sync::Arc; use std::time::{SystemTime, UNIX_EPOCH}; -use tokio::fs::{create_dir_all, File, OpenOptions}; +use tokio::fs::{File, OpenOptions, create_dir_all}; use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader, BufWriter}; use tokio::sync::RwLock; use tracing::instrument; diff --git a/crates/filemeta/benches/xl_meta_bench.rs b/crates/filemeta/benches/xl_meta_bench.rs index fd835beb..20993ded 100644 --- a/crates/filemeta/benches/xl_meta_bench.rs +++ b/crates/filemeta/benches/xl_meta_bench.rs @@ -1,5 +1,5 @@ -use criterion::{black_box, criterion_group, criterion_main, Criterion}; -use rustfs_filemeta::{test_data::*, FileMeta}; +use criterion::{Criterion, black_box, criterion_group, criterion_main}; +use rustfs_filemeta::{FileMeta, test_data::*}; fn bench_create_real_xlmeta(c: &mut Criterion) { c.bench_function("create_real_xlmeta", |b| b.iter(|| black_box(create_real_xlmeta().unwrap()))); diff --git a/crates/filemeta/src/error.rs b/crates/filemeta/src/error.rs index 88fb2e13..142436e1 100644 --- a/crates/filemeta/src/error.rs +++ b/crates/filemeta/src/error.rs @@ -176,3 +176,378 @@ pub fn is_io_eof(e: &Error) -> bool { _ => false, } } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Error as IoError, ErrorKind}; + + #[test] + fn test_filemeta_error_from_io_error() { + let io_error = IoError::new(ErrorKind::PermissionDenied, "permission denied"); + let filemeta_error: Error = io_error.into(); + + match filemeta_error { + Error::Io(inner_io) => { + assert_eq!(inner_io.kind(), ErrorKind::PermissionDenied); + assert!(inner_io.to_string().contains("permission denied")); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_filemeta_error_other_function() { + let custom_error = "Custom filemeta error"; + let filemeta_error = Error::other(custom_error); + + match filemeta_error { + Error::Io(io_error) => { + assert!(io_error.to_string().contains(custom_error)); + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_filemeta_error_conversions() { + // Test various error conversions + let serde_decode_err = + rmp_serde::decode::Error::InvalidMarkerRead(std::io::Error::new(ErrorKind::InvalidData, "invalid")); + let filemeta_error: Error = serde_decode_err.into(); + assert!(matches!(filemeta_error, Error::RmpSerdeDecode(_))); + + // Test with string-based error that we can actually create + let encode_error_string = "test encode error"; + let filemeta_error = Error::RmpSerdeEncode(encode_error_string.to_string()); + assert!(matches!(filemeta_error, Error::RmpSerdeEncode(_))); + + let utf8_err = std::string::String::from_utf8(vec![0xFF]).unwrap_err(); + let filemeta_error: Error = utf8_err.into(); + assert!(matches!(filemeta_error, Error::FromUtf8(_))); + } + + #[test] + fn test_filemeta_error_clone() { + let test_cases = vec![ + Error::FileNotFound, + Error::FileVersionNotFound, + Error::VolumeNotFound, + Error::FileCorrupt, + Error::DoneForNow, + Error::MethodNotAllowed, + Error::Unexpected, + Error::Io(IoError::new(ErrorKind::NotFound, "test")), + Error::RmpSerdeDecode("test decode error".to_string()), + Error::RmpSerdeEncode("test encode error".to_string()), + Error::FromUtf8("test utf8 error".to_string()), + Error::RmpDecodeValueRead("test value read error".to_string()), + Error::RmpEncodeValueWrite("test value write error".to_string()), + Error::RmpDecodeNumValueRead("test num read error".to_string()), + Error::RmpDecodeMarkerRead("test marker read error".to_string()), + Error::TimeComponentRange("test time error".to_string()), + Error::UuidParse("test uuid error".to_string()), + ]; + + for original_error in test_cases { + let cloned_error = original_error.clone(); + assert_eq!(original_error, cloned_error); + } + } + + #[test] + fn test_filemeta_error_partial_eq() { + // Test equality for simple variants + assert_eq!(Error::FileNotFound, Error::FileNotFound); + assert_ne!(Error::FileNotFound, Error::FileVersionNotFound); + + // Test equality for Io variants + let io1 = Error::Io(IoError::new(ErrorKind::NotFound, "test")); + let io2 = Error::Io(IoError::new(ErrorKind::NotFound, "test")); + let io3 = Error::Io(IoError::new(ErrorKind::PermissionDenied, "test")); + assert_eq!(io1, io2); + assert_ne!(io1, io3); + + // Test equality for string variants + let decode1 = Error::RmpSerdeDecode("error message".to_string()); + let decode2 = Error::RmpSerdeDecode("error message".to_string()); + let decode3 = Error::RmpSerdeDecode("different message".to_string()); + assert_eq!(decode1, decode2); + assert_ne!(decode1, decode3); + } + + #[test] + fn test_filemeta_error_display() { + let test_cases = vec![ + (Error::FileNotFound, "File not found"), + (Error::FileVersionNotFound, "File version not found"), + (Error::VolumeNotFound, "Volume not found"), + (Error::FileCorrupt, "File corrupt"), + (Error::DoneForNow, "Done for now"), + (Error::MethodNotAllowed, "Method not allowed"), + (Error::Unexpected, "Unexpected error"), + (Error::RmpSerdeDecode("test".to_string()), "rmp serde decode error: test"), + (Error::RmpSerdeEncode("test".to_string()), "rmp serde encode error: test"), + (Error::FromUtf8("test".to_string()), "Invalid UTF-8: test"), + (Error::TimeComponentRange("test".to_string()), "time component range error: test"), + (Error::UuidParse("test".to_string()), "uuid parse error: test"), + ]; + + for (error, expected_message) in test_cases { + assert_eq!(error.to_string(), expected_message); + } + } + + #[test] + fn test_rmp_conversions() { + // Test rmp value read error (this one works since it has the same signature) + let value_read_err = rmp::decode::ValueReadError::InvalidMarkerRead(std::io::Error::new(ErrorKind::InvalidData, "test")); + let filemeta_error: Error = value_read_err.into(); + assert!(matches!(filemeta_error, Error::RmpDecodeValueRead(_))); + + // Test rmp num value read error + let num_value_err = + rmp::decode::NumValueReadError::InvalidMarkerRead(std::io::Error::new(ErrorKind::InvalidData, "test")); + let filemeta_error: Error = num_value_err.into(); + assert!(matches!(filemeta_error, Error::RmpDecodeNumValueRead(_))); + } + + #[test] + fn test_time_and_uuid_conversions() { + // Test time component range error + use time::{Date, Month}; + let time_result = Date::from_calendar_date(2023, Month::January, 32); // Invalid day + assert!(time_result.is_err()); + let time_error = time_result.unwrap_err(); + let filemeta_error: Error = time_error.into(); + assert!(matches!(filemeta_error, Error::TimeComponentRange(_))); + + // Test UUID parse error + let uuid_result = uuid::Uuid::parse_str("invalid-uuid"); + assert!(uuid_result.is_err()); + let uuid_error = uuid_result.unwrap_err(); + let filemeta_error: Error = uuid_error.into(); + assert!(matches!(filemeta_error, Error::UuidParse(_))); + } + + #[test] + fn test_marker_read_error_conversion() { + // Test rmp marker read error conversion + let marker_err = rmp::decode::MarkerReadError(std::io::Error::new(ErrorKind::InvalidData, "marker test")); + let filemeta_error: Error = marker_err.into(); + assert!(matches!(filemeta_error, Error::RmpDecodeMarkerRead(_))); + assert!(filemeta_error.to_string().contains("marker")); + } + + #[test] + fn test_is_io_eof_function() { + // Test is_io_eof helper function + let eof_error = Error::Io(IoError::new(ErrorKind::UnexpectedEof, "eof")); + assert!(is_io_eof(&eof_error)); + + let not_eof_error = Error::Io(IoError::new(ErrorKind::NotFound, "not found")); + assert!(!is_io_eof(¬_eof_error)); + + let non_io_error = Error::FileNotFound; + assert!(!is_io_eof(&non_io_error)); + } + + #[test] + fn test_filemeta_error_to_io_error_conversion() { + // Test conversion from FileMeta Error to io::Error through other function + let original_io_error = IoError::new(ErrorKind::InvalidData, "test data"); + let filemeta_error = Error::other(original_io_error); + + match filemeta_error { + Error::Io(io_err) => { + assert_eq!(io_err.kind(), ErrorKind::Other); + assert!(io_err.to_string().contains("test data")); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_filemeta_error_roundtrip_conversion() { + // Test roundtrip conversion: io::Error -> FileMeta Error -> io::Error + let original_io_error = IoError::new(ErrorKind::PermissionDenied, "permission test"); + + // Convert to FileMeta Error + let filemeta_error: Error = original_io_error.into(); + + // Extract the io::Error back + match filemeta_error { + Error::Io(extracted_io_error) => { + assert_eq!(extracted_io_error.kind(), ErrorKind::PermissionDenied); + assert!(extracted_io_error.to_string().contains("permission test")); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_filemeta_error_io_error_kinds_preservation() { + let io_error_kinds = vec![ + ErrorKind::NotFound, + ErrorKind::PermissionDenied, + ErrorKind::ConnectionRefused, + ErrorKind::ConnectionReset, + ErrorKind::ConnectionAborted, + ErrorKind::NotConnected, + ErrorKind::AddrInUse, + ErrorKind::AddrNotAvailable, + ErrorKind::BrokenPipe, + ErrorKind::AlreadyExists, + ErrorKind::WouldBlock, + ErrorKind::InvalidInput, + ErrorKind::InvalidData, + ErrorKind::TimedOut, + ErrorKind::WriteZero, + ErrorKind::Interrupted, + ErrorKind::UnexpectedEof, + ErrorKind::Other, + ]; + + for kind in io_error_kinds { + let io_error = IoError::new(kind, format!("test error for {:?}", kind)); + let filemeta_error: Error = io_error.into(); + + match filemeta_error { + Error::Io(extracted_io_error) => { + assert_eq!(extracted_io_error.kind(), kind); + assert!(extracted_io_error.to_string().contains("test error")); + } + _ => panic!("Expected Io variant for kind {:?}", kind), + } + } + } + + #[test] + fn test_filemeta_error_downcast_chain() { + // Test error downcast chain functionality + let original_io_error = IoError::new(ErrorKind::InvalidData, "original error"); + let filemeta_error = Error::other(original_io_error); + + // The error should be wrapped as an Io variant + if let Error::Io(io_err) = filemeta_error { + // The wrapped error should be Other kind (from std::io::Error::other) + assert_eq!(io_err.kind(), ErrorKind::Other); + // But the message should still contain the original error information + assert!(io_err.to_string().contains("original error")); + } else { + panic!("Expected Io variant"); + } + } + + #[test] + fn test_filemeta_error_maintains_error_information() { + let test_cases = vec![ + (ErrorKind::NotFound, "file not found"), + (ErrorKind::PermissionDenied, "access denied"), + (ErrorKind::InvalidData, "corrupt data"), + (ErrorKind::TimedOut, "operation timed out"), + ]; + + for (kind, message) in test_cases { + let io_error = IoError::new(kind, message); + let error_message = io_error.to_string(); + let filemeta_error: Error = io_error.into(); + + match filemeta_error { + Error::Io(extracted_io_error) => { + assert_eq!(extracted_io_error.kind(), kind); + assert_eq!(extracted_io_error.to_string(), error_message); + } + _ => panic!("Expected Io variant"), + } + } + } + + #[test] + fn test_filemeta_error_complex_conversion_chain() { + // Test conversion from string error types that we can actually create + + // Test with UUID error conversion + let uuid_result = uuid::Uuid::parse_str("invalid-uuid-format"); + assert!(uuid_result.is_err()); + let uuid_error = uuid_result.unwrap_err(); + let filemeta_error: Error = uuid_error.into(); + + match filemeta_error { + Error::UuidParse(message) => { + assert!(message.contains("invalid")); + } + _ => panic!("Expected UuidParse variant"), + } + + // Test with time error conversion + use time::{Date, Month}; + let time_result = Date::from_calendar_date(2023, Month::January, 32); // Invalid day + assert!(time_result.is_err()); + let time_error = time_result.unwrap_err(); + let filemeta_error2: Error = time_error.into(); + + match filemeta_error2 { + Error::TimeComponentRange(message) => { + assert!(message.contains("range")); + } + _ => panic!("Expected TimeComponentRange variant"), + } + + // Test with UTF8 error conversion + let utf8_result = std::string::String::from_utf8(vec![0xFF]); + assert!(utf8_result.is_err()); + let utf8_error = utf8_result.unwrap_err(); + let filemeta_error3: Error = utf8_error.into(); + + match filemeta_error3 { + Error::FromUtf8(message) => { + assert!(message.contains("utf")); + } + _ => panic!("Expected FromUtf8 variant"), + } + } + + #[test] + fn test_filemeta_error_equality_with_io_errors() { + // Test equality comparison for Io variants + let io_error1 = IoError::new(ErrorKind::NotFound, "test message"); + let io_error2 = IoError::new(ErrorKind::NotFound, "test message"); + let io_error3 = IoError::new(ErrorKind::PermissionDenied, "test message"); + let io_error4 = IoError::new(ErrorKind::NotFound, "different message"); + + let filemeta_error1 = Error::Io(io_error1); + let filemeta_error2 = Error::Io(io_error2); + let filemeta_error3 = Error::Io(io_error3); + let filemeta_error4 = Error::Io(io_error4); + + // Same kind and message should be equal + assert_eq!(filemeta_error1, filemeta_error2); + + // Different kinds should not be equal + assert_ne!(filemeta_error1, filemeta_error3); + + // Different messages should not be equal + assert_ne!(filemeta_error1, filemeta_error4); + } + + #[test] + fn test_filemeta_error_clone_io_variants() { + let io_error = IoError::new(ErrorKind::ConnectionReset, "connection lost"); + let original_error = Error::Io(io_error); + let cloned_error = original_error.clone(); + + // Cloned error should be equal to original + assert_eq!(original_error, cloned_error); + + // Both should maintain the same properties + match (original_error, cloned_error) { + (Error::Io(orig_io), Error::Io(cloned_io)) => { + assert_eq!(orig_io.kind(), cloned_io.kind()); + assert_eq!(orig_io.to_string(), cloned_io.to_string()); + } + _ => panic!("Both should be Io variants"), + } + } +} diff --git a/crates/filemeta/src/filemeta_inline.rs b/crates/filemeta/src/filemeta_inline.rs index 69d6a99a..47fb9233 100644 --- a/crates/filemeta/src/filemeta_inline.rs +++ b/crates/filemeta/src/filemeta_inline.rs @@ -27,11 +27,7 @@ impl InlineData { } pub fn after_version(&self) -> &[u8] { - if self.0.is_empty() { - &self.0 - } else { - &self.0[1..] - } + if self.0.is_empty() { &self.0 } else { &self.0[1..] } } pub fn find(&self, key: &str) -> Result>> { diff --git a/crates/filemeta/src/metacache.rs b/crates/filemeta/src/metacache.rs index 84330938..88b7ad0c 100644 --- a/crates/filemeta/src/metacache.rs +++ b/crates/filemeta/src/metacache.rs @@ -1,5 +1,5 @@ use crate::error::{Error, Result}; -use crate::{merge_file_meta_versions, FileInfo, FileInfoVersions, FileMeta, FileMetaShallowVersion, VersionType}; +use crate::{FileInfo, FileInfoVersions, FileMeta, FileMetaShallowVersion, VersionType, merge_file_meta_versions}; use rmp::Marker; use serde::{Deserialize, Serialize}; use std::cmp::Ordering; @@ -10,8 +10,8 @@ use std::{ pin::Pin, ptr, sync::{ - atomic::{AtomicPtr, AtomicU64, Ordering as AtomicOrdering}, Arc, + atomic::{AtomicPtr, AtomicU64, Ordering as AtomicOrdering}, }, time::{Duration, SystemTime, UNIX_EPOCH}, }; diff --git a/crates/obs/examples/server.rs b/crates/obs/examples/server.rs index f010581d..ed2f74c1 100644 --- a/crates/obs/examples/server.rs +++ b/crates/obs/examples/server.rs @@ -1,5 +1,5 @@ use opentelemetry::global; -use rustfs_obs::{get_logger, init_obs, log_info, BaseLogEntry, ServerLogEntry, SystemObserver}; +use rustfs_obs::{BaseLogEntry, ServerLogEntry, SystemObserver, get_logger, init_obs, log_info}; use std::collections::HashMap; use std::time::{Duration, SystemTime}; use tracing::{error, info, instrument}; diff --git a/crates/obs/src/global.rs b/crates/obs/src/global.rs index 7d15290a..bfed7594 100644 --- a/crates/obs/src/global.rs +++ b/crates/obs/src/global.rs @@ -1,6 +1,6 @@ use crate::logger::InitLogStatus; -use crate::telemetry::{init_telemetry, OtelGuard}; -use crate::{get_global_logger, init_global_logger, AppConfig, Logger}; +use crate::telemetry::{OtelGuard, init_telemetry}; +use crate::{AppConfig, Logger, get_global_logger, init_global_logger}; use std::sync::{Arc, Mutex}; use tokio::sync::{OnceCell, SetError}; use tracing::{error, info}; diff --git a/crates/obs/src/logger.rs b/crates/obs/src/logger.rs index 2ba498b7..9a29f67c 100644 --- a/crates/obs/src/logger.rs +++ b/crates/obs/src/logger.rs @@ -1,6 +1,6 @@ use crate::sinks::Sink; use crate::{ - sinks, AppConfig, AuditLogEntry, BaseLogEntry, ConsoleLogEntry, GlobalError, OtelConfig, ServerLogEntry, UnifiedLogEntry, + AppConfig, AuditLogEntry, BaseLogEntry, ConsoleLogEntry, GlobalError, OtelConfig, ServerLogEntry, UnifiedLogEntry, sinks, }; use rustfs_config::{APP_NAME, ENVIRONMENT, SERVICE_VERSION}; use std::sync::Arc; diff --git a/crates/obs/src/sinks/webhook.rs b/crates/obs/src/sinks/webhook.rs index 77a874d9..82d4df41 100644 --- a/crates/obs/src/sinks/webhook.rs +++ b/crates/obs/src/sinks/webhook.rs @@ -1,5 +1,5 @@ -use crate::sinks::Sink; use crate::UnifiedLogEntry; +use crate::sinks::Sink; use async_trait::async_trait; /// Webhook Sink Implementation diff --git a/crates/obs/src/system/collector.rs b/crates/obs/src/system/collector.rs index ea9bae3b..0d1cac5a 100644 --- a/crates/obs/src/system/collector.rs +++ b/crates/obs/src/system/collector.rs @@ -1,11 +1,11 @@ +use crate::GlobalError; use crate::system::attributes::ProcessAttributes; use crate::system::gpu::GpuCollector; -use crate::system::metrics::{Metrics, DIRECTION, INTERFACE, STATUS}; -use crate::GlobalError; +use crate::system::metrics::{DIRECTION, INTERFACE, Metrics, STATUS}; use opentelemetry::KeyValue; use std::time::SystemTime; use sysinfo::{Networks, Pid, ProcessStatus, System}; -use tokio::time::{sleep, Duration}; +use tokio::time::{Duration, sleep}; /// Collector is responsible for collecting system metrics and attributes. /// It uses the sysinfo crate to gather information about the system and processes. diff --git a/crates/obs/src/system/gpu.rs b/crates/obs/src/system/gpu.rs index ce47f2c5..735af335 100644 --- a/crates/obs/src/system/gpu.rs +++ b/crates/obs/src/system/gpu.rs @@ -1,14 +1,14 @@ #[cfg(feature = "gpu")] +use crate::GlobalError; +#[cfg(feature = "gpu")] use crate::system::attributes::ProcessAttributes; #[cfg(feature = "gpu")] use crate::system::metrics::Metrics; #[cfg(feature = "gpu")] -use crate::GlobalError; +use nvml_wrapper::Nvml; #[cfg(feature = "gpu")] use nvml_wrapper::enums::device::UsedGpuMemory; #[cfg(feature = "gpu")] -use nvml_wrapper::Nvml; -#[cfg(feature = "gpu")] use sysinfo::Pid; #[cfg(feature = "gpu")] use tracing::warn; diff --git a/crates/obs/src/worker.rs b/crates/obs/src/worker.rs index cfe2f26c..7dec8a11 100644 --- a/crates/obs/src/worker.rs +++ b/crates/obs/src/worker.rs @@ -1,4 +1,4 @@ -use crate::{sinks::Sink, UnifiedLogEntry}; +use crate::{UnifiedLogEntry, sinks::Sink}; use std::sync::Arc; use tokio::sync::mpsc::Receiver; diff --git a/crates/rio/src/bitrot.rs b/crates/rio/src/bitrot.rs index f9e2ee21..370e7a96 100644 --- a/crates/rio/src/bitrot.rs +++ b/crates/rio/src/bitrot.rs @@ -1,6 +1,6 @@ use crate::{Reader, Writer}; use pin_project_lite::pin_project; -use rustfs_utils::{read_full, write_all, HashAlgorithm}; +use rustfs_utils::{HashAlgorithm, read_full, write_all}; use tokio::io::{AsyncRead, AsyncReadExt}; pin_project! { diff --git a/crates/rio/src/compress_reader.rs b/crates/rio/src/compress_reader.rs index 396a3763..2986ca90 100644 --- a/crates/rio/src/compress_reader.rs +++ b/crates/rio/src/compress_reader.rs @@ -1,4 +1,4 @@ -use crate::compress::{compress_block, decompress_block, CompressionAlgorithm}; +use crate::compress::{CompressionAlgorithm, compress_block, decompress_block}; use crate::{EtagResolvable, HashReaderDetector}; use crate::{HashReaderMut, Reader}; use pin_project_lite::pin_project; diff --git a/crates/rio/src/hash_reader.rs b/crates/rio/src/hash_reader.rs index ff1ad9bd..a5e11f33 100644 --- a/crates/rio/src/hash_reader.rs +++ b/crates/rio/src/hash_reader.rs @@ -284,7 +284,7 @@ impl HashReaderDetector for HashReader { #[cfg(test)] mod tests { use super::*; - use crate::{encrypt_reader, DecryptReader}; + use crate::{DecryptReader, encrypt_reader}; use std::io::Cursor; use tokio::io::{AsyncReadExt, BufReader}; diff --git a/crates/utils/src/certs.rs b/crates/utils/src/certs.rs index dbb10959..d9ef6380 100644 --- a/crates/utils/src/certs.rs +++ b/crates/utils/src/certs.rs @@ -396,10 +396,12 @@ mod tests { // Should fail because no certificates found let result = load_all_certs_from_directory(temp_dir.path().to_str().unwrap()); assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No valid certificate/private key pair found")); + assert!( + result + .unwrap_err() + .to_string() + .contains("No valid certificate/private key pair found") + ); } #[test] @@ -412,10 +414,12 @@ mod tests { let result = load_all_certs_from_directory(unicode_dir.to_str().unwrap()); assert!(result.is_err()); - assert!(result - .unwrap_err() - .to_string() - .contains("No valid certificate/private key pair found")); + assert!( + result + .unwrap_err() + .to_string() + .contains("No valid certificate/private key pair found") + ); } #[test] diff --git a/crates/utils/src/hash.rs b/crates/utils/src/hash.rs index 2234b414..4db5ee9e 100644 --- a/crates/utils/src/hash.rs +++ b/crates/utils/src/hash.rs @@ -114,7 +114,7 @@ mod tests { let data = b"test data"; let hash = HashAlgorithm::BLAKE2b512.hash_encode(data); assert_eq!(hash.len(), 32); // blake3 outputs 32 bytes by default - // BLAKE2b512 should be deterministic + // BLAKE2b512 should be deterministic let hash2 = HashAlgorithm::BLAKE2b512.hash_encode(data); assert_eq!(hash, hash2); } diff --git a/crates/utils/src/os/linux.rs b/crates/utils/src/os/linux.rs index b94ad7e0..06c14a86 100644 --- a/crates/utils/src/os/linux.rs +++ b/crates/utils/src/os/linux.rs @@ -1,5 +1,5 @@ use nix::sys::stat::{self, stat}; -use nix::sys::statfs::{self, statfs, FsType}; +use nix::sys::statfs::{self, FsType, statfs}; use std::fs::File; use std::io::{self, BufRead, Error, ErrorKind}; use std::path::Path; @@ -26,7 +26,7 @@ pub fn get_info(p: impl AsRef) -> std::io::Result { bfree, p.as_ref().display() ), - )) + )); } }; @@ -41,7 +41,7 @@ pub fn get_info(p: impl AsRef) -> std::io::Result { blocks, p.as_ref().display() ), - )) + )); } }; @@ -57,7 +57,7 @@ pub fn get_info(p: impl AsRef) -> std::io::Result { total, p.as_ref().display() ), - )) + )); } }; diff --git a/crates/utils/src/os/unix.rs b/crates/utils/src/os/unix.rs index 87e7faf8..ad8c07cb 100644 --- a/crates/utils/src/os/unix.rs +++ b/crates/utils/src/os/unix.rs @@ -20,7 +20,7 @@ pub fn get_info(p: impl AsRef) -> std::io::Result { bavail, bfree, p.as_ref().display() - ))) + ))); } }; @@ -32,7 +32,7 @@ pub fn get_info(p: impl AsRef) -> std::io::Result { reserved, blocks, p.as_ref().display() - ))) + ))); } }; @@ -45,7 +45,7 @@ pub fn get_info(p: impl AsRef) -> std::io::Result { free, total, p.as_ref().display() - ))) + ))); } }; diff --git a/crates/zip/src/lib.rs b/crates/zip/src/lib.rs index 76e244fb..554c65e4 100644 --- a/crates/zip/src/lib.rs +++ b/crates/zip/src/lib.rs @@ -608,8 +608,8 @@ mod tests { #[tokio::test] async fn test_decompress_with_invalid_format() { // Test decompression with invalid format - use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; let sample_content = b"Hello, compression world!"; let cursor = Cursor::new(sample_content); @@ -634,8 +634,8 @@ mod tests { #[tokio::test] async fn test_decompress_with_zip_format() { // Test decompression with Zip format (currently not supported) - use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; let sample_content = b"Hello, compression world!"; let cursor = Cursor::new(sample_content); @@ -660,8 +660,8 @@ mod tests { #[tokio::test] async fn test_decompress_error_propagation() { // Test error propagation during decompression process - use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; + use std::sync::atomic::{AtomicUsize, Ordering}; let sample_content = b"Hello, compression world!"; let cursor = Cursor::new(sample_content); @@ -690,8 +690,8 @@ mod tests { #[tokio::test] async fn test_decompress_callback_execution() { // Test callback function execution during decompression - use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::Arc; + use std::sync::atomic::{AtomicBool, Ordering}; let sample_content = b"Hello, compression world!"; let cursor = Cursor::new(sample_content); diff --git a/crypto/src/jwt/decode.rs b/crypto/src/jwt/decode.rs index ad76fa43..e221d1d4 100644 --- a/crypto/src/jwt/decode.rs +++ b/crypto/src/jwt/decode.rs @@ -1,7 +1,7 @@ use jsonwebtoken::{Algorithm, DecodingKey, TokenData, Validation}; -use crate::jwt::Claims; use crate::Error; +use crate::jwt::Claims; pub fn decode(token: &str, token_secret: &[u8]) -> Result, Error> { Ok(jsonwebtoken::decode( diff --git a/crypto/src/jwt/encode.rs b/crypto/src/jwt/encode.rs index 04e3a1c7..e2d31483 100644 --- a/crypto/src/jwt/encode.rs +++ b/crypto/src/jwt/encode.rs @@ -1,7 +1,7 @@ use jsonwebtoken::{Algorithm, EncodingKey, Header}; -use crate::jwt::Claims; use crate::Error; +use crate::jwt::Claims; pub fn encode(token_secret: &[u8], claims: &Claims) -> Result { Ok(jsonwebtoken::encode( diff --git a/e2e_test/src/reliant/lock.rs b/e2e_test/src/reliant/lock.rs index 5cd189e8..bed60b22 100644 --- a/e2e_test/src/reliant/lock.rs +++ b/e2e_test/src/reliant/lock.rs @@ -5,7 +5,7 @@ use std::{error::Error, sync::Arc, time::Duration}; use lock::{ drwmutex::Options, lock_args::LockArgs, - namespace_lock::{new_nslock, NsLockMap}, + namespace_lock::{NsLockMap, new_nslock}, new_lock_api, }; use protos::{node_service_time_out_client, proto_gen::node_service::GenerallyLockRequest}; @@ -60,16 +60,16 @@ async fn test_lock_unlock_ns_lock() -> Result<(), Box> { vec![locker], ) .await; - assert!(ns - .0 - .write() - .await - .get_lock(&Options { - timeout: Duration::from_secs(5), - retry_interval: Duration::from_secs(1), - }) - .await - .unwrap()); + assert!( + ns.0.write() + .await + .get_lock(&Options { + timeout: Duration::from_secs(5), + retry_interval: Duration::from_secs(1), + }) + .await + .unwrap() + ); ns.0.write().await.un_lock().await.unwrap(); Ok(()) diff --git a/ecstore/src/bucket/error.rs b/ecstore/src/bucket/error.rs index 6b1afd38..44b2df1d 100644 --- a/ecstore/src/bucket/error.rs +++ b/ecstore/src/bucket/error.rs @@ -95,7 +95,7 @@ impl BucketMetadataError { 0x06 => Some(BucketMetadataError::BucketQuotaConfigNotFound), 0x07 => Some(BucketMetadataError::BucketReplicationConfigNotFound), 0x08 => Some(BucketMetadataError::BucketRemoteTargetNotFound), - 0x09 => Some(BucketMetadataError::Io(std::io::Error::new(std::io::ErrorKind::Other, "Io error"))), + 0x09 => Some(BucketMetadataError::Io(std::io::Error::other("Io error"))), _ => None, } } diff --git a/ecstore/src/bucket/metadata_sys.rs b/ecstore/src/bucket/metadata_sys.rs index a0c382b4..bbd47895 100644 --- a/ecstore/src/bucket/metadata_sys.rs +++ b/ecstore/src/bucket/metadata_sys.rs @@ -3,15 +3,15 @@ use std::sync::OnceLock; use std::time::Duration; use std::{collections::HashMap, sync::Arc}; +use crate::StorageAPI; use crate::bucket::error::BucketMetadataError; -use crate::bucket::metadata::{load_bucket_metadata_parse, BUCKET_LIFECYCLE_CONFIG}; +use crate::bucket::metadata::{BUCKET_LIFECYCLE_CONFIG, load_bucket_metadata_parse}; use crate::bucket::utils::is_meta_bucketname; -use crate::error::{is_err_bucket_not_found, Error, Result}; -use crate::global::{is_dist_erasure, is_erasure, new_object_layer_fn, GLOBAL_Endpoints}; +use crate::error::{Error, Result, is_err_bucket_not_found}; +use crate::global::{GLOBAL_Endpoints, is_dist_erasure, is_erasure, new_object_layer_fn}; use crate::heal::heal_commands::HealOpts; use crate::store::ECStore; use crate::utils::xml::deserialize; -use crate::StorageAPI; use futures::future::join_all; use policy::policy::BucketPolicy; use s3s::dto::{ @@ -23,7 +23,7 @@ use tokio::sync::RwLock; use tokio::time::sleep; use tracing::{error, warn}; -use super::metadata::{load_bucket_metadata, BucketMetadata}; +use super::metadata::{BucketMetadata, load_bucket_metadata}; use super::quota::BucketQuota; use super::target::BucketTargets; @@ -363,7 +363,7 @@ impl BucketMetadataSys { Err(Error::other("errBucketMetadataNotInitialized")) } else { Err(err) - } + }; } }; diff --git a/ecstore/src/config/com.rs b/ecstore/src/config/com.rs index 8e77a299..b6a4ca50 100644 --- a/ecstore/src/config/com.rs +++ b/ecstore/src/config/com.rs @@ -1,4 +1,4 @@ -use super::{storageclass, Config, GLOBAL_StorageClass}; +use super::{Config, GLOBAL_StorageClass, storageclass}; use crate::disk::RUSTFS_META_BUCKET; use crate::error::{Error, Result}; use crate::store_api::{ObjectInfo, ObjectOptions, PutObjReader, StorageAPI}; @@ -123,7 +123,7 @@ pub async fn read_config_without_migrate(api: Arc) -> Result(api: Arc, data: &[u8]) -> Result String { - if let Some(v) = self.lookup(key) { - v - } else { - "".to_owned() - } + if let Some(v) = self.lookup(key) { v } else { "".to_owned() } } pub fn lookup(&self, key: &str) -> Option { for kv in self.0.iter() { diff --git a/ecstore/src/config/storageclass.rs b/ecstore/src/config/storageclass.rs index 73172c02..e0dc6252 100644 --- a/ecstore/src/config/storageclass.rs +++ b/ecstore/src/config/storageclass.rs @@ -174,13 +174,7 @@ pub fn lookup_config(kvs: &KVS, set_drive_count: usize) -> Result { parse_storage_class(&ssc_str)? } else { StorageClass { - parity: { - if set_drive_count == 1 { - 0 - } else { - DEFAULT_RRS_PARITY - } - }, + parity: { if set_drive_count == 1 { 0 } else { DEFAULT_RRS_PARITY } }, } } }; @@ -193,7 +187,10 @@ pub fn lookup_config(kvs: &KVS, set_drive_count: usize) -> Result { if let Ok(ev) = env::var(INLINE_BLOCK_ENV) { if let Ok(block) = ev.parse::() { if block.as_u64() as usize > DEFAULT_INLINE_BLOCK { - warn!("inline block value bigger than recommended max of 128KiB -> {}, performance may degrade for PUT please benchmark the changes",block); + warn!( + "inline block value bigger than recommended max of 128KiB -> {}, performance may degrade for PUT please benchmark the changes", + block + ); } block.as_u64() as usize } else { @@ -295,7 +292,10 @@ pub fn validate_parity_inner(ss_parity: usize, rrs_parity: usize, set_drive_coun } if ss_parity > 0 && rrs_parity > 0 && ss_parity < rrs_parity { - return Err(Error::other(format!("Standard storage class parity drives {} should be greater than or equal to Reduced redundancy storage class parity drives {}", ss_parity, rrs_parity))); + return Err(Error::other(format!( + "Standard storage class parity drives {} should be greater than or equal to Reduced redundancy storage class parity drives {}", + ss_parity, rrs_parity + ))); } Ok(()) } diff --git a/ecstore/src/disk/error.rs b/ecstore/src/disk/error.rs index dd3d2361..427a8608 100644 --- a/ecstore/src/disk/error.rs +++ b/ecstore/src/disk/error.rs @@ -721,4 +721,144 @@ mod tests { assert!(inner.source().is_none()); // std::io::Error typically doesn't have a source } } + + #[test] + fn test_io_error_roundtrip_conversion() { + // Test DiskError -> std::io::Error -> DiskError roundtrip + let original_disk_errors = vec![ + DiskError::FileNotFound, + DiskError::VolumeNotFound, + DiskError::DiskFull, + DiskError::FileCorrupt, + DiskError::MethodNotAllowed, + ]; + + for original_error in original_disk_errors { + // Convert to io::Error and back + let io_error: std::io::Error = original_error.clone().into(); + let recovered_error: DiskError = io_error.into(); + + // For non-Io variants, they become Io(ErrorKind::Other) and then back to the original + match &original_error { + DiskError::Io(_) => { + // Io errors should maintain their kind + assert!(matches!(recovered_error, DiskError::Io(_))); + } + _ => { + // Other errors become Io(Other) and then are recovered via downcast + // The recovered error should be functionally equivalent + assert_eq!(original_error.to_u32(), recovered_error.to_u32()); + } + } + } + } + + #[test] + fn test_io_error_with_disk_error_inside() { + // Test that io::Error containing DiskError can be properly converted back + let original_disk_error = DiskError::FileNotFound; + let io_with_disk_error = std::io::Error::other(original_disk_error.clone()); + + // Convert io::Error back to DiskError + let recovered_disk_error: DiskError = io_with_disk_error.into(); + assert_eq!(original_disk_error, recovered_disk_error); + } + + #[test] + fn test_io_error_different_kinds() { + use std::io::ErrorKind; + + let test_cases = vec![ + (ErrorKind::NotFound, "file not found"), + (ErrorKind::PermissionDenied, "permission denied"), + (ErrorKind::ConnectionRefused, "connection refused"), + (ErrorKind::TimedOut, "timed out"), + (ErrorKind::InvalidInput, "invalid input"), + ]; + + for (kind, message) in test_cases { + let io_error = std::io::Error::new(kind, message); + let disk_error: DiskError = io_error.into(); + + // Should become DiskError::Io with the same kind and message + match disk_error { + DiskError::Io(inner_io) => { + assert_eq!(inner_io.kind(), kind); + assert!(inner_io.to_string().contains(message)); + } + _ => panic!("Expected DiskError::Io variant"), + } + } + } + + #[test] + fn test_disk_error_to_io_error_preserves_information() { + let test_cases = vec![ + DiskError::FileNotFound, + DiskError::VolumeNotFound, + DiskError::DiskFull, + DiskError::FileCorrupt, + DiskError::MethodNotAllowed, + DiskError::ErasureReadQuorum, + DiskError::ErasureWriteQuorum, + ]; + + for disk_error in test_cases { + let io_error: std::io::Error = disk_error.clone().into(); + + // Error message should be preserved + assert!(io_error.to_string().contains(&disk_error.to_string())); + + // Should be able to downcast back to DiskError + let recovered_error = io_error.downcast::(); + assert!(recovered_error.is_ok()); + assert_eq!(recovered_error.unwrap(), disk_error); + } + } + + #[test] + fn test_io_error_downcast_chain() { + // Test nested error downcasting chain + let original_disk_error = DiskError::FileNotFound; + + // Create a chain: DiskError -> io::Error -> DiskError -> io::Error + let io_error1: std::io::Error = original_disk_error.clone().into(); + let disk_error2: DiskError = io_error1.into(); + let io_error2: std::io::Error = disk_error2.into(); + + // Final io::Error should still contain the original DiskError + let final_disk_error = io_error2.downcast::(); + assert!(final_disk_error.is_ok()); + assert_eq!(final_disk_error.unwrap(), original_disk_error); + } + + #[test] + fn test_io_error_with_original_io_content() { + // Test DiskError::Io variant preserves original io::Error + let original_io = std::io::Error::new(std::io::ErrorKind::BrokenPipe, "broken pipe"); + let disk_error = DiskError::Io(original_io); + + let converted_io: std::io::Error = disk_error.into(); + assert_eq!(converted_io.kind(), std::io::ErrorKind::BrokenPipe); + assert!(converted_io.to_string().contains("broken pipe")); + } + + #[test] + fn test_error_display_preservation() { + let disk_errors = vec![ + DiskError::MaxVersionsExceeded, + DiskError::CorruptedFormat, + DiskError::UnformattedDisk, + DiskError::DiskNotFound, + DiskError::FileAccessDenied, + ]; + + for disk_error in disk_errors { + let original_message = disk_error.to_string(); + let io_error: std::io::Error = disk_error.clone().into(); + + // The io::Error should contain the original error message + assert!(io_error.to_string().contains(&original_message)); + } + } } diff --git a/ecstore/src/erasure_coding/encode.rs b/ecstore/src/erasure_coding/encode.rs index 671a5777..e4fc3912 100644 --- a/ecstore/src/erasure_coding/encode.rs +++ b/ecstore/src/erasure_coding/encode.rs @@ -6,7 +6,7 @@ use rustfs_rio::Reader; use super::Erasure; use crate::disk::error::Error; use crate::disk::error_reduce::count_errs; -use crate::disk::error_reduce::{reduce_write_quorum_errs, OBJECT_OP_IGNORED_ERRS}; +use crate::disk::error_reduce::{OBJECT_OP_IGNORED_ERRS, reduce_write_quorum_errs}; use std::sync::Arc; use std::vec; use tokio::sync::mpsc; @@ -62,15 +62,12 @@ impl<'a> MultiWriter<'a> { } if let Some(write_err) = reduce_write_quorum_errs(&self.errs, OBJECT_OP_IGNORED_ERRS, self.write_quorum) { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!( - "Failed to write data: {} (offline-disks={}/{})", - write_err, - count_errs(&self.errs, &Error::DiskNotFound), - self.writers.len() - ), - )); + return Err(std::io::Error::other(format!( + "Failed to write data: {} (offline-disks={}/{})", + write_err, + count_errs(&self.errs, &Error::DiskNotFound), + self.writers.len() + ))); } Err(std::io::Error::other(format!( @@ -103,10 +100,7 @@ impl Erasure { total += n; let res = self.encode_data(&buf[..n])?; if let Err(err) = tx.send(res).await { - return Err(std::io::Error::new( - std::io::ErrorKind::Other, - format!("Failed to send encoded data : {}", err), - )); + return Err(std::io::Error::other(format!("Failed to send encoded data : {}", err))); } } Ok(_) => break, diff --git a/ecstore/src/erasure_coding/erasure.rs b/ecstore/src/erasure_coding/erasure.rs index 32f6e6d8..7c80045b 100644 --- a/ecstore/src/erasure_coding/erasure.rs +++ b/ecstore/src/erasure_coding/erasure.rs @@ -3,7 +3,6 @@ use reed_solomon_erasure::galois_8::ReedSolomon; // use rustfs_rio::Reader; use smallvec::SmallVec; use std::io; -use std::io::ErrorKind; use tracing::error; use tracing::warn; use uuid::Uuid; @@ -98,7 +97,7 @@ impl Erasure { if let Some(encoder) = self.encoder.as_ref() { encoder.encode(data_slices).map_err(|e| { error!("encode data error: {:?}", e); - io::Error::new(ErrorKind::Other, format!("encode data error {:?}", e)) + io::Error::other(format!("encode data error {:?}", e)) })?; } else { warn!("parity_shards > 0, but encoder is None"); @@ -129,7 +128,7 @@ impl Erasure { if let Some(encoder) = self.encoder.as_ref() { encoder.reconstruct(shards).map_err(|e| { error!("decode data error: {:?}", e); - io::Error::new(ErrorKind::Other, format!("decode data error {:?}", e)) + io::Error::other(format!("decode data error {:?}", e)) })?; } else { warn!("parity_shards > 0, but encoder is None"); diff --git a/ecstore/src/error.rs b/ecstore/src/error.rs index ca80059d..8765cdf6 100644 --- a/ecstore/src/error.rs +++ b/ecstore/src/error.rs @@ -732,7 +732,7 @@ mod tests { #[test] fn test_storage_error_to_u32() { // Test Io error uses 0x01 - let io_error = StorageError::Io(IoError::new(ErrorKind::Other, "test")); + let io_error = StorageError::Io(IoError::other("test")); assert_eq!(io_error.to_u32(), 0x01); // Test other errors have correct codes @@ -781,7 +781,7 @@ mod tests { #[test] fn test_storage_error_from_disk_error() { // Test conversion from DiskError - let disk_io = DiskError::Io(IoError::new(ErrorKind::Other, "disk io error")); + let disk_io = DiskError::Io(IoError::other("disk io error")); let storage_error: StorageError = disk_io.into(); assert!(matches!(storage_error, StorageError::Io(_))); @@ -877,4 +877,195 @@ mod tests { } } } + + #[test] + fn test_storage_error_io_roundtrip() { + // Test StorageError -> std::io::Error -> StorageError roundtrip conversion + let original_storage_errors = vec![ + StorageError::FileNotFound, + StorageError::VolumeNotFound, + StorageError::DiskFull, + StorageError::FileCorrupt, + StorageError::MethodNotAllowed, + StorageError::BucketExists("test-bucket".to_string()), + StorageError::ObjectNotFound("bucket".to_string(), "object".to_string()), + ]; + + for original_error in original_storage_errors { + // Convert to io::Error and back + let io_error: std::io::Error = original_error.clone().into(); + let recovered_error: StorageError = io_error.into(); + + // Check that conversion preserves the essential error information + match &original_error { + StorageError::Io(_) => { + // Io errors should maintain their inner structure + assert!(matches!(recovered_error, StorageError::Io(_))); + } + _ => { + // Other errors should be recoverable via downcast or match to equivalent type + assert_eq!(original_error.to_u32(), recovered_error.to_u32()); + } + } + } + } + + #[test] + fn test_io_error_with_storage_error_inside() { + // Test that io::Error containing StorageError can be properly converted back + let original_storage_error = StorageError::FileNotFound; + let io_with_storage_error = std::io::Error::other(original_storage_error.clone()); + + // Convert io::Error back to StorageError + let recovered_storage_error: StorageError = io_with_storage_error.into(); + assert_eq!(original_storage_error, recovered_storage_error); + } + + #[test] + fn test_io_error_with_disk_error_inside() { + // Test io::Error containing DiskError -> StorageError conversion + let original_disk_error = DiskError::FileNotFound; + let io_with_disk_error = std::io::Error::other(original_disk_error.clone()); + + // Convert io::Error to StorageError + let storage_error: StorageError = io_with_disk_error.into(); + assert_eq!(storage_error, StorageError::FileNotFound); + } + + #[test] + fn test_nested_error_conversion_chain() { + // Test complex conversion chain: DiskError -> StorageError -> io::Error -> StorageError + let original_disk_error = DiskError::DiskFull; + let storage_error1: StorageError = original_disk_error.into(); + let io_error: std::io::Error = storage_error1.into(); + let storage_error2: StorageError = io_error.into(); + + assert_eq!(storage_error2, StorageError::DiskFull); + } + + #[test] + fn test_storage_error_different_io_kinds() { + use std::io::ErrorKind; + + let test_cases = vec![ + (ErrorKind::NotFound, "not found"), + (ErrorKind::PermissionDenied, "permission denied"), + (ErrorKind::ConnectionRefused, "connection refused"), + (ErrorKind::TimedOut, "timed out"), + (ErrorKind::InvalidInput, "invalid input"), + (ErrorKind::BrokenPipe, "broken pipe"), + ]; + + for (kind, message) in test_cases { + let io_error = std::io::Error::new(kind, message); + let storage_error: StorageError = io_error.into(); + + // Should become StorageError::Io with the same kind and message + match storage_error { + StorageError::Io(inner_io) => { + assert_eq!(inner_io.kind(), kind); + assert!(inner_io.to_string().contains(message)); + } + _ => panic!("Expected StorageError::Io variant for kind: {:?}", kind), + } + } + } + + #[test] + fn test_storage_error_to_io_error_preserves_information() { + let test_cases = vec![ + StorageError::FileNotFound, + StorageError::VolumeNotFound, + StorageError::DiskFull, + StorageError::FileCorrupt, + StorageError::MethodNotAllowed, + StorageError::StorageFull, + StorageError::SlowDown, + StorageError::BucketExists("test-bucket".to_string()), + ]; + + for storage_error in test_cases { + let io_error: std::io::Error = storage_error.clone().into(); + + // Error message should be preserved + assert!(io_error.to_string().contains(&storage_error.to_string())); + + // Should be able to downcast back to StorageError + let recovered_error = io_error.downcast::(); + assert!(recovered_error.is_ok()); + assert_eq!(recovered_error.unwrap(), storage_error); + } + } + + #[test] + fn test_storage_error_io_variant_preservation() { + // Test StorageError::Io variant preserves original io::Error + let original_io = std::io::Error::new(std::io::ErrorKind::UnexpectedEof, "unexpected eof"); + let storage_error = StorageError::Io(original_io); + + let converted_io: std::io::Error = storage_error.into(); + assert_eq!(converted_io.kind(), std::io::ErrorKind::UnexpectedEof); + assert!(converted_io.to_string().contains("unexpected eof")); + } + + #[test] + fn test_from_filemeta_error_conversions() { + // Test conversions from rustfs_filemeta::Error + use rustfs_filemeta::Error as FilemetaError; + + let filemeta_errors = vec![ + (FilemetaError::FileNotFound, StorageError::FileNotFound), + (FilemetaError::FileVersionNotFound, StorageError::FileVersionNotFound), + (FilemetaError::FileCorrupt, StorageError::FileCorrupt), + (FilemetaError::MethodNotAllowed, StorageError::MethodNotAllowed), + (FilemetaError::VolumeNotFound, StorageError::VolumeNotFound), + (FilemetaError::DoneForNow, StorageError::DoneForNow), + (FilemetaError::Unexpected, StorageError::Unexpected), + ]; + + for (filemeta_error, expected_storage_error) in filemeta_errors { + let converted_storage_error: StorageError = filemeta_error.into(); + assert_eq!(converted_storage_error, expected_storage_error); + + // Test reverse conversion + let converted_back: rustfs_filemeta::Error = converted_storage_error.into(); + assert_eq!(converted_back, expected_storage_error.into()); + } + } + + #[test] + fn test_error_message_consistency() { + let storage_errors = vec![ + StorageError::BucketNotFound("test-bucket".to_string()), + StorageError::ObjectNotFound("bucket".to_string(), "object".to_string()), + StorageError::VersionNotFound("bucket".to_string(), "object".to_string(), "v1".to_string()), + StorageError::InvalidUploadID("bucket".to_string(), "object".to_string(), "upload123".to_string()), + ]; + + for storage_error in storage_errors { + let original_message = storage_error.to_string(); + let io_error: std::io::Error = storage_error.clone().into(); + + // The io::Error should contain the original error message or info + assert!(io_error.to_string().contains(&original_message)); + } + } + + #[test] + fn test_error_equality_after_conversion() { + let storage_errors = vec![ + StorageError::FileNotFound, + StorageError::VolumeNotFound, + StorageError::DiskFull, + StorageError::MethodNotAllowed, + ]; + + for original_error in storage_errors { + // Test that equality is preserved through conversion + let io_error: std::io::Error = original_error.clone().into(); + let recovered_error: StorageError = io_error.into(); + + assert_eq!(original_error, recovered_error); + } + } } diff --git a/ecstore/src/heal/background_heal_ops.rs b/ecstore/src/heal/background_heal_ops.rs index 71f52cf1..8f6d5642 100644 --- a/ecstore/src/heal/background_heal_ops.rs +++ b/ecstore/src/heal/background_heal_ops.rs @@ -4,8 +4,8 @@ use std::{cmp::Ordering, env, path::PathBuf, sync::Arc, time::Duration}; use tokio::{ spawn, sync::{ - mpsc::{self, Receiver, Sender}, RwLock, + mpsc::{self, Receiver, Sender}, }, time::interval, }; @@ -14,16 +14,16 @@ use uuid::Uuid; use super::{ heal_commands::HealOpts, - heal_ops::{new_bg_heal_sequence, HealSequence}, + heal_ops::{HealSequence, new_bg_heal_sequence}, }; use crate::error::{Error, Result}; use crate::global::GLOBAL_MRFState; use crate::heal::error::ERR_RETRY_HEALING; -use crate::heal::heal_commands::{HealScanMode, HEAL_ITEM_BUCKET}; -use crate::heal::heal_ops::{HealSource, BG_HEALING_UUID}; +use crate::heal::heal_commands::{HEAL_ITEM_BUCKET, HealScanMode}; +use crate::heal::heal_ops::{BG_HEALING_UUID, HealSource}; use crate::{ config::RUSTFS_CONFIG_PREFIX, - disk::{endpoint::Endpoint, error::DiskError, DiskAPI, DiskInfoOptions, BUCKET_META_PREFIX, RUSTFS_META_BUCKET}, + disk::{BUCKET_META_PREFIX, DiskAPI, DiskInfoOptions, RUSTFS_META_BUCKET, endpoint::Endpoint, error::DiskError}, global::{GLOBAL_BackgroundHealRoutine, GLOBAL_BackgroundHealState, GLOBAL_LOCAL_DISK_MAP}, heal::{ data_usage::{DATA_USAGE_CACHE_NAME, DATA_USAGE_ROOT}, @@ -34,7 +34,7 @@ use crate::{ new_object_layer_fn, store::get_disk_via_endpoint, store_api::{BucketInfo, BucketOptions, StorageAPI}, - utils::path::{path_join, SLASH_SEPARATOR}, + utils::path::{SLASH_SEPARATOR, path_join}, }; pub static DEFAULT_MONITOR_NEW_DISK_INTERVAL: Duration = Duration::from_secs(10); @@ -149,7 +149,7 @@ async fn heal_fresh_disk(endpoint: &Endpoint) -> Result<()> { return Err(Error::other(format!( "Unexpected error disk must be initialized by now after formatting: {}", endpoint - ))) + ))); } }; diff --git a/ecstore/src/heal/data_scanner.rs b/ecstore/src/heal/data_scanner.rs index 170d7132..0986c76f 100644 --- a/ecstore/src/heal/data_scanner.rs +++ b/ecstore/src/heal/data_scanner.rs @@ -6,17 +6,17 @@ use std::{ path::{Path, PathBuf}, pin::Pin, sync::{ - atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}, Arc, + atomic::{AtomicBool, AtomicU32, AtomicU64, Ordering}, }, time::{Duration, SystemTime}, }; use super::{ - data_scanner_metric::{globalScannerMetrics, ScannerMetric, ScannerMetrics}, - data_usage::{store_data_usage_in_backend, DATA_USAGE_BLOOM_NAME_PATH}, + data_scanner_metric::{ScannerMetric, ScannerMetrics, globalScannerMetrics}, + data_usage::{DATA_USAGE_BLOOM_NAME_PATH, store_data_usage_in_backend}, data_usage_cache::{DataUsageCache, DataUsageEntry, DataUsageHash}, - heal_commands::{HealScanMode, HEAL_DEEP_SCAN, HEAL_NORMAL_SCAN}, + heal_commands::{HEAL_DEEP_SCAN, HEAL_NORMAL_SCAN, HealScanMode}, }; use crate::{ bucket::{versioning::VersioningApi, versioning_sys::BucketVersioningSys}, @@ -24,7 +24,7 @@ use crate::{ heal::data_usage::DATA_USAGE_ROOT, }; use crate::{ - cache_value::metacache_set::{list_path_raw, ListPathRawOptions}, + cache_value::metacache_set::{ListPathRawOptions, list_path_raw}, config::{ com::{read_config, save_config}, heal::Config, @@ -33,22 +33,22 @@ use crate::{ global::{GLOBAL_BackgroundHealState, GLOBAL_IsErasure, GLOBAL_IsErasureSD}, heal::{ data_usage::BACKGROUND_HEAL_INFO_PATH, - data_usage_cache::{hash_path, DataUsageHashMap}, + data_usage_cache::{DataUsageHashMap, hash_path}, error::ERR_IGNORE_FILE_CONTRIB, heal_commands::{HEAL_ITEM_BUCKET, HEAL_ITEM_OBJECT}, - heal_ops::{HealSource, BG_HEALING_UUID}, + heal_ops::{BG_HEALING_UUID, HealSource}, }, new_object_layer_fn, peer::is_reserved_or_invalid_bucket, store::ECStore, - utils::path::{path_join, path_to_bucket_object, path_to_bucket_object_with_base_path, SLASH_SEPARATOR}, + utils::path::{SLASH_SEPARATOR, path_join, path_to_bucket_object, path_to_bucket_object_with_base_path}, }; +use crate::{disk::DiskAPI, store_api::ObjectInfo}; use crate::{ disk::error::DiskError, error::{Error, Result}, }; use crate::{disk::local::LocalDisk, heal::data_scanner_metric::current_path_updater}; -use crate::{disk::DiskAPI, store_api::ObjectInfo}; use chrono::{DateTime, Utc}; use lazy_static::lazy_static; use rand::Rng; @@ -58,9 +58,8 @@ use s3s::dto::{BucketLifecycleConfiguration, ExpirationStatus, LifecycleRule, Re use serde::{Deserialize, Serialize}; use tokio::{ sync::{ - broadcast, + RwLock, broadcast, mpsc::{self, Sender}, - RwLock, }, time::sleep, }; diff --git a/ecstore/src/heal/data_scanner_metric.rs b/ecstore/src/heal/data_scanner_metric.rs index d5bf1688..6d97106a 100644 --- a/ecstore/src/heal/data_scanner_metric.rs +++ b/ecstore/src/heal/data_scanner_metric.rs @@ -6,8 +6,8 @@ use std::{ collections::HashMap, pin::Pin, sync::{ - atomic::{AtomicU64, Ordering}, Arc, + atomic::{AtomicU64, Ordering}, }, time::{Duration, SystemTime}, }; diff --git a/ecstore/src/heal/data_usage_cache.rs b/ecstore/src/heal/data_usage_cache.rs index eb2ac9a9..a47c6fa8 100644 --- a/ecstore/src/heal/data_usage_cache.rs +++ b/ecstore/src/heal/data_usage_cache.rs @@ -18,7 +18,7 @@ use std::time::{Duration, SystemTime}; use tokio::sync::mpsc::Sender; use tokio::time::sleep; -use super::data_scanner::{SizeSummary, DATA_SCANNER_FORCE_COMPACT_AT_FOLDERS}; +use super::data_scanner::{DATA_SCANNER_FORCE_COMPACT_AT_FOLDERS, SizeSummary}; use super::data_usage::{BucketTargetUsageInfo, BucketUsageInfo, DataUsageInfo}; // DATA_USAGE_BUCKET_LEN must be length of ObjectsHistogramIntervals diff --git a/ecstore/src/heal/heal_commands.rs b/ecstore/src/heal/heal_commands.rs index 6e27bf45..73e311ad 100644 --- a/ecstore/src/heal/heal_commands.rs +++ b/ecstore/src/heal/heal_commands.rs @@ -6,7 +6,7 @@ use std::{ use crate::{ config::storageclass::{RRS, STANDARD}, - disk::{error::DiskError, DeleteOptions, DiskAPI, DiskStore, BUCKET_META_PREFIX, RUSTFS_META_BUCKET}, + disk::{BUCKET_META_PREFIX, DeleteOptions, DiskAPI, DiskStore, RUSTFS_META_BUCKET, error::DiskError}, global::GLOBAL_BackgroundHealState, heal::heal_ops::HEALING_TRACKER_FILENAME, new_object_layer_fn, diff --git a/ecstore/src/heal/heal_ops.rs b/ecstore/src/heal/heal_ops.rs index 8fc4ec42..3c195ff6 100644 --- a/ecstore/src/heal/heal_ops.rs +++ b/ecstore/src/heal/heal_ops.rs @@ -2,7 +2,7 @@ use super::{ background_heal_ops::HealTask, data_scanner::HEAL_DELETE_DANGLING, error::ERR_SKIP_FILE, - heal_commands::{HealOpts, HealScanMode, HealStopSuccess, HealingTracker, HEAL_ITEM_BUCKET_METADATA}, + heal_commands::{HEAL_ITEM_BUCKET_METADATA, HealOpts, HealScanMode, HealStopSuccess, HealingTracker}, }; use crate::error::{Error, Result}; use crate::store_api::StorageAPI; @@ -16,7 +16,7 @@ use crate::{ disk::endpoint::Endpoint, endpoints::Endpoints, global::GLOBAL_IsDistErasure, - heal::heal_commands::{HealStartSuccess, HEAL_UNKNOWN_SCAN}, + heal::heal_commands::{HEAL_UNKNOWN_SCAN, HealStartSuccess}, new_object_layer_fn, utils::path::has_prefix, }; @@ -41,10 +41,9 @@ use std::{ use tokio::{ select, spawn, sync::{ - broadcast, + RwLock, broadcast, mpsc::{self, Receiver as M_Receiver, Sender as M_Sender}, watch::{self, Receiver as W_Receiver, Sender as W_Sender}, - RwLock, }, time::{interval, sleep}, }; @@ -784,7 +783,10 @@ impl AllHealState { self.stop_heal_sequence(path_s).await?; } else if let Some(hs) = self.get_heal_sequence(path_s).await { if !hs.has_ended().await { - return Err(Error::other(format!("Heal is already running on the given path (use force-start option to stop and start afresh). The heal was started by IP {} at {:?}, token is {}", heal_sequence.client_address, heal_sequence.start_time, heal_sequence.client_token))); + return Err(Error::other(format!( + "Heal is already running on the given path (use force-start option to stop and start afresh). The heal was started by IP {} at {:?}, token is {}", + heal_sequence.client_address, heal_sequence.start_time, heal_sequence.client_token + ))); } } diff --git a/ecstore/src/heal/mrf.rs b/ecstore/src/heal/mrf.rs index 3b31c9db..27b4e759 100644 --- a/ecstore/src/heal/mrf.rs +++ b/ecstore/src/heal/mrf.rs @@ -8,8 +8,8 @@ use regex::Regex; use std::ops::Sub; use std::sync::atomic::{AtomicBool, Ordering}; use std::time::Duration; -use tokio::sync::mpsc::{Receiver, Sender}; use tokio::sync::RwLock; +use tokio::sync::mpsc::{Receiver, Sender}; use tokio::time::sleep; use tracing::error; use uuid::Uuid; diff --git a/ecstore/src/notification_sys.rs b/ecstore/src/notification_sys.rs index 09a74f63..ec71fc63 100644 --- a/ecstore/src/notification_sys.rs +++ b/ecstore/src/notification_sys.rs @@ -1,8 +1,8 @@ +use crate::StorageAPI; use crate::admin_server_info::get_commit_id; use crate::error::{Error, Result}; -use crate::global::{get_global_endpoints, GLOBAL_BOOT_TIME}; +use crate::global::{GLOBAL_BOOT_TIME, get_global_endpoints}; use crate::peer_rest_client::PeerRestClient; -use crate::StorageAPI; use crate::{endpoints::EndpointServerPools, new_object_layer_fn}; use futures::future::join_all; use lazy_static::lazy_static; diff --git a/ecstore/src/peer_rest_client.rs b/ecstore/src/peer_rest_client.rs index 40e50e12..20714277 100644 --- a/ecstore/src/peer_rest_client.rs +++ b/ecstore/src/peer_rest_client.rs @@ -7,10 +7,10 @@ use crate::{ utils::net::XHost, }; use madmin::{ + ServerProperties, health::{Cpus, MemInfo, OsInfo, Partitions, ProcInfo, SysConfig, SysErrors, SysService}, metrics::RealtimeMetrics, net::NetInfo, - ServerProperties, }; use protos::{ node_service_time_out_client, diff --git a/ecstore/src/set_disk.rs b/ecstore/src/set_disk.rs index 0dd97683..be11ccb3 100644 --- a/ecstore/src/set_disk.rs +++ b/ecstore/src/set_disk.rs @@ -3824,14 +3824,13 @@ impl ObjectIO for SetDisks { let sc_parity_drives = { if let Some(sc) = GLOBAL_StorageClass.get() { - let a = sc.get_parity_for_sc( + sc.get_parity_for_sc( user_defined .get(xhttp::AMZ_STORAGE_CLASS) .cloned() .unwrap_or_default() .as_str(), - ); - a + ) } else { None } @@ -4806,14 +4805,13 @@ impl StorageAPI for SetDisks { let sc_parity_drives = { if let Some(sc) = GLOBAL_StorageClass.get() { - let a = sc.get_parity_for_sc( + sc.get_parity_for_sc( user_defined .get(xhttp::AMZ_STORAGE_CLASS) .cloned() .unwrap_or_default() .as_str(), - ); - a + ) } else { None } diff --git a/ecstore/src/sets.rs b/ecstore/src/sets.rs index 93c6c4fd..3962c242 100644 --- a/ecstore/src/sets.rs +++ b/ecstore/src/sets.rs @@ -5,15 +5,16 @@ use crate::disk::error_reduce::count_errs; use crate::error::{Error, Result}; use crate::{ disk::{ + DiskAPI, DiskInfo, DiskOption, DiskStore, error::DiskError, format::{DistributionAlgoVersion, FormatV3}, - new_disk, DiskAPI, DiskInfo, DiskOption, DiskStore, + new_disk, }, endpoints::{Endpoints, PoolEndpoints}, error::StorageError, - global::{is_dist_erasure, GLOBAL_LOCAL_DISK_SET_DRIVES}, + global::{GLOBAL_LOCAL_DISK_SET_DRIVES, is_dist_erasure}, heal::heal_commands::{ - HealOpts, DRIVE_STATE_CORRUPT, DRIVE_STATE_MISSING, DRIVE_STATE_OFFLINE, DRIVE_STATE_OK, HEAL_ITEM_METADATA, + DRIVE_STATE_CORRUPT, DRIVE_STATE_MISSING, DRIVE_STATE_OFFLINE, DRIVE_STATE_OK, HEAL_ITEM_METADATA, HealOpts, }, set_disk::SetDisks, store_api::{ @@ -27,7 +28,7 @@ use crate::{ use common::globals::GLOBAL_Local_Node_Name; use futures::future::join_all; use http::HeaderMap; -use lock::{namespace_lock::NsLockMap, new_lock_api, LockApi}; +use lock::{LockApi, namespace_lock::NsLockMap, new_lock_api}; use madmin::heal_commands::{HealDriveInfo, HealResultItem}; use tokio::sync::RwLock; use uuid::Uuid; diff --git a/ecstore/src/store_api.rs b/ecstore/src/store_api.rs index a9bf332a..17983919 100644 --- a/ecstore/src/store_api.rs +++ b/ecstore/src/store_api.rs @@ -160,13 +160,7 @@ impl HTTPRangeSpec { Some(HTTPRangeSpec { is_suffix_length: false, start: start as usize, - end: { - if end < 0 { - None - } else { - Some(end as usize) - } - }, + end: { if end < 0 { None } else { Some(end as usize) } }, }) } @@ -827,7 +821,7 @@ pub trait StorageAPI: ObjectIO { opts: &HealOpts, ) -> Result<(HealResultItem, Option)>; async fn heal_objects(&self, bucket: &str, prefix: &str, opts: &HealOpts, hs: Arc, is_meta: bool) - -> Result<()>; + -> Result<()>; async fn get_pool_and_set(&self, id: &str) -> Result<(Option, Option, Option)>; async fn check_abandoned_parts(&self, bucket: &str, object: &str, opts: &HealOpts) -> Result<()>; } diff --git a/ecstore/src/store_init.rs b/ecstore/src/store_init.rs index 5419b7e1..6bdc677c 100644 --- a/ecstore/src/store_init.rs +++ b/ecstore/src/store_init.rs @@ -1,18 +1,19 @@ -use crate::config::{storageclass, KVS}; +use crate::config::{KVS, storageclass}; use crate::disk::error_reduce::{count_errs, reduce_write_quorum_errs}; use crate::disk::{self, DiskAPI}; use crate::error::{Error, Result}; use crate::{ disk::{ + DiskInfoOptions, DiskOption, DiskStore, FORMAT_CONFIG_FILE, RUSTFS_META_BUCKET, error::DiskError, format::{FormatErasureVersion, FormatMetaVersion, FormatV3}, - new_disk, DiskInfoOptions, DiskOption, DiskStore, FORMAT_CONFIG_FILE, RUSTFS_META_BUCKET, + new_disk, }, endpoints::Endpoints, heal::heal_commands::init_healing_tracker, }; use futures::future::join_all; -use std::collections::{hash_map::Entry, HashMap}; +use std::collections::{HashMap, hash_map::Entry}; use tracing::{debug, warn}; use uuid::Uuid; diff --git a/ecstore/src/utils/wildcard.rs b/ecstore/src/utils/wildcard.rs index 8e178d84..6462d86d 100644 --- a/ecstore/src/utils/wildcard.rs +++ b/ecstore/src/utils/wildcard.rs @@ -32,7 +32,7 @@ fn deep_match_rune(str_: &[u8], pattern: &[u8], simple: bool) -> bool { } else { deep_match_rune(str_, &pattern[1..], simple) || (!str_.is_empty() && deep_match_rune(&str_[1..], pattern, simple)) - } + }; } '?' => { if str_.is_empty() { diff --git a/iam/src/error.rs b/iam/src/error.rs index 758df317..f1b7f185 100644 --- a/iam/src/error.rs +++ b/iam/src/error.rs @@ -97,6 +97,61 @@ pub enum Error { Io(std::io::Error), } +impl PartialEq for Error { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Error::StringError(a), Error::StringError(b)) => a == b, + (Error::NoSuchUser(a), Error::NoSuchUser(b)) => a == b, + (Error::NoSuchAccount(a), Error::NoSuchAccount(b)) => a == b, + (Error::NoSuchServiceAccount(a), Error::NoSuchServiceAccount(b)) => a == b, + (Error::NoSuchTempAccount(a), Error::NoSuchTempAccount(b)) => a == b, + (Error::NoSuchGroup(a), Error::NoSuchGroup(b)) => a == b, + (Error::InvalidServiceType(a), Error::InvalidServiceType(b)) => a == b, + (Error::Io(a), Error::Io(b)) => a.kind() == b.kind() && a.to_string() == b.to_string(), + // For complex types like PolicyError, CryptoError, JWTError, compare string representations + (a, b) => std::mem::discriminant(a) == std::mem::discriminant(b) && a.to_string() == b.to_string(), + } + } +} + +impl Clone for Error { + fn clone(&self) -> Self { + match self { + Error::PolicyError(e) => Error::StringError(e.to_string()), // Convert to string since PolicyError may not be cloneable + Error::StringError(s) => Error::StringError(s.clone()), + Error::CryptoError(e) => Error::StringError(format!("crypto: {}", e)), // Convert to string + Error::NoSuchUser(s) => Error::NoSuchUser(s.clone()), + Error::NoSuchAccount(s) => Error::NoSuchAccount(s.clone()), + Error::NoSuchServiceAccount(s) => Error::NoSuchServiceAccount(s.clone()), + Error::NoSuchTempAccount(s) => Error::NoSuchTempAccount(s.clone()), + Error::NoSuchGroup(s) => Error::NoSuchGroup(s.clone()), + Error::NoSuchPolicy => Error::NoSuchPolicy, + Error::PolicyInUse => Error::PolicyInUse, + Error::GroupNotEmpty => Error::GroupNotEmpty, + Error::InvalidArgument => Error::InvalidArgument, + Error::IamSysNotInitialized => Error::IamSysNotInitialized, + Error::InvalidServiceType(s) => Error::InvalidServiceType(s.clone()), + Error::ErrCredMalformed => Error::ErrCredMalformed, + Error::CredNotInitialized => Error::CredNotInitialized, + Error::InvalidAccessKeyLength => Error::InvalidAccessKeyLength, + Error::InvalidSecretKeyLength => Error::InvalidSecretKeyLength, + Error::ContainsReservedChars => Error::ContainsReservedChars, + Error::GroupNameContainsReservedChars => Error::GroupNameContainsReservedChars, + Error::JWTError(e) => Error::StringError(format!("jwt err {}", e)), // Convert to string + Error::NoAccessKey => Error::NoAccessKey, + Error::InvalidToken => Error::InvalidToken, + Error::InvalidAccessKey => Error::InvalidAccessKey, + Error::IAMActionNotAllowed => Error::IAMActionNotAllowed, + Error::InvalidExpiration => Error::InvalidExpiration, + Error::NoSecretKeyWithAccessKey => Error::NoSecretKeyWithAccessKey, + Error::NoAccessKeyWithSecretKey => Error::NoAccessKeyWithSecretKey, + Error::PolicyTooLarge => Error::PolicyTooLarge, + Error::ConfigNotFound => Error::ConfigNotFound, + Error::Io(e) => Error::Io(std::io::Error::new(e.kind(), e.to_string())), + } + } +} + impl Error { pub fn other(error: E) -> Self where @@ -225,3 +280,142 @@ pub fn is_err_no_such_service_account(err: &Error) -> bool { // Error::msg(e.to_string()) // } // } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Error as IoError, ErrorKind}; + + #[test] + fn test_iam_error_to_io_error_conversion() { + let iam_errors = vec![ + Error::NoSuchUser("testuser".to_string()), + Error::NoSuchAccount("testaccount".to_string()), + Error::InvalidArgument, + Error::IAMActionNotAllowed, + Error::PolicyTooLarge, + Error::ConfigNotFound, + ]; + + for iam_error in iam_errors { + let io_error: std::io::Error = iam_error.clone().into(); + + // Check that conversion creates an io::Error + assert_eq!(io_error.kind(), ErrorKind::Other); + + // Check that the error message is preserved + assert!(io_error.to_string().contains(&iam_error.to_string())); + } + } + + #[test] + fn test_iam_error_from_storage_error() { + // Test conversion from StorageError + let storage_error = ecstore::error::StorageError::ConfigNotFound; + let iam_error: Error = storage_error.into(); + assert_eq!(iam_error, Error::ConfigNotFound); + + // Test reverse conversion + let back_to_storage: ecstore::error::StorageError = iam_error.into(); + assert_eq!(back_to_storage, ecstore::error::StorageError::ConfigNotFound); + } + + #[test] + fn test_iam_error_from_policy_error() { + use policy::error::Error as PolicyError; + + let policy_errors = vec![ + (PolicyError::NoSuchUser("user1".to_string()), Error::NoSuchUser("user1".to_string())), + (PolicyError::NoSuchPolicy, Error::NoSuchPolicy), + (PolicyError::InvalidArgument, Error::InvalidArgument), + (PolicyError::PolicyTooLarge, Error::PolicyTooLarge), + ]; + + for (policy_error, expected_iam_error) in policy_errors { + let converted_iam_error: Error = policy_error.into(); + assert_eq!(converted_iam_error, expected_iam_error); + } + } + + #[test] + fn test_iam_error_other_function() { + let custom_error = "Custom IAM error"; + let iam_error = Error::other(custom_error); + + match iam_error { + Error::Io(io_error) => { + assert!(io_error.to_string().contains(custom_error)); + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_iam_error_from_serde_json() { + // Test conversion from serde_json::Error + let invalid_json = r#"{"invalid": json}"#; + let json_error = serde_json::from_str::(invalid_json).unwrap_err(); + let iam_error: Error = json_error.into(); + + match iam_error { + Error::Io(io_error) => { + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_helper_functions() { + // Test helper functions for error type checking + assert!(is_err_config_not_found(&Error::ConfigNotFound)); + assert!(!is_err_config_not_found(&Error::NoSuchPolicy)); + + assert!(is_err_no_such_policy(&Error::NoSuchPolicy)); + assert!(!is_err_no_such_policy(&Error::ConfigNotFound)); + + assert!(is_err_no_such_user(&Error::NoSuchUser("test".to_string()))); + assert!(!is_err_no_such_user(&Error::NoSuchAccount("test".to_string()))); + + assert!(is_err_no_such_account(&Error::NoSuchAccount("test".to_string()))); + assert!(!is_err_no_such_account(&Error::NoSuchUser("test".to_string()))); + + assert!(is_err_no_such_temp_account(&Error::NoSuchTempAccount("test".to_string()))); + assert!(!is_err_no_such_temp_account(&Error::NoSuchAccount("test".to_string()))); + + assert!(is_err_no_such_group(&Error::NoSuchGroup("test".to_string()))); + assert!(!is_err_no_such_group(&Error::NoSuchUser("test".to_string()))); + + assert!(is_err_no_such_service_account(&Error::NoSuchServiceAccount("test".to_string()))); + assert!(!is_err_no_such_service_account(&Error::NoSuchAccount("test".to_string()))); + } + + #[test] + fn test_iam_error_io_preservation() { + // Test that Io variant preserves original io::Error + let original_io = IoError::new(ErrorKind::PermissionDenied, "access denied"); + let iam_error = Error::Io(original_io); + + let converted_io: std::io::Error = iam_error.into(); + // Note: Our clone implementation creates a new io::Error with the same kind and message + // but it becomes ErrorKind::Other when cloned + assert_eq!(converted_io.kind(), ErrorKind::Other); + assert!(converted_io.to_string().contains("access denied")); + } + + #[test] + fn test_error_display_format() { + let test_cases = vec![ + (Error::NoSuchUser("testuser".to_string()), "user 'testuser' does not exist"), + (Error::NoSuchAccount("testaccount".to_string()), "account 'testaccount' does not exist"), + (Error::InvalidArgument, "invalid arguments specified"), + (Error::IAMActionNotAllowed, "action not allowed"), + (Error::ConfigNotFound, "config not found"), + ]; + + for (error, expected_message) in test_cases { + assert_eq!(error.to_string(), expected_message); + } + } +} diff --git a/iam/src/manager.rs b/iam/src/manager.rs index b6bc8c62..e0fa80b1 100644 --- a/iam/src/manager.rs +++ b/iam/src/manager.rs @@ -1,22 +1,22 @@ -use crate::error::{is_err_config_not_found, Error, Result}; +use crate::error::{Error, Result, is_err_config_not_found}; use crate::{ cache::{Cache, CacheEntity}, - error::{is_err_no_such_group, is_err_no_such_policy, is_err_no_such_user, Error as IamError}, + error::{Error as IamError, is_err_no_such_group, is_err_no_such_policy, is_err_no_such_user}, get_global_action_cred, - store::{object::IAM_CONFIG_PREFIX, GroupInfo, MappedPolicy, Store, UserType}, + store::{GroupInfo, MappedPolicy, Store, UserType, object::IAM_CONFIG_PREFIX}, sys::{ - UpdateServiceAccountOpts, MAX_SVCSESSION_POLICY_SIZE, SESSION_POLICY_NAME, SESSION_POLICY_NAME_EXTRACTED, - STATUS_DISABLED, STATUS_ENABLED, + MAX_SVCSESSION_POLICY_SIZE, SESSION_POLICY_NAME, SESSION_POLICY_NAME_EXTRACTED, STATUS_DISABLED, STATUS_ENABLED, + UpdateServiceAccountOpts, }, }; use ecstore::utils::{crypto::base64_encode, path::path_join_buf}; use madmin::{AccountStatus, AddOrUpdateUserReq, GroupDesc}; use policy::{ arn::ARN, - auth::{self, get_claims_from_token_with_secret, is_secret_key_valid, jwt_sign, Credentials, UserIdentity}, + auth::{self, Credentials, UserIdentity, get_claims_from_token_with_secret, is_secret_key_valid, jwt_sign}, format::Format, policy::{ - default::DEFAULT_POLICIES, iam_policy_claim_name_sa, Policy, PolicyDoc, EMBEDDED_POLICY_TYPE, INHERITED_POLICY_TYPE, + EMBEDDED_POLICY_TYPE, INHERITED_POLICY_TYPE, Policy, PolicyDoc, default::DEFAULT_POLICIES, iam_policy_claim_name_sa, }, }; use serde::{Deserialize, Serialize}; @@ -24,8 +24,8 @@ use serde_json::Value; use std::{ collections::{HashMap, HashSet}, sync::{ - atomic::{AtomicBool, AtomicI64, Ordering}, Arc, + atomic::{AtomicBool, AtomicI64, Ordering}, }, time::Duration, }; @@ -633,7 +633,7 @@ where Cache::add_or_update(&self.cache.user_policies, name, p, OffsetDateTime::now_utc()); p.clone() } else { - let mp = match self.cache.sts_policies.load().get(name) { + match self.cache.sts_policies.load().get(name) { Some(p) => p.clone(), None => { let mut m = HashMap::new(); @@ -645,8 +645,7 @@ where MappedPolicy::default() } } - }; - mp + } } } }; diff --git a/iam/src/store.rs b/iam/src/store.rs index d29b2114..54bc25ed 100644 --- a/iam/src/store.rs +++ b/iam/src/store.rs @@ -3,7 +3,7 @@ pub mod object; use crate::cache::Cache; use crate::error::Result; use policy::{auth::UserIdentity, policy::PolicyDoc}; -use serde::{de::DeserializeOwned, Deserialize, Serialize}; +use serde::{Deserialize, Serialize, de::DeserializeOwned}; use std::collections::{HashMap, HashSet}; use time::OffsetDateTime; @@ -49,7 +49,7 @@ pub trait Store: Clone + Send + Sync + 'static { m: &mut HashMap, ) -> Result<()>; async fn load_mapped_policys(&self, user_type: UserType, is_group: bool, m: &mut HashMap) - -> Result<()>; + -> Result<()>; async fn load_all(&self, cache: &Cache) -> Result<()>; } diff --git a/iam/src/store/object.rs b/iam/src/store/object.rs index afb950e1..1792b6e9 100644 --- a/iam/src/store/object.rs +++ b/iam/src/store/object.rs @@ -1,5 +1,5 @@ use super::{GroupInfo, MappedPolicy, Store, UserType}; -use crate::error::{is_err_config_not_found, Error, Result}; +use crate::error::{Error, Result, is_err_config_not_found}; use crate::{ cache::{Cache, CacheEntity}, error::{is_err_no_such_policy, is_err_no_such_user}, @@ -8,18 +8,18 @@ use crate::{ }; use ecstore::{ config::{ - com::{delete_config, read_config, read_config_with_metadata, save_config}, RUSTFS_CONFIG_PREFIX, + com::{delete_config, read_config, read_config_with_metadata, save_config}, }, store::ECStore, store_api::{ObjectInfo, ObjectOptions}, store_list_objects::{ObjectInfoOrErr, WalkOptions}, - utils::path::{path_join_buf, SLASH_SEPARATOR}, + utils::path::{SLASH_SEPARATOR, path_join_buf}, }; use futures::future::join_all; use lazy_static::lazy_static; use policy::{auth::UserIdentity, policy::PolicyDoc}; -use serde::{de::DeserializeOwned, Serialize}; +use serde::{Serialize, de::DeserializeOwned}; use std::{collections::HashMap, sync::Arc}; use tokio::sync::broadcast::{self, Receiver as B_Receiver}; use tokio::sync::mpsc::{self, Sender}; diff --git a/iam/src/sys.rs b/iam/src/sys.rs index db32f96f..0d078de4 100644 --- a/iam/src/sys.rs +++ b/iam/src/sys.rs @@ -1,11 +1,11 @@ +use crate::error::Error as IamError; use crate::error::is_err_no_such_account; use crate::error::is_err_no_such_temp_account; -use crate::error::Error as IamError; use crate::error::{Error, Result}; use crate::get_global_action_cred; +use crate::manager::IamCache; use crate::manager::extract_jwt_claims; use crate::manager::get_default_policyes; -use crate::manager::IamCache; use crate::store::MappedPolicy; use crate::store::Store; use crate::store::UserType; @@ -14,22 +14,22 @@ use ecstore::utils::crypto::base64_encode; use madmin::AddOrUpdateUserReq; use madmin::GroupDesc; use policy::arn::ARN; +use policy::auth::ACCOUNT_ON; +use policy::auth::Credentials; +use policy::auth::UserIdentity; use policy::auth::contains_reserved_chars; use policy::auth::create_new_credentials_with_metadata; use policy::auth::generate_credentials; use policy::auth::is_access_key_valid; use policy::auth::is_secret_key_valid; -use policy::auth::Credentials; -use policy::auth::UserIdentity; -use policy::auth::ACCOUNT_ON; -use policy::policy::iam_policy_claim_name_sa; use policy::policy::Args; -use policy::policy::Policy; -use policy::policy::PolicyDoc; use policy::policy::EMBEDDED_POLICY_TYPE; use policy::policy::INHERITED_POLICY_TYPE; -use serde_json::json; +use policy::policy::Policy; +use policy::policy::PolicyDoc; +use policy::policy::iam_policy_claim_name_sa; use serde_json::Value; +use serde_json::json; use std::collections::HashMap; use std::sync::Arc; use time::OffsetDateTime; diff --git a/iam/src/utils.rs b/iam/src/utils.rs index ca08dacf..e53c0ab2 100644 --- a/iam/src/utils.rs +++ b/iam/src/utils.rs @@ -1,6 +1,6 @@ use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header}; use rand::{Rng, RngCore}; -use serde::{de::DeserializeOwned, Serialize}; +use serde::{Serialize, de::DeserializeOwned}; use std::io::{Error, Result}; pub fn gen_access_key(length: usize) -> Result { diff --git a/policy/src/auth/credentials.rs b/policy/src/auth/credentials.rs index 94eebce2..017c7561 100644 --- a/policy/src/auth/credentials.rs +++ b/policy/src/auth/credentials.rs @@ -1,14 +1,14 @@ use crate::error::Error as IamError; use crate::error::{Error, Result}; -use crate::policy::{iam_policy_claim_name_sa, Policy, Validator, INHERITED_POLICY_TYPE}; +use crate::policy::{INHERITED_POLICY_TYPE, Policy, Validator, iam_policy_claim_name_sa}; use crate::utils; use crate::utils::extract_claims; use serde::de::DeserializeOwned; use serde::{Deserialize, Serialize}; -use serde_json::{json, Value}; +use serde_json::{Value, json}; use std::collections::HashMap; -use time::macros::offset; use time::OffsetDateTime; +use time::macros::offset; const ACCESS_KEY_MIN_LEN: usize = 3; const ACCESS_KEY_MAX_LEN: usize = 20; diff --git a/policy/src/error.rs b/policy/src/error.rs index 46c8db09..afc3a9ce 100644 --- a/policy/src/error.rs +++ b/policy/src/error.rs @@ -160,3 +160,204 @@ pub fn is_err_no_such_group(err: &Error) -> bool { pub fn is_err_no_such_service_account(err: &Error) -> bool { matches!(err, Error::NoSuchServiceAccount(_)) } + +#[cfg(test)] +mod tests { + use super::*; + use std::io::{Error as IoError, ErrorKind}; + + #[test] + fn test_policy_error_from_io_error() { + let io_error = IoError::new(ErrorKind::PermissionDenied, "permission denied"); + let policy_error: Error = io_error.into(); + + match policy_error { + Error::Io(inner_io) => { + assert_eq!(inner_io.kind(), ErrorKind::PermissionDenied); + assert!(inner_io.to_string().contains("permission denied")); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_policy_error_other_function() { + let custom_error = "Custom policy error"; + let policy_error = Error::other(custom_error); + + match policy_error { + Error::Io(io_error) => { + assert!(io_error.to_string().contains(custom_error)); + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_policy_error_from_crypto_error() { + // Test conversion from crypto::Error - use an actual variant + let crypto_error = crypto::Error::ErrUnexpectedHeader; + let policy_error: Error = crypto_error.into(); + + match policy_error { + Error::CryptoError(_) => { + // Verify the conversion worked + assert!(policy_error.to_string().contains("crypto")); + } + _ => panic!("Expected CryptoError variant"), + } + } + + #[test] + fn test_policy_error_from_jwt_error() { + use jsonwebtoken::{Algorithm, DecodingKey, Validation, decode}; + use serde::{Deserialize, Serialize}; + + #[derive(Debug, Serialize, Deserialize)] + struct Claims { + sub: String, + exp: usize, + } + + // Create an invalid JWT to generate a JWT error + let invalid_token = "invalid.jwt.token"; + let key = DecodingKey::from_secret(b"secret"); + let validation = Validation::new(Algorithm::HS256); + + let jwt_result = decode::(invalid_token, &key, &validation); + assert!(jwt_result.is_err()); + + let jwt_error = jwt_result.unwrap_err(); + let policy_error: Error = jwt_error.into(); + + match policy_error { + Error::JWTError(_) => { + // Verify the conversion worked + assert!(policy_error.to_string().contains("jwt err")); + } + _ => panic!("Expected JWTError variant"), + } + } + + #[test] + fn test_policy_error_from_serde_json() { + // Test conversion from serde_json::Error + let invalid_json = r#"{"invalid": json}"#; + let json_error = serde_json::from_str::(invalid_json).unwrap_err(); + let policy_error: Error = json_error.into(); + + match policy_error { + Error::Io(io_error) => { + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_policy_error_from_time_component_range() { + use time::{Date, Month}; + + // Create an invalid date to generate a ComponentRange error + let time_result = Date::from_calendar_date(2023, Month::January, 32); // Invalid day + assert!(time_result.is_err()); + + let time_error = time_result.unwrap_err(); + let policy_error: Error = time_error.into(); + + match policy_error { + Error::Io(io_error) => { + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + #[allow(clippy::invalid_regex)] + fn test_policy_error_from_regex_error() { + use regex::Regex; + + // Create an invalid regex to generate a regex error (unclosed bracket) + let regex_result = Regex::new("["); + assert!(regex_result.is_err()); + + let regex_error = regex_result.unwrap_err(); + let policy_error: Error = regex_error.into(); + + match policy_error { + Error::Io(io_error) => { + assert_eq!(io_error.kind(), ErrorKind::Other); + } + _ => panic!("Expected Io variant"), + } + } + + #[test] + fn test_helper_functions() { + // Test helper functions for error type checking + assert!(is_err_no_such_policy(&Error::NoSuchPolicy)); + assert!(!is_err_no_such_policy(&Error::NoSuchUser("test".to_string()))); + + assert!(is_err_no_such_user(&Error::NoSuchUser("test".to_string()))); + assert!(!is_err_no_such_user(&Error::NoSuchAccount("test".to_string()))); + + assert!(is_err_no_such_account(&Error::NoSuchAccount("test".to_string()))); + assert!(!is_err_no_such_account(&Error::NoSuchUser("test".to_string()))); + + assert!(is_err_no_such_temp_account(&Error::NoSuchTempAccount("test".to_string()))); + assert!(!is_err_no_such_temp_account(&Error::NoSuchAccount("test".to_string()))); + + assert!(is_err_no_such_group(&Error::NoSuchGroup("test".to_string()))); + assert!(!is_err_no_such_group(&Error::NoSuchUser("test".to_string()))); + + assert!(is_err_no_such_service_account(&Error::NoSuchServiceAccount("test".to_string()))); + assert!(!is_err_no_such_service_account(&Error::NoSuchAccount("test".to_string()))); + } + + #[test] + fn test_error_display_format() { + let test_cases = vec![ + (Error::NoSuchUser("testuser".to_string()), "user 'testuser' does not exist"), + (Error::NoSuchAccount("testaccount".to_string()), "account 'testaccount' does not exist"), + ( + Error::NoSuchServiceAccount("service1".to_string()), + "service account 'service1' does not exist", + ), + (Error::NoSuchTempAccount("temp1".to_string()), "temp account 'temp1' does not exist"), + (Error::NoSuchGroup("group1".to_string()), "group 'group1' does not exist"), + (Error::NoSuchPolicy, "policy does not exist"), + (Error::PolicyInUse, "policy in use"), + (Error::GroupNotEmpty, "group not empty"), + (Error::InvalidArgument, "invalid arguments specified"), + (Error::IamSysNotInitialized, "not initialized"), + (Error::InvalidServiceType("invalid".to_string()), "invalid service type: invalid"), + (Error::ErrCredMalformed, "malformed credential"), + (Error::CredNotInitialized, "CredNotInitialized"), + (Error::InvalidAccessKeyLength, "invalid access key length"), + (Error::InvalidSecretKeyLength, "invalid secret key length"), + (Error::ContainsReservedChars, "access key contains reserved characters =,"), + (Error::GroupNameContainsReservedChars, "group name contains reserved characters =,"), + (Error::NoAccessKey, "no access key"), + (Error::InvalidToken, "invalid token"), + (Error::InvalidAccessKey, "invalid access_key"), + (Error::IAMActionNotAllowed, "action not allowed"), + (Error::InvalidExpiration, "invalid expiration"), + (Error::NoSecretKeyWithAccessKey, "no secret key with access key"), + (Error::NoAccessKeyWithSecretKey, "no access key with secret key"), + (Error::PolicyTooLarge, "policy too large"), + ]; + + for (error, expected_message) in test_cases { + assert_eq!(error.to_string(), expected_message); + } + } + + #[test] + fn test_string_error_variant() { + let custom_message = "Custom error message"; + let error = Error::StringError(custom_message.to_string()); + assert_eq!(error.to_string(), custom_message); + } +} diff --git a/policy/src/policy/action.rs b/policy/src/policy/action.rs index 157916fc..e5401224 100644 --- a/policy/src/policy/action.rs +++ b/policy/src/policy/action.rs @@ -3,7 +3,7 @@ use serde::{Deserialize, Serialize}; use std::{collections::HashSet, ops::Deref}; use strum::{EnumString, IntoStaticStr}; -use super::{utils::wildcard, Error as IamError, Validator}; +use super::{Error as IamError, Validator, utils::wildcard}; #[derive(Serialize, Deserialize, Clone, Default, Debug)] pub struct ActionSet(pub HashSet); diff --git a/policy/src/policy/function.rs b/policy/src/policy/function.rs index 54313fb7..64a78878 100644 --- a/policy/src/policy/function.rs +++ b/policy/src/policy/function.rs @@ -1,6 +1,6 @@ use crate::policy::function::condition::Condition; use serde::ser::SerializeMap; -use serde::{de, Deserialize, Serialize, Serializer}; +use serde::{Deserialize, Serialize, Serializer, de}; use std::collections::HashMap; use std::collections::HashSet; @@ -163,12 +163,12 @@ pub struct Value; #[cfg(test)] mod tests { + use crate::policy::Functions; use crate::policy::function::condition::Condition::*; use crate::policy::function::func::FuncKeyValue; use crate::policy::function::key::Key; use crate::policy::function::string::StringFunc; use crate::policy::function::string::StringFuncValue; - use crate::policy::Functions; use test_case::test_case; #[test_case( diff --git a/policy/src/policy/function/addr.rs b/policy/src/policy/function/addr.rs index b7577f46..6c5e529e 100644 --- a/policy/src/policy/function/addr.rs +++ b/policy/src/policy/function/addr.rs @@ -1,6 +1,6 @@ use super::func::InnerFunc; use ipnetwork::IpNetwork; -use serde::{de::Visitor, Deserialize, Serialize}; +use serde::{Deserialize, Serialize, de::Visitor}; use std::{borrow::Cow, collections::HashMap, net::IpAddr}; pub type AddrFunc = InnerFunc; diff --git a/policy/src/policy/function/bool_null.rs b/policy/src/policy/function/bool_null.rs index 5914c1da..0bf793ad 100644 --- a/policy/src/policy/function/bool_null.rs +++ b/policy/src/policy/function/bool_null.rs @@ -1,6 +1,6 @@ use super::func::InnerFunc; use serde::de::{Error, IgnoredAny, SeqAccess}; -use serde::{de, Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, de}; use std::{collections::HashMap, fmt}; pub type BoolFunc = InnerFunc; diff --git a/policy/src/policy/function/condition.rs b/policy/src/policy/function/condition.rs index 3de30660..1f5f545a 100644 --- a/policy/src/policy/function/condition.rs +++ b/policy/src/policy/function/condition.rs @@ -1,6 +1,6 @@ +use serde::Deserialize; use serde::de::{Error, MapAccess}; use serde::ser::SerializeMap; -use serde::Deserialize; use std::collections::HashMap; use time::OffsetDateTime; @@ -122,11 +122,7 @@ impl Condition { DateGreaterThanEquals(s) => s.evaluate(OffsetDateTime::ge, values), }; - if self.is_negate() { - !r - } else { - r - } + if self.is_negate() { !r } else { r } } #[inline] diff --git a/policy/src/policy/function/date.rs b/policy/src/policy/function/date.rs index 4f02fb89..78abeefe 100644 --- a/policy/src/policy/function/date.rs +++ b/policy/src/policy/function/date.rs @@ -1,7 +1,7 @@ use super::func::InnerFunc; -use serde::{de, Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, de}; use std::{collections::HashMap, fmt}; -use time::{format_description::well_known::Rfc3339, OffsetDateTime}; +use time::{OffsetDateTime, format_description::well_known::Rfc3339}; pub type DateFunc = InnerFunc; @@ -82,7 +82,7 @@ mod tests { key_name::S3KeyName::*, }; use test_case::test_case; - use time::{format_description::well_known::Rfc3339, OffsetDateTime}; + use time::{OffsetDateTime, format_description::well_known::Rfc3339}; fn new_func(name: KeyName, variable: Option, value: &str) -> DateFunc { DateFunc { diff --git a/policy/src/policy/function/func.rs b/policy/src/policy/function/func.rs index caa647b8..8017d558 100644 --- a/policy/src/policy/function/func.rs +++ b/policy/src/policy/function/func.rs @@ -1,8 +1,8 @@ use std::marker::PhantomData; use serde::{ - de::{self, Visitor}, Deserialize, Deserializer, Serialize, + de::{self, Visitor}, }; use super::key::Key; diff --git a/policy/src/policy/function/number.rs b/policy/src/policy/function/number.rs index 3f94980c..d0d6cb38 100644 --- a/policy/src/policy/function/number.rs +++ b/policy/src/policy/function/number.rs @@ -2,8 +2,8 @@ use std::collections::HashMap; use super::func::InnerFunc; use serde::{ - de::{Error, Visitor}, Deserialize, Deserializer, Serialize, + de::{Error, Visitor}, }; pub type NumberFunc = InnerFunc; diff --git a/policy/src/policy/function/string.rs b/policy/src/policy/function/string.rs index 7991c8ac..7fdc9ca3 100644 --- a/policy/src/policy/function/string.rs +++ b/policy/src/policy/function/string.rs @@ -7,7 +7,7 @@ use std::{borrow::Cow, collections::HashMap}; use crate::policy::function::func::FuncKeyValue; use crate::policy::utils::wildcard; -use serde::{de, ser::SerializeSeq, Deserialize, Deserializer, Serialize}; +use serde::{Deserialize, Deserializer, Serialize, de, ser::SerializeSeq}; use super::{func::InnerFunc, key_name::KeyName}; diff --git a/policy/src/policy/policy.rs b/policy/src/policy/policy.rs index a0126889..78ce19c4 100644 --- a/policy/src/policy/policy.rs +++ b/policy/src/policy/policy.rs @@ -1,4 +1,4 @@ -use super::{action::Action, statement::BPStatement, Effect, Error as IamError, Statement, ID}; +use super::{Effect, Error as IamError, ID, Statement, action::Action, statement::BPStatement}; use crate::error::{Error, Result}; use serde::{Deserialize, Serialize}; use serde_json::Value; @@ -252,9 +252,9 @@ pub mod default { use std::{collections::HashSet, sync::LazyLock}; use crate::policy::{ + ActionSet, DEFAULT_VERSION, Effect, Functions, ResourceSet, Statement, action::{Action, AdminAction, KmsAction, S3Action}, resource::Resource, - ActionSet, Effect, Functions, ResourceSet, Statement, DEFAULT_VERSION, }; use super::Policy; diff --git a/policy/src/policy/principal.rs b/policy/src/policy/principal.rs index a1316a74..5b642870 100644 --- a/policy/src/policy/principal.rs +++ b/policy/src/policy/principal.rs @@ -1,4 +1,4 @@ -use super::{utils::wildcard, Validator}; +use super::{Validator, utils::wildcard}; use crate::error::{Error, Result}; use serde::{Deserialize, Serialize}; use std::collections::HashSet; diff --git a/policy/src/policy/resource.rs b/policy/src/policy/resource.rs index b7797b0c..31b53d35 100644 --- a/policy/src/policy/resource.rs +++ b/policy/src/policy/resource.rs @@ -7,9 +7,9 @@ use std::{ }; use super::{ + Error as IamError, Validator, function::key_name::KeyName, utils::{path, wildcard}, - Error as IamError, Validator, }; #[derive(Serialize, Deserialize, Clone, Default, Debug)] diff --git a/policy/src/policy/statement.rs b/policy/src/policy/statement.rs index 72151c0a..37b617a1 100644 --- a/policy/src/policy/statement.rs +++ b/policy/src/policy/statement.rs @@ -1,6 +1,6 @@ use super::{ - action::Action, ActionSet, Args, BucketPolicyArgs, Effect, Error as IamError, Functions, Principal, ResourceSet, Validator, - ID, + ActionSet, Args, BucketPolicyArgs, Effect, Error as IamError, Functions, ID, Principal, ResourceSet, Validator, + action::Action, }; use crate::error::{Error, Result}; use serde::{Deserialize, Serialize}; diff --git a/policy/src/policy/utils/path.rs b/policy/src/policy/utils/path.rs index 9eb52bf0..bac9f7b6 100644 --- a/policy/src/policy/utils/path.rs +++ b/policy/src/policy/utils/path.rs @@ -85,11 +85,7 @@ pub fn clean(path: &str) -> String { } } - if out.w == 0 { - ".".into() - } else { - out.string() - } + if out.w == 0 { ".".into() } else { out.string() } } #[cfg(test)] diff --git a/policy/src/utils.rs b/policy/src/utils.rs index afb3b135..c32958b3 100644 --- a/policy/src/utils.rs +++ b/policy/src/utils.rs @@ -1,6 +1,6 @@ use jsonwebtoken::{Algorithm, DecodingKey, EncodingKey, Header}; use rand::{Rng, RngCore}; -use serde::{de::DeserializeOwned, Serialize}; +use serde::{Serialize, de::DeserializeOwned}; use std::io::{Error, Result}; pub fn gen_access_key(length: usize) -> Result { diff --git a/policy/tests/policy_is_allowed.rs b/policy/tests/policy_is_allowed.rs index b72be6dc..44471d07 100644 --- a/policy/tests/policy_is_allowed.rs +++ b/policy/tests/policy_is_allowed.rs @@ -1,7 +1,7 @@ -use policy::policy::action::Action; -use policy::policy::action::S3Action::*; use policy::policy::ActionSet; use policy::policy::Effect::*; +use policy::policy::action::Action; +use policy::policy::action::S3Action::*; use policy::policy::*; use serde_json::Value; use std::collections::HashMap; diff --git a/rustfs/src/admin/handlers/group.rs b/rustfs/src/admin/handlers/group.rs index 70025d37..e64ef4c1 100644 --- a/rustfs/src/admin/handlers/group.rs +++ b/rustfs/src/admin/handlers/group.rs @@ -5,7 +5,7 @@ use iam::{ }; use madmin::GroupAddRemove; use matchit::Params; -use s3s::{header::CONTENT_TYPE, s3_error, Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result}; +use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, header::CONTENT_TYPE, s3_error}; use serde::Deserialize; use serde_urlencoded::from_bytes; use tracing::warn; diff --git a/rustfs/src/admin/handlers/policys.rs b/rustfs/src/admin/handlers/policys.rs index 2ef7f42f..41329a0b 100644 --- a/rustfs/src/admin/handlers/policys.rs +++ b/rustfs/src/admin/handlers/policys.rs @@ -3,7 +3,7 @@ use http::{HeaderMap, StatusCode}; use iam::{error::is_err_no_such_user, get_global_action_cred, store::MappedPolicy}; use matchit::Params; use policy::policy::Policy; -use s3s::{header::CONTENT_TYPE, s3_error, Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result}; +use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, header::CONTENT_TYPE, s3_error}; use serde::Deserialize; use serde_urlencoded::from_bytes; use std::collections::HashMap; diff --git a/rustfs/src/admin/handlers/service_account.rs b/rustfs/src/admin/handlers/service_account.rs index f7d561ed..7893082f 100644 --- a/rustfs/src/admin/handlers/service_account.rs +++ b/rustfs/src/admin/handlers/service_account.rs @@ -16,7 +16,7 @@ use matchit::Params; use policy::policy::action::{Action, AdminAction}; use policy::policy::{Args, Policy}; use s3s::S3ErrorCode::InvalidRequest; -use s3s::{header::CONTENT_TYPE, s3_error, Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result}; +use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, header::CONTENT_TYPE, s3_error}; use serde::Deserialize; use serde_urlencoded::from_bytes; use std::collections::HashMap; diff --git a/rustfs/src/admin/handlers/sts.rs b/rustfs/src/admin/handlers/sts.rs index fd87be5a..f3a7eabc 100644 --- a/rustfs/src/admin/handlers/sts.rs +++ b/rustfs/src/admin/handlers/sts.rs @@ -10,8 +10,9 @@ use iam::{manager::get_token_signing_key, sys::SESSION_POLICY_NAME}; use matchit::Params; use policy::{auth::get_new_credentials_with_metadata, policy::Policy}; use s3s::{ + Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, dto::{AssumeRoleOutput, Credentials, Timestamp}, - s3_error, Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, + s3_error, }; use serde::Deserialize; use serde_json::Value; diff --git a/rustfs/src/admin/handlers/trace.rs b/rustfs/src/admin/handlers/trace.rs index 0134c4fd..55a489b5 100644 --- a/rustfs/src/admin/handlers/trace.rs +++ b/rustfs/src/admin/handlers/trace.rs @@ -1,9 +1,9 @@ -use ecstore::{peer_rest_client::PeerRestClient, GLOBAL_Endpoints}; +use ecstore::{GLOBAL_Endpoints, peer_rest_client::PeerRestClient}; use http::StatusCode; use hyper::Uri; use madmin::service_commands::ServiceTraceOpts; use matchit::Params; -use s3s::{s3_error, Body, S3Request, S3Response, S3Result}; +use s3s::{Body, S3Request, S3Response, S3Result, s3_error}; use tracing::warn; use crate::admin::router::Operation; diff --git a/rustfs/src/admin/handlers/user.rs b/rustfs/src/admin/handlers/user.rs index 36d7c593..a375d7ad 100644 --- a/rustfs/src/admin/handlers/user.rs +++ b/rustfs/src/admin/handlers/user.rs @@ -5,10 +5,10 @@ use iam::get_global_action_cred; use madmin::{AccountStatus, AddOrUpdateUserReq}; use matchit::Params; use policy::policy::{ - action::{Action, AdminAction}, Args, + action::{Action, AdminAction}, }; -use s3s::{header::CONTENT_TYPE, s3_error, Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result}; +use s3s::{Body, S3Error, S3ErrorCode, S3Request, S3Response, S3Result, header::CONTENT_TYPE, s3_error}; use serde::Deserialize; use serde_urlencoded::from_bytes; use tracing::warn; diff --git a/rustfs/src/auth.rs b/rustfs/src/auth.rs index 6102960b..a66a9242 100644 --- a/rustfs/src/auth.rs +++ b/rustfs/src/auth.rs @@ -7,13 +7,13 @@ use iam::get_global_action_cred; use iam::sys::SESSION_POLICY_NAME; use policy::auth; use policy::auth::get_claims_from_token_with_secret; +use s3s::S3Error; +use s3s::S3ErrorCode; +use s3s::S3Result; use s3s::auth::S3Auth; use s3s::auth::SecretKey; use s3s::auth::SimpleAuth; use s3s::s3_error; -use s3s::S3Error; -use s3s::S3ErrorCode; -use s3s::S3Result; use serde_json::Value; pub struct IAMAuth { diff --git a/rustfs/src/console.rs b/rustfs/src/console.rs index 4be5c5d8..4acfe209 100644 --- a/rustfs/src/console.rs +++ b/rustfs/src/console.rs @@ -1,10 +1,10 @@ use crate::license::get_license; use axum::{ + Router, body::Body, http::{Response, StatusCode}, response::IntoResponse, routing::get, - Router, }; use axum_extra::extract::Host; use rustfs_config::{RUSTFS_TLS_CERT, RUSTFS_TLS_KEY}; @@ -12,7 +12,7 @@ use std::io; use axum::response::Redirect; use axum_server::tls_rustls::RustlsConfig; -use http::{header, Uri}; +use http::{Uri, header}; use mime_guess::from_path; use rust_embed::RustEmbed; use serde::Serialize; diff --git a/rustfs/src/error.rs b/rustfs/src/error.rs index 3c4569a2..15a44c91 100644 --- a/rustfs/src/error.rs +++ b/rustfs/src/error.rs @@ -94,3 +94,238 @@ impl From for ApiError { serr.into() } } + +#[cfg(test)] +mod tests { + use super::*; + use s3s::{S3Error, S3ErrorCode}; + use std::io::{Error as IoError, ErrorKind}; + + #[test] + fn test_api_error_from_io_error() { + let io_error = IoError::new(ErrorKind::PermissionDenied, "permission denied"); + let api_error: ApiError = io_error.into(); + + assert_eq!(api_error.code, S3ErrorCode::InternalError); + assert!(api_error.message.contains("permission denied")); + assert!(api_error.source.is_some()); + } + + #[test] + fn test_api_error_from_io_error_different_kinds() { + let test_cases = vec![ + (ErrorKind::NotFound, "not found"), + (ErrorKind::InvalidInput, "invalid input"), + (ErrorKind::TimedOut, "timed out"), + (ErrorKind::WriteZero, "write zero"), + (ErrorKind::Other, "other error"), + ]; + + for (kind, message) in test_cases { + let io_error = IoError::new(kind, message); + let api_error: ApiError = io_error.into(); + + assert_eq!(api_error.code, S3ErrorCode::InternalError); + assert!(api_error.message.contains(message)); + assert!(api_error.source.is_some()); + + // Test that source can be downcast back to io::Error + let source = api_error.source.as_ref().unwrap(); + let downcast_io_error = source.downcast_ref::(); + assert!(downcast_io_error.is_some()); + assert_eq!(downcast_io_error.unwrap().kind(), kind); + } + } + + #[test] + fn test_api_error_other_function() { + let custom_error = "Custom API error"; + let api_error = ApiError::other(custom_error); + + assert_eq!(api_error.code, S3ErrorCode::InternalError); + assert_eq!(api_error.message, custom_error); + assert!(api_error.source.is_some()); + } + + #[test] + fn test_api_error_other_function_with_complex_error() { + let io_error = IoError::new(ErrorKind::InvalidData, "complex error"); + let api_error = ApiError::other(io_error); + + assert_eq!(api_error.code, S3ErrorCode::InternalError); + assert!(api_error.message.contains("complex error")); + assert!(api_error.source.is_some()); + + // Test that source can be downcast back to io::Error + let source = api_error.source.as_ref().unwrap(); + let downcast_io_error = source.downcast_ref::(); + assert!(downcast_io_error.is_some()); + assert_eq!(downcast_io_error.unwrap().kind(), ErrorKind::InvalidData); + } + + #[test] + fn test_api_error_from_storage_error() { + let storage_error = StorageError::BucketNotFound("test-bucket".to_string()); + let api_error: ApiError = storage_error.into(); + + assert_eq!(api_error.code, S3ErrorCode::NoSuchBucket); + assert!(api_error.message.contains("test-bucket")); + assert!(api_error.source.is_some()); + + // Test that source can be downcast back to StorageError + let source = api_error.source.as_ref().unwrap(); + let downcast_storage_error = source.downcast_ref::(); + assert!(downcast_storage_error.is_some()); + } + + #[test] + fn test_api_error_from_storage_error_mappings() { + let test_cases = vec![ + (StorageError::NotImplemented, S3ErrorCode::NotImplemented), + ( + StorageError::InvalidArgument("test".into(), "test".into(), "test".into()), + S3ErrorCode::InvalidArgument, + ), + (StorageError::MethodNotAllowed, S3ErrorCode::MethodNotAllowed), + (StorageError::BucketNotFound("test".into()), S3ErrorCode::NoSuchBucket), + (StorageError::BucketNotEmpty("test".into()), S3ErrorCode::BucketNotEmpty), + (StorageError::BucketNameInvalid("test".into()), S3ErrorCode::InvalidBucketName), + ( + StorageError::ObjectNameInvalid("test".into(), "test".into()), + S3ErrorCode::InvalidArgument, + ), + (StorageError::BucketExists("test".into()), S3ErrorCode::BucketAlreadyExists), + (StorageError::StorageFull, S3ErrorCode::ServiceUnavailable), + (StorageError::SlowDown, S3ErrorCode::SlowDown), + (StorageError::PrefixAccessDenied("test".into(), "test".into()), S3ErrorCode::AccessDenied), + (StorageError::ObjectNotFound("test".into(), "test".into()), S3ErrorCode::NoSuchKey), + (StorageError::ConfigNotFound, S3ErrorCode::NoSuchKey), + (StorageError::VolumeNotFound, S3ErrorCode::NoSuchBucket), + (StorageError::FileNotFound, S3ErrorCode::NoSuchKey), + (StorageError::FileVersionNotFound, S3ErrorCode::NoSuchVersion), + ]; + + for (storage_error, expected_code) in test_cases { + let api_error: ApiError = storage_error.into(); + assert_eq!(api_error.code, expected_code); + assert!(api_error.source.is_some()); + } + } + + #[test] + fn test_api_error_from_iam_error() { + let iam_error = iam::error::Error::other("IAM test error"); + let api_error: ApiError = iam_error.into(); + + // IAM error is first converted to StorageError, then to ApiError + assert!(api_error.source.is_some()); + assert!(api_error.message.contains("test error")); + } + + #[test] + fn test_api_error_to_s3_error() { + let api_error = ApiError { + code: S3ErrorCode::NoSuchBucket, + message: "Bucket not found".to_string(), + source: Some(Box::new(IoError::new(ErrorKind::NotFound, "not found"))), + }; + + let s3_error: S3Error = api_error.into(); + assert_eq!(*s3_error.code(), S3ErrorCode::NoSuchBucket); + assert!(s3_error.message().unwrap_or("").contains("Bucket not found")); + assert!(s3_error.source().is_some()); + } + + #[test] + fn test_api_error_to_s3_error_without_source() { + let api_error = ApiError { + code: S3ErrorCode::InvalidArgument, + message: "Invalid argument".to_string(), + source: None, + }; + + let s3_error: S3Error = api_error.into(); + assert_eq!(*s3_error.code(), S3ErrorCode::InvalidArgument); + assert!(s3_error.message().unwrap_or("").contains("Invalid argument")); + } + + #[test] + fn test_api_error_display() { + let api_error = ApiError { + code: S3ErrorCode::InternalError, + message: "Test error message".to_string(), + source: None, + }; + + assert_eq!(api_error.to_string(), "Test error message"); + } + + #[test] + fn test_api_error_debug() { + let api_error = ApiError { + code: S3ErrorCode::NoSuchKey, + message: "Object not found".to_string(), + source: Some(Box::new(IoError::new(ErrorKind::NotFound, "file not found"))), + }; + + let debug_str = format!("{:?}", api_error); + assert!(debug_str.contains("NoSuchKey")); + assert!(debug_str.contains("Object not found")); + } + + #[test] + fn test_api_error_roundtrip_through_io_error() { + let original_io_error = IoError::new(ErrorKind::PermissionDenied, "original permission error"); + + // Convert to ApiError + let api_error: ApiError = original_io_error.into(); + + // Verify the conversion preserved the information + assert_eq!(api_error.code, S3ErrorCode::InternalError); + assert!(api_error.message.contains("original permission error")); + assert!(api_error.source.is_some()); + + // Test that we can downcast back to the original io::Error + let source = api_error.source.as_ref().unwrap(); + let downcast_io_error = source.downcast_ref::(); + assert!(downcast_io_error.is_some()); + assert_eq!(downcast_io_error.unwrap().kind(), ErrorKind::PermissionDenied); + assert!(downcast_io_error.unwrap().to_string().contains("original permission error")); + } + + #[test] + fn test_api_error_chain_conversion() { + // Start with an io::Error + let io_error = IoError::new(ErrorKind::InvalidData, "invalid data"); + + // Convert to StorageError (simulating what happens in the codebase) + let storage_error = StorageError::other(io_error); + + // Convert to ApiError + let api_error: ApiError = storage_error.into(); + + // Verify the chain is preserved + assert!(api_error.source.is_some()); + + // Check that we can still access the original error information + let source = api_error.source.as_ref().unwrap(); + let downcast_storage_error = source.downcast_ref::(); + assert!(downcast_storage_error.is_some()); + } + + #[test] + fn test_api_error_error_trait_implementation() { + let api_error = ApiError { + code: S3ErrorCode::InternalError, + message: "Test error".to_string(), + source: Some(Box::new(IoError::other("source error"))), + }; + + // Test that it implements std::error::Error + let error: &dyn std::error::Error = &api_error; + assert_eq!(error.to_string(), "Test error"); + // ApiError doesn't implement Error::source() properly, so this would be None + // This is expected because ApiError is not a typical Error implementation + assert!(error.source().is_none()); + } +} diff --git a/rustfs/src/server/mod.rs b/rustfs/src/server/mod.rs index b1c764a1..15f851e5 100644 --- a/rustfs/src/server/mod.rs +++ b/rustfs/src/server/mod.rs @@ -1,6 +1,6 @@ mod service_state; -pub(crate) use service_state::wait_for_shutdown; +pub(crate) use service_state::SHUTDOWN_TIMEOUT; pub(crate) use service_state::ServiceState; pub(crate) use service_state::ServiceStateManager; pub(crate) use service_state::ShutdownSignal; -pub(crate) use service_state::SHUTDOWN_TIMEOUT; +pub(crate) use service_state::wait_for_shutdown; diff --git a/rustfs/src/server/service_state.rs b/rustfs/src/server/service_state.rs index 5390fb1e..a54c68fe 100644 --- a/rustfs/src/server/service_state.rs +++ b/rustfs/src/server/service_state.rs @@ -1,8 +1,8 @@ use atomic_enum::atomic_enum; -use std::sync::atomic::Ordering; use std::sync::Arc; +use std::sync::atomic::Ordering; use std::time::Duration; -use tokio::signal::unix::{signal, SignalKind}; +use tokio::signal::unix::{SignalKind, signal}; use tracing::info; // a configurable shutdown timeout @@ -10,7 +10,7 @@ pub(crate) const SHUTDOWN_TIMEOUT: Duration = Duration::from_secs(1); #[cfg(target_os = "linux")] fn notify_systemd(state: &str) { - use libsystemd::daemon::{notify, NotifyState}; + use libsystemd::daemon::{NotifyState, notify}; use tracing::{debug, error}; let notify_state = match state { "ready" => NotifyState::Ready, diff --git a/s3select/api/src/query/dispatcher.rs b/s3select/api/src/query/dispatcher.rs index 433ddf01..3799e067 100644 --- a/s3select/api/src/query/dispatcher.rs +++ b/s3select/api/src/query/dispatcher.rs @@ -5,9 +5,9 @@ use async_trait::async_trait; use crate::QueryResult; use super::{ + Query, execution::{Output, QueryStateMachine}, logical_planner::Plan, - Query, }; #[async_trait] diff --git a/s3select/api/src/query/execution.rs b/s3select/api/src/query/execution.rs index 99fb9671..01849779 100644 --- a/s3select/api/src/query/execution.rs +++ b/s3select/api/src/query/execution.rs @@ -1,7 +1,7 @@ use std::fmt::Display; use std::pin::Pin; -use std::sync::atomic::{AtomicPtr, Ordering}; use std::sync::Arc; +use std::sync::atomic::{AtomicPtr, Ordering}; use std::task::{Context, Poll}; use std::time::{Duration, Instant}; @@ -13,9 +13,9 @@ use futures::{Stream, StreamExt, TryStreamExt}; use crate::{QueryError, QueryResult}; +use super::Query; use super::logical_planner::Plan; use super::session::SessionCtx; -use super::Query; pub type QueryExecutionRef = Arc; diff --git a/s3select/api/src/query/session.rs b/s3select/api/src/query/session.rs index 581cdf39..6e739bb9 100644 --- a/s3select/api/src/query/session.rs +++ b/s3select/api/src/query/session.rs @@ -1,14 +1,14 @@ use std::sync::Arc; use datafusion::{ - execution::{context::SessionState, runtime_env::RuntimeEnvBuilder, SessionStateBuilder}, + execution::{SessionStateBuilder, context::SessionState, runtime_env::RuntimeEnvBuilder}, parquet::data_type::AsBytes, prelude::SessionContext, }; -use object_store::{memory::InMemory, path::Path, ObjectStore}; +use object_store::{ObjectStore, memory::InMemory, path::Path}; use tracing::error; -use crate::{object_store::EcObjectStore, QueryError, QueryResult}; +use crate::{QueryError, QueryResult, object_store::EcObjectStore}; use super::Context; diff --git a/s3select/api/src/server/dbms.rs b/s3select/api/src/server/dbms.rs index 85d32055..ee908634 100644 --- a/s3select/api/src/server/dbms.rs +++ b/s3select/api/src/server/dbms.rs @@ -1,12 +1,12 @@ use async_trait::async_trait; use crate::{ + QueryResult, query::{ + Query, execution::{Output, QueryStateMachineRef}, logical_planner::Plan, - Query, }, - QueryResult, }; pub struct QueryHandle { diff --git a/s3select/query/src/data_source/table_source.rs b/s3select/query/src/data_source/table_source.rs index 77df6e81..1fc06de6 100644 --- a/s3select/query/src/data_source/table_source.rs +++ b/s3select/query/src/data_source/table_source.rs @@ -8,7 +8,7 @@ use async_trait::async_trait; use datafusion::arrow::datatypes::SchemaRef; use datafusion::common::Result as DFResult; use datafusion::datasource::listing::ListingTable; -use datafusion::datasource::{provider_as_source, TableProvider}; +use datafusion::datasource::{TableProvider, provider_as_source}; use datafusion::error::DataFusionError; use datafusion::logical_expr::{LogicalPlan, LogicalPlanBuilder, TableProviderFilterPushDown, TableSource}; use datafusion::prelude::Expr; diff --git a/s3select/query/src/dispatcher/manager.rs b/s3select/query/src/dispatcher/manager.rs index ee5386e2..20e8bb13 100644 --- a/s3select/query/src/dispatcher/manager.rs +++ b/s3select/query/src/dispatcher/manager.rs @@ -6,7 +6,9 @@ use std::{ }; use api::{ + QueryError, QueryResult, query::{ + Query, ast::ExtStatement, dispatcher::QueryDispatcher, execution::{Output, QueryStateMachine}, @@ -14,9 +16,7 @@ use api::{ logical_planner::{LogicalPlanner, Plan}, parser::Parser, session::{SessionCtx, SessionCtxFactory}, - Query, }, - QueryError, QueryResult, }; use async_trait::async_trait; use datafusion::{ @@ -37,7 +37,7 @@ use s3s::dto::{FileHeaderInfo, SelectObjectContentInput}; use crate::{ execution::factory::QueryExecutionFactoryRef, - metadata::{base_table::BaseTableProvider, ContextProviderExtension, MetadataProvider, TableHandleProviderRef}, + metadata::{ContextProviderExtension, MetadataProvider, TableHandleProviderRef, base_table::BaseTableProvider}, sql::logical::planner::DefaultLogicalPlanner, }; diff --git a/s3select/query/src/execution/factory.rs b/s3select/query/src/execution/factory.rs index 9960d68a..4f9ba343 100644 --- a/s3select/query/src/execution/factory.rs +++ b/s3select/query/src/execution/factory.rs @@ -1,13 +1,13 @@ use std::sync::Arc; use api::{ + QueryError, query::{ execution::{QueryExecutionFactory, QueryExecutionRef, QueryStateMachineRef}, logical_planner::Plan, optimizer::Optimizer, scheduler::SchedulerRef, }, - QueryError, }; use async_trait::async_trait; diff --git a/s3select/query/src/execution/scheduler/local.rs b/s3select/query/src/execution/scheduler/local.rs index e105d4b9..43b5adfe 100644 --- a/s3select/query/src/execution/scheduler/local.rs +++ b/s3select/query/src/execution/scheduler/local.rs @@ -4,7 +4,7 @@ use api::query::scheduler::{ExecutionResults, Scheduler}; use async_trait::async_trait; use datafusion::error::DataFusionError; use datafusion::execution::context::TaskContext; -use datafusion::physical_plan::{execute_stream, ExecutionPlan}; +use datafusion::physical_plan::{ExecutionPlan, execute_stream}; pub struct LocalScheduler {} diff --git a/s3select/query/src/instance.rs b/s3select/query/src/instance.rs index 34492063..5c5f9304 100644 --- a/s3select/query/src/instance.rs +++ b/s3select/query/src/instance.rs @@ -1,11 +1,11 @@ use std::sync::Arc; use api::{ + QueryResult, query::{ - dispatcher::QueryDispatcher, execution::QueryStateMachineRef, logical_planner::Plan, session::SessionCtxFactory, Query, + Query, dispatcher::QueryDispatcher, execution::QueryStateMachineRef, logical_planner::Plan, session::SessionCtxFactory, }, server::dbms::{DatabaseManagerSystem, QueryHandle}, - QueryResult, }; use async_trait::async_trait; use derive_builder::Builder; diff --git a/s3select/query/src/metadata/mod.rs b/s3select/query/src/metadata/mod.rs index 04a71d4e..fd7c215a 100644 --- a/s3select/query/src/metadata/mod.rs +++ b/s3select/query/src/metadata/mod.rs @@ -10,7 +10,7 @@ use datafusion::logical_expr::{AggregateUDF, ScalarUDF, TableSource, WindowUDF}; use datafusion::variable::VarType; use datafusion::{ config::ConfigOptions, - sql::{planner::ContextProvider, TableReference}, + sql::{TableReference, planner::ContextProvider}, }; use crate::data_source::table_source::{TableHandle, TableSourceAdapter}; diff --git a/s3select/query/src/sql/analyzer.rs b/s3select/query/src/sql/analyzer.rs index 6507c842..b68c5574 100644 --- a/s3select/query/src/sql/analyzer.rs +++ b/s3select/query/src/sql/analyzer.rs @@ -1,6 +1,6 @@ +use api::QueryResult; use api::query::analyzer::Analyzer; use api::query::session::SessionCtx; -use api::QueryResult; use datafusion::logical_expr::LogicalPlan; use datafusion::optimizer::analyzer::Analyzer as DFAnalyzer; diff --git a/s3select/query/src/sql/logical/optimizer.rs b/s3select/query/src/sql/logical/optimizer.rs index e97e2967..ed860bcc 100644 --- a/s3select/query/src/sql/logical/optimizer.rs +++ b/s3select/query/src/sql/logical/optimizer.rs @@ -1,22 +1,22 @@ use std::sync::Arc; use api::{ - query::{analyzer::AnalyzerRef, logical_planner::QueryPlan, session::SessionCtx}, QueryResult, + query::{analyzer::AnalyzerRef, logical_planner::QueryPlan, session::SessionCtx}, }; use datafusion::{ execution::SessionStateBuilder, logical_expr::LogicalPlan, optimizer::{ - common_subexpr_eliminate::CommonSubexprEliminate, decorrelate_predicate_subquery::DecorrelatePredicateSubquery, - eliminate_cross_join::EliminateCrossJoin, eliminate_duplicated_expr::EliminateDuplicatedExpr, - eliminate_filter::EliminateFilter, eliminate_join::EliminateJoin, eliminate_limit::EliminateLimit, - eliminate_outer_join::EliminateOuterJoin, extract_equijoin_predicate::ExtractEquijoinPredicate, - filter_null_join_keys::FilterNullJoinKeys, propagate_empty_relation::PropagateEmptyRelation, - push_down_filter::PushDownFilter, push_down_limit::PushDownLimit, + OptimizerRule, common_subexpr_eliminate::CommonSubexprEliminate, + decorrelate_predicate_subquery::DecorrelatePredicateSubquery, eliminate_cross_join::EliminateCrossJoin, + eliminate_duplicated_expr::EliminateDuplicatedExpr, eliminate_filter::EliminateFilter, eliminate_join::EliminateJoin, + eliminate_limit::EliminateLimit, eliminate_outer_join::EliminateOuterJoin, + extract_equijoin_predicate::ExtractEquijoinPredicate, filter_null_join_keys::FilterNullJoinKeys, + propagate_empty_relation::PropagateEmptyRelation, push_down_filter::PushDownFilter, push_down_limit::PushDownLimit, replace_distinct_aggregate::ReplaceDistinctWithAggregate, scalar_subquery_to_join::ScalarSubqueryToJoin, simplify_expressions::SimplifyExpressions, single_distinct_to_groupby::SingleDistinctToGroupBy, - unwrap_cast_in_comparison::UnwrapCastInComparison, OptimizerRule, + unwrap_cast_in_comparison::UnwrapCastInComparison, }, }; use tracing::debug; diff --git a/s3select/query/src/sql/optimizer.rs b/s3select/query/src/sql/optimizer.rs index 2ceb0cb8..a77b1b1a 100644 --- a/s3select/query/src/sql/optimizer.rs +++ b/s3select/query/src/sql/optimizer.rs @@ -1,11 +1,11 @@ use std::sync::Arc; use api::{ - query::{logical_planner::QueryPlan, optimizer::Optimizer, physical_planner::PhysicalPlanner, session::SessionCtx}, QueryResult, + query::{logical_planner::QueryPlan, optimizer::Optimizer, physical_planner::PhysicalPlanner, session::SessionCtx}, }; use async_trait::async_trait; -use datafusion::physical_plan::{displayable, ExecutionPlan}; +use datafusion::physical_plan::{ExecutionPlan, displayable}; use tracing::debug; use super::{ diff --git a/s3select/query/src/sql/parser.rs b/s3select/query/src/sql/parser.rs index 84732c2b..ac98075c 100644 --- a/s3select/query/src/sql/parser.rs +++ b/s3select/query/src/sql/parser.rs @@ -1,8 +1,8 @@ use std::{collections::VecDeque, fmt::Display}; use api::{ - query::{ast::ExtStatement, parser::Parser as RustFsParser}, ParserSnafu, + query::{ast::ExtStatement, parser::Parser as RustFsParser}, }; use datafusion::sql::sqlparser::{ dialect::Dialect, diff --git a/s3select/query/src/sql/physical/optimizer.rs b/s3select/query/src/sql/physical/optimizer.rs index 12f16e3d..84032582 100644 --- a/s3select/query/src/sql/physical/optimizer.rs +++ b/s3select/query/src/sql/physical/optimizer.rs @@ -1,7 +1,7 @@ use std::sync::Arc; -use api::query::session::SessionCtx; use api::QueryResult; +use api::query::session::SessionCtx; use datafusion::physical_optimizer::PhysicalOptimizerRule; use datafusion::physical_plan::ExecutionPlan; diff --git a/s3select/query/src/sql/physical/planner.rs b/s3select/query/src/sql/physical/planner.rs index 5857b6b6..eda5c478 100644 --- a/s3select/query/src/sql/physical/planner.rs +++ b/s3select/query/src/sql/physical/planner.rs @@ -1,15 +1,15 @@ use std::sync::Arc; +use api::QueryResult; use api::query::physical_planner::PhysicalPlanner; use api::query::session::SessionCtx; -use api::QueryResult; use async_trait::async_trait; use datafusion::execution::SessionStateBuilder; use datafusion::logical_expr::LogicalPlan; +use datafusion::physical_optimizer::PhysicalOptimizerRule; use datafusion::physical_optimizer::aggregate_statistics::AggregateStatistics; use datafusion::physical_optimizer::coalesce_batches::CoalesceBatches; use datafusion::physical_optimizer::join_selection::JoinSelection; -use datafusion::physical_optimizer::PhysicalOptimizerRule; use datafusion::physical_plan::ExecutionPlan; use datafusion::physical_planner::{ DefaultPhysicalPlanner as DFDefaultPhysicalPlanner, ExtensionPlanner, PhysicalPlanner as DFPhysicalPlanner, diff --git a/s3select/query/src/sql/planner.rs b/s3select/query/src/sql/planner.rs index a6c9f8c1..1705294c 100644 --- a/s3select/query/src/sql/planner.rs +++ b/s3select/query/src/sql/planner.rs @@ -1,10 +1,10 @@ use api::{ + QueryError, QueryResult, query::{ ast::ExtStatement, logical_planner::{LogicalPlanner, Plan, QueryPlan}, session::SessionCtx, }, - QueryError, QueryResult, }; use async_recursion::async_recursion; use async_trait::async_trait;