refactor(ecstore): simplify bucket metadata error handling by using ConfigNotFound

- Remove specific bucket metadata error variants (BucketPolicyNotFound, etc.)
- Unify all configuration-not-found errors to use StorageError::ConfigNotFound
- Simplify error handling in bucket metadata system by removing redundant error conversions
- Clean up error matching patterns in metadata getter methods
- Remove debug print statement from get_bucket_targets_config
- Update error handling in S3 API layer to use unified ConfigNotFound error

This change improves code maintainability by reducing error type complexity
while preserving the same error semantics for API consumers.
This commit is contained in:
weisd
2025-06-23 17:04:14 +08:00
parent 811d1a598b
commit 317cadaed8
4 changed files with 39 additions and 126 deletions
-10
View File
@@ -32,19 +32,9 @@ impl BucketMetadataError {
}
}
impl From<BucketMetadataError> for Error {
fn from(e: BucketMetadataError) -> Self {
match e {
BucketMetadataError::BucketPolicyNotFound => Error::BucketPolicyNotFound,
_ => Error::other(e),
}
}
}
impl From<Error> for BucketMetadataError {
fn from(e: Error) -> Self {
match e {
Error::BucketPolicyNotFound => BucketMetadataError::BucketPolicyNotFound,
Error::Io(e) => e.into(),
_ => BucketMetadataError::other(e),
}
+21 -109
View File
@@ -1,10 +1,4 @@
use std::collections::HashSet;
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::{BUCKET_LIFECYCLE_CONFIG, load_bucket_metadata_parse};
use crate::bucket::utils::{deserialize, is_meta_bucketname};
use crate::cmd::bucket_targets;
@@ -18,6 +12,10 @@ use s3s::dto::{
BucketLifecycleConfiguration, NotificationConfiguration, ObjectLockConfiguration, ReplicationConfiguration,
ServerSideEncryptionConfiguration, Tagging, VersioningConfiguration,
};
use std::collections::HashSet;
use std::sync::OnceLock;
use std::time::Duration;
use std::{collections::HashMap, sync::Arc};
use time::OffsetDateTime;
use tokio::sync::RwLock;
use tokio::time::sleep;
@@ -400,85 +398,46 @@ impl BucketMetadataSys {
}
pub async fn get_bucket_policy(&self, bucket: &str) -> Result<(BucketPolicy, OffsetDateTime)> {
let bm = match self.get_config(bucket).await {
Ok((res, _)) => res,
Err(err) => {
warn!("get_bucket_policy err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketPolicyNotFound.into())
} else {
Err(err)
};
}
};
let (bm, _) = self.get_config(bucket).await?;
if let Some(config) = &bm.policy_config {
Ok((config.clone(), bm.policy_config_updated_at))
} else {
Err(BucketMetadataError::BucketPolicyNotFound.into())
Err(Error::ConfigNotFound)
}
}
pub async fn get_tagging_config(&self, bucket: &str) -> Result<(Tagging, OffsetDateTime)> {
let bm = match self.get_config(bucket).await {
Ok((res, _)) => res,
Err(err) => {
warn!("get_tagging_config err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::TaggingNotFound.into())
} else {
Err(err)
};
}
};
let (bm, _) = self.get_config(bucket).await?;
if let Some(config) = &bm.tagging_config {
Ok((config.clone(), bm.tagging_config_updated_at))
} else {
Err(BucketMetadataError::TaggingNotFound.into())
Err(Error::ConfigNotFound)
}
}
pub async fn get_object_lock_config(&self, bucket: &str) -> Result<(ObjectLockConfiguration, OffsetDateTime)> {
let bm = match self.get_config(bucket).await {
Ok((res, _)) => res,
Err(err) => {
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketObjectLockConfigNotFound.into())
} else {
Err(err)
};
}
};
let (bm, _) = self.get_config(bucket).await?;
if let Some(config) = &bm.object_lock_config {
Ok((config.clone(), bm.object_lock_config_updated_at))
} else {
Err(BucketMetadataError::BucketObjectLockConfigNotFound.into())
Err(Error::ConfigNotFound)
}
}
pub async fn get_lifecycle_config(&self, bucket: &str) -> Result<(BucketLifecycleConfiguration, OffsetDateTime)> {
let bm = match self.get_config(bucket).await {
Ok((res, _)) => res,
Err(err) => {
warn!("get_lifecycle_config err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketLifecycleNotFound.into())
} else {
Err(err)
};
}
};
let (bm, _) = self.get_config(bucket).await?;
if let Some(config) = &bm.lifecycle_config {
if config.rules.is_empty() {
Err(BucketMetadataError::BucketLifecycleNotFound.into())
Err(Error::ConfigNotFound)
} else {
Ok((config.clone(), bm.lifecycle_config_updated_at))
}
} else {
Err(BucketMetadataError::BucketLifecycleNotFound.into())
Err(Error::ConfigNotFound)
}
}
@@ -499,22 +458,12 @@ impl BucketMetadataSys {
}
pub async fn get_sse_config(&self, bucket: &str) -> Result<(ServerSideEncryptionConfiguration, OffsetDateTime)> {
let bm = match self.get_config(bucket).await {
Ok((res, _)) => res,
Err(err) => {
warn!("get_sse_config err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketSSEConfigNotFound.into())
} else {
Err(err)
};
}
};
let (bm, _) = self.get_config(bucket).await?;
if let Some(config) = &bm.sse_config {
Ok((config.clone(), bm.encryption_config_updated_at))
} else {
Err(BucketMetadataError::BucketSSEConfigNotFound.into())
Err(Error::ConfigNotFound)
}
}
@@ -530,42 +479,17 @@ impl BucketMetadataSys {
}
pub async fn get_quota_config(&self, bucket: &str) -> Result<(BucketQuota, OffsetDateTime)> {
let bm = match self.get_config(bucket).await {
Ok((res, _)) => res,
Err(err) => {
warn!("get_quota_config err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketQuotaConfigNotFound.into())
} else {
Err(err)
};
}
};
let (bm, _) = self.get_config(bucket).await?;
if let Some(config) = &bm.quota_config {
Ok((config.clone(), bm.quota_config_updated_at))
} else {
Err(BucketMetadataError::BucketQuotaConfigNotFound.into())
Err(Error::ConfigNotFound)
}
}
pub async fn get_replication_config(&self, bucket: &str) -> Result<(ReplicationConfiguration, OffsetDateTime)> {
let (bm, reload) = match self.get_config(bucket).await {
Ok(res) => {
if res.0.replication_config.is_none() {
return Err(BucketMetadataError::BucketReplicationConfigNotFound.into());
}
res
}
Err(err) => {
warn!("get_replication_config err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketReplicationConfigNotFound.into())
} else {
Err(err)
};
}
};
let (bm, reload) = self.get_config(bucket).await?;
if let Some(config) = &bm.replication_config {
if reload {
@@ -574,24 +498,12 @@ impl BucketMetadataSys {
//println!("549 {:?}", config.clone());
Ok((config.clone(), bm.replication_config_updated_at))
} else {
Err(BucketMetadataError::BucketReplicationConfigNotFound.into())
Err(Error::ConfigNotFound)
}
}
pub async fn get_bucket_targets_config(&self, bucket: &str) -> Result<BucketTargets> {
let (bm, reload) = match self.get_config(bucket).await {
Ok(res) => res,
Err(err) => {
warn!("get_replication_config err {:?}", &err);
return if err == Error::ConfigNotFound {
Err(BucketMetadataError::BucketRemoteTargetNotFound.into())
} else {
Err(err)
};
}
};
println!("573");
let (bm, reload) = self.get_config(bucket).await?;
if let Some(config) = &bm.bucket_target_config {
if reload {
@@ -601,7 +513,7 @@ impl BucketMetadataSys {
Ok(config.clone())
} else {
Err(BucketMetadataError::BucketRemoteTargetNotFound.into())
Err(Error::ConfigNotFound)
}
}
}
+17 -6
View File
@@ -1,5 +1,6 @@
use rustfs_utils::path::decode_dir_object;
use crate::bucket::error::BucketMetadataError;
use crate::disk::error::DiskError;
pub type Error = StorageError;
@@ -164,9 +165,6 @@ pub enum StorageError {
#[error("first disk wait")]
FirstDiskWait,
#[error("Bucket policy not found")]
BucketPolicyNotFound,
#[error("Io error: {0}")]
Io(std::io::Error),
}
@@ -255,6 +253,22 @@ impl From<StorageError> for DiskError {
}
}
impl From<BucketMetadataError> for Error {
fn from(e: BucketMetadataError) -> Self {
match e {
BucketMetadataError::TaggingNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketPolicyNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketObjectLockConfigNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketLifecycleNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketSSEConfigNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketQuotaConfigNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketReplicationConfigNotFound => Error::ConfigNotFound,
BucketMetadataError::BucketRemoteTargetNotFound => Error::ConfigNotFound,
_ => Error::other(e),
}
}
}
impl From<std::io::Error> for StorageError {
fn from(e: std::io::Error) -> Self {
match e.downcast::<StorageError>() {
@@ -379,7 +393,6 @@ impl Clone for StorageError {
StorageError::FirstDiskWait => StorageError::FirstDiskWait,
StorageError::TooManyOpenFiles => StorageError::TooManyOpenFiles,
StorageError::NoHealRequired => StorageError::NoHealRequired,
StorageError::BucketPolicyNotFound => StorageError::BucketPolicyNotFound,
}
}
}
@@ -442,7 +455,6 @@ impl StorageError {
StorageError::ConfigNotFound => 0x35,
StorageError::TooManyOpenFiles => 0x36,
StorageError::NoHealRequired => 0x37,
StorageError::BucketPolicyNotFound => 0x38,
}
}
@@ -507,7 +519,6 @@ impl StorageError {
0x35 => Some(StorageError::ConfigNotFound),
0x36 => Some(StorageError::TooManyOpenFiles),
0x37 => Some(StorageError::NoHealRequired),
0x38 => Some(StorageError::BucketPolicyNotFound),
_ => None,
}
}
+1 -1
View File
@@ -1601,7 +1601,7 @@ impl S3 for FS {
let cfg = match PolicySys::get(&bucket).await {
Ok(res) => res,
Err(err) => {
if StorageError::BucketPolicyNotFound == err {
if StorageError::ConfigNotFound == err {
return Err(s3_error!(NoSuchBucketPolicy));
}
return Err(S3Error::with_message(S3ErrorCode::InternalError, err.to_string()));