From 53337a6f71e70ac67b1cda80ea2a21225766b31e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Thu, 18 Jun 2026 00:47:23 +0800 Subject: [PATCH] refactor: move HTTP range helper contracts (#3533) --- Cargo.lock | 1 + .../bucket/lifecycle/bucket_lifecycle_ops.rs | 4 +- .../replication/replication_resyncer.rs | 5 +- crates/ecstore/src/client/object_api_utils.rs | 3 +- crates/ecstore/src/config/com.rs | 4 +- crates/ecstore/src/error.rs | 10 +- crates/ecstore/src/rebalance.rs | 13 +- crates/ecstore/src/set_disk.rs | 5 +- crates/ecstore/src/sets.rs | 3 +- crates/ecstore/src/store.rs | 5 +- crates/ecstore/src/store_api.rs | 1 + crates/ecstore/src/store_api/readers.rs | 115 ++------------ crates/protocols/src/swift/dlo.rs | 2 +- crates/protocols/src/swift/object.rs | 6 +- crates/protocols/src/swift/slo.rs | 2 +- crates/s3select-api/Cargo.toml | 1 + crates/s3select-api/src/object_store.rs | 3 +- crates/scanner/src/scanner.rs | 2 +- crates/storage-api/src/lib.rs | 2 +- crates/storage-api/src/object.rs | 144 ++++++++++++++++++ docs/architecture/crate-boundaries.md | 2 +- docs/architecture/migration-progress.md | 38 +++-- rustfs/src/app/multipart_usecase.rs | 4 +- rustfs/src/app/object_usecase.rs | 15 +- rustfs/src/error.rs | 11 ++ rustfs/src/storage/options.rs | 4 +- scripts/check_architecture_migration_rules.sh | 15 +- 27 files changed, 270 insertions(+), 150 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index bebfbecf5..fd6ffe9cd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9976,6 +9976,7 @@ dependencies = [ "pin-project-lite", "rustfs-common", "rustfs-ecstore", + "rustfs-storage-api", "s3s", "serde_json", "snafu 0.9.1", diff --git a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs index 9b7c70fd4..d41797cf2 100644 --- a/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs +++ b/crates/ecstore/src/bucket/lifecycle/bucket_lifecycle_ops.rs @@ -38,8 +38,7 @@ use crate::global::{GLOBAL_LifecycleSys, GLOBAL_TierConfigMgr, get_global_deploy use crate::set_disk::{MAX_PARTS_COUNT, RUSTFS_MULTIPART_BUCKET_KEY, RUSTFS_MULTIPART_OBJECT_KEY, SetDisks}; use crate::store::ECStore; use crate::store_api::{ - GetObjectReader, HTTPRangeSpec, ListOperations, MultipartOperations, ObjectInfo, ObjectOperations, ObjectOptions, - ObjectToDelete, + GetObjectReader, ListOperations, MultipartOperations, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, }; use crate::tier::warm_backend::WarmBackendGetOpts; use async_channel::{Receiver as A_Receiver, Sender as A_Sender, bounded}; @@ -61,6 +60,7 @@ use rustfs_filemeta::{ VersionPurgeStatusType, get_file_info, is_restored_object_on_disk, }; use rustfs_s3_types::EventName; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_utils::{get_env_i64, get_env_usize, path::encode_dir_object, string::strings_has_prefix_fold}; use s3s::dto::{ BucketLifecycleConfiguration, DefaultRetention, ExpirationStatus, ReplicationConfiguration, RestoreRequest, diff --git a/crates/ecstore/src/bucket/replication/replication_resyncer.rs b/crates/ecstore/src/bucket/replication/replication_resyncer.rs index fabfd36a1..747de6e14 100644 --- a/crates/ecstore/src/bucket/replication/replication_resyncer.rs +++ b/crates/ecstore/src/bucket/replication/replication_resyncer.rs @@ -34,8 +34,8 @@ use crate::global::get_global_bucket_monitor; use crate::resolve_object_store_handle; use crate::set_disk::get_lock_acquire_timeout; use crate::store_api::{ - DeletedObject, HTTPRangeSpec, ListOperations, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions, - ObjectToDelete, WalkOptions, + DeletedObject, ListOperations, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, + WalkOptions, }; use aws_sdk_s3::error::{ProvideErrorMetadata, SdkError}; use aws_sdk_s3::operation::head_object::{HeadObjectError, HeadObjectOutput}; @@ -61,6 +61,7 @@ use rustfs_filemeta::{ get_replication_state, parse_replicate_decision, replication_statuses_map, target_reset_header, version_purge_statuses_map, }; use rustfs_s3_types::EventName; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_utils::http::{ AMZ_BUCKET_REPLICATION_STATUS, AMZ_OBJECT_TAGGING, AMZ_TAGGING_DIRECTIVE, CONTENT_ENCODING, HeaderExt as _, SSEC_ALGORITHM_HEADER, SSEC_KEY_HEADER, SSEC_KEY_MD5_HEADER, SUFFIX_OBJECTLOCK_LEGALHOLD_TIMESTAMP, diff --git a/crates/ecstore/src/client/object_api_utils.rs b/crates/ecstore/src/client/object_api_utils.rs index 30f638cd8..479c5dee6 100644 --- a/crates/ecstore/src/client/object_api_utils.rs +++ b/crates/ecstore/src/client/object_api_utils.rs @@ -25,9 +25,10 @@ use std::{collections::HashMap, io::Cursor, sync::Arc}; use tokio::io::BufReader; use crate::error::ErrorResponse; -use crate::store_api::{GetObjectReader, HTTPRangeSpec, ObjectInfo, ObjectOptions}; +use crate::store_api::{GetObjectReader, ObjectInfo, ObjectOptions}; use rustfs_filemeta::ObjectPartInfo; use rustfs_rio::HashReader; +use rustfs_storage_api::HTTPRangeSpec; use s3s::S3ErrorCode; //#[derive(Clone)] diff --git a/crates/ecstore/src/config/com.rs b/crates/ecstore/src/config/com.rs index 670fdf022..c19eea58d 100644 --- a/crates/ecstore/src/config/com.rs +++ b/crates/ecstore/src/config/com.rs @@ -1217,7 +1217,7 @@ mod tests { use crate::error::{Error, Result}; use crate::global::{is_dist_erasure, is_erasure, is_erasure_sd, update_erasure_type}; use crate::set_disk::SetDisks; - use crate::store_api::{GetObjectReader, HTTPRangeSpec, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOptions, PutObjReader}; + use crate::store_api::{GetObjectReader, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOptions, PutObjReader}; use http::HeaderMap; use rustfs_config::audit::{AUDIT_AMQP_SUB_SYS, AUDIT_KAFKA_SUB_SYS, AUDIT_MQTT_SUB_SYS, AUDIT_WEBHOOK_SUB_SYS}; use rustfs_config::notify::{ @@ -1231,7 +1231,7 @@ mod tests { use rustfs_lock::client::LockClient; use rustfs_lock::client::local::LocalClient; use rustfs_lock::{LockError, LockInfo, LockResponse, LockStats}; - use rustfs_storage_api::StorageAdminApi; + use rustfs_storage_api::{HTTPRangeSpec, StorageAdminApi}; use serde_json::Value; use serial_test::serial; use std::collections::HashMap; diff --git a/crates/ecstore/src/error.rs b/crates/ecstore/src/error.rs index a75ab8769..62077dfad 100644 --- a/crates/ecstore/src/error.rs +++ b/crates/ecstore/src/error.rs @@ -14,7 +14,7 @@ use crate::bucket::error::BucketMetadataError; use crate::disk::error::DiskError; -use rustfs_storage_api::StorageErrorCode; +use rustfs_storage_api::{HTTPRangeError, StorageErrorCode}; use rustfs_utils::path::decode_dir_object; use s3s::{S3Error, S3ErrorCode}; @@ -218,6 +218,14 @@ impl StorageError { } } +impl From for StorageError { + fn from(err: HTTPRangeError) -> Self { + match err { + HTTPRangeError::InvalidRangeSpec(message) => Self::InvalidRangeSpec(message), + } + } +} + impl From for StorageError { fn from(e: DiskError) -> Self { match e { diff --git a/crates/ecstore/src/rebalance.rs b/crates/ecstore/src/rebalance.rs index b310b9509..ab90da22b 100644 --- a/crates/ecstore/src/rebalance.rs +++ b/crates/ecstore/src/rebalance.rs @@ -23,11 +23,11 @@ use crate::global::get_global_endpoints; use crate::pools::ListCallback; use crate::set_disk::{SetDisks, get_lock_acquire_timeout}; use crate::store::ECStore; -use crate::store_api::{GetObjectReader, HTTPRangeSpec, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions}; +use crate::store_api::{GetObjectReader, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions}; use http::HeaderMap; use rand::RngExt as _; use rustfs_filemeta::{FileInfo, MetaCacheEntries, MetaCacheEntry, MetadataResolutionParams}; -use rustfs_storage_api::StorageAdminApi; +use rustfs_storage_api::{HTTPRangeSpec, StorageAdminApi}; use rustfs_utils::path::encode_dir_object; use serde::{Deserialize, Serialize}; use std::collections::HashSet; @@ -3167,10 +3167,10 @@ mod rebalance_unit_tests { use super::percent_free_ratio; use super::rebalance_goal_reached; use super::{ - DiskError, GetObjectReader, HTTPRangeSpec, MigrationBackend, MigrationVersionResult, ObjectInfo, ObjectOptions, - RebalSaveOpt, RebalStatus, RebalanceBucketOutcome, RebalanceCleanupWarnings, RebalanceEntryOutcome, RebalanceInfo, - RebalanceMeta, RebalanceStats, RebalanceTerminalEvent, apply_rebalance_save_option, apply_rebalance_terminal_event, - apply_stopped_at, classify_rebalance_terminal_event, clone_arc_by_index, clone_first_arc, clone_rebalance_pool_stats, + DiskError, GetObjectReader, MigrationBackend, MigrationVersionResult, ObjectInfo, ObjectOptions, RebalSaveOpt, + RebalStatus, RebalanceBucketOutcome, RebalanceCleanupWarnings, RebalanceEntryOutcome, RebalanceInfo, RebalanceMeta, + RebalanceStats, RebalanceTerminalEvent, apply_rebalance_save_option, apply_rebalance_terminal_event, apply_stopped_at, + classify_rebalance_terminal_event, clone_arc_by_index, clone_first_arc, clone_rebalance_pool_stats, complete_rebalance_pools_at_goal, complete_rebalance_pools_with_empty_queue, defer_bucket_in_rebalance_queue, ensure_rebalance_listing_disks_available, ensure_rebalance_not_decommissioning, ensure_valid_rebalance_pool_index, has_deferred_rebalance_error, is_rebalance_stopped_terminal_event, is_transient_rebalance_error, @@ -3197,6 +3197,7 @@ mod rebalance_unit_tests { use rustfs_filemeta::FileInfo; use rustfs_filemeta::TRANSITION_COMPLETE; use rustfs_rio::Index; + use rustfs_storage_api::HTTPRangeSpec; use s3s::dto::ReplicationConfiguration; use serde::Serialize; use std::io::Cursor; diff --git a/crates/ecstore/src/set_disk.rs b/crates/ecstore/src/set_disk.rs index 991ac4d29..d8180b3ce 100644 --- a/crates/ecstore/src/set_disk.rs +++ b/crates/ecstore/src/set_disk.rs @@ -52,8 +52,8 @@ use crate::{ event_notification::{EventArgs, send_event}, global::{GLOBAL_LOCAL_DISK_MAP, GLOBAL_LOCAL_DISK_SET_DRIVES, get_global_deployment_id, is_dist_erasure}, store_api::{ - DeletedObject, GetObjectReader, HTTPRangeSpec, HealOperations, ListObjectsV2Info, ListOperations, MultipartOperations, - NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, PutObjReader, + DeletedObject, GetObjectReader, HealOperations, ListObjectsV2Info, ListOperations, MultipartOperations, NamespaceLocking, + ObjectIO, ObjectInfo, ObjectOperations, PutObjReader, }, store_init::load_format_erasure, }; @@ -85,6 +85,7 @@ use rustfs_object_capacity::capacity_scope::{ CapacityScope, CapacityScopeDisk, record_capacity_scope, record_global_dirty_scope, }; use rustfs_s3_types::EventName; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_storage_api::{ BucketInfo, BucketOperations, BucketOptions, CompletePart, DeleteBucketOptions, ListMultipartsInfo, ListPartsInfo, MakeBucketOptions, MultipartInfo, MultipartUploadResult, PartInfo, diff --git a/crates/ecstore/src/sets.rs b/crates/ecstore/src/sets.rs index 3afae538d..4fe678cd0 100644 --- a/crates/ecstore/src/sets.rs +++ b/crates/ecstore/src/sets.rs @@ -28,7 +28,7 @@ use crate::{ global::{GLOBAL_LOCAL_DISK_SET_DRIVES, get_global_lock_clients, is_dist_erasure}, set_disk::SetDisks, store_api::{ - DeletedObject, GetObjectReader, HTTPRangeSpec, HealOperations, ListObjectVersionsInfo, ListObjectsV2Info, ListOperations, + DeletedObject, GetObjectReader, HealOperations, ListObjectVersionsInfo, ListObjectsV2Info, ListOperations, MultipartOperations, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, PutObjReader, }, @@ -49,6 +49,7 @@ use rustfs_lock::NamespaceLockWrapper; use rustfs_lock::client::LockClient; use rustfs_madmin::heal_commands::{HealDriveInfo, HealResultItem}; use rustfs_storage_api::CompletePart; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_storage_api::{ BucketInfo, BucketOperations, BucketOptions, DeleteBucketOptions, ListMultipartsInfo, ListPartsInfo, MakeBucketOptions, MultipartInfo, MultipartUploadResult, PartInfo, diff --git a/crates/ecstore/src/store.rs b/crates/ecstore/src/store.rs index 43801c51b..d2207c417 100644 --- a/crates/ecstore/src/store.rs +++ b/crates/ecstore/src/store.rs @@ -63,8 +63,8 @@ use crate::{ rpc::S3PeerSys, sets::Sets, store_api::{ - DeletedObject, GetObjectReader, HTTPRangeSpec, HealOperations, ListObjectsV2Info, ListOperations, MultipartOperations, - NamespaceLocking, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, PutObjReader, + DeletedObject, GetObjectReader, HealOperations, ListObjectsV2Info, ListOperations, MultipartOperations, NamespaceLocking, + ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, PutObjReader, }, store_init, }; @@ -78,6 +78,7 @@ use rustfs_config::server_config::{Config, get_global_server_config, set_global_ use rustfs_filemeta::FileInfo; use rustfs_lock::{LocalClient, LockClient, NamespaceLockWrapper}; use rustfs_madmin::heal_commands::HealResultItem; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_storage_api::{ BucketInfo, BucketOperations, BucketOptions, CompletePart, DeleteBucketOptions, ListMultipartsInfo, ListPartsInfo, MakeBucketOptions, MultipartInfo, MultipartUploadResult, PartInfo, diff --git a/crates/ecstore/src/store_api.rs b/crates/ecstore/src/store_api.rs index b69a31017..dc1c3535d 100644 --- a/crates/ecstore/src/store_api.rs +++ b/crates/ecstore/src/store_api.rs @@ -34,6 +34,7 @@ use rustfs_filemeta::{ use rustfs_lock::NamespaceLockWrapper; use rustfs_madmin::heal_commands::HealResultItem; use rustfs_rio::Checksum; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_utils::CompressionAlgorithm; use rustfs_utils::http::headers::AMZ_OBJECT_TAGGING; use rustfs_utils::http::{AMZ_BUCKET_REPLICATION_STATUS, AMZ_RESTORE, AMZ_STORAGE_CLASS}; diff --git a/crates/ecstore/src/store_api/readers.rs b/crates/ecstore/src/store_api/readers.rs index ed83ac00e..831f2b19d 100644 --- a/crates/ecstore/src/store_api/readers.rs +++ b/crates/ecstore/src/store_api/readers.rs @@ -99,6 +99,10 @@ fn part_plaintext_size(part: &ObjectPartInfo) -> i64 { } } +fn http_range_spec_from_object_info(oi: &ObjectInfo, part_number: usize) -> Option { + HTTPRangeSpec::from_part_sizes(oi.size, part_number, oi.parts.iter().map(part_plaintext_size)) +} + fn restore_request_active(opts: &ObjectOptions) -> bool { let restore = &opts.transition.restore_request; restore.type_.is_some() || restore.days.is_some() || restore.output_location.is_some() || restore.select_parameters.is_some() @@ -301,7 +305,7 @@ impl ReadPlan { if let Some(part_number) = opts.part_number && rs.is_none() { - rs = HTTPRangeSpec::from_object_info(oi, part_number); + rs = http_range_spec_from_object_info(oi, part_number); } let mut is_encrypted = oi.is_encrypted(); @@ -719,105 +723,6 @@ impl AsyncRead for SkipReader { } } -#[derive(Debug, Clone)] -pub struct HTTPRangeSpec { - pub is_suffix_length: bool, - pub start: i64, - pub end: i64, -} - -impl HTTPRangeSpec { - pub fn from_object_info(oi: &ObjectInfo, part_number: usize) -> Option { - if oi.size == 0 || oi.parts.is_empty() { - return None; - } - - if part_number == 0 || part_number > oi.parts.len() { - return None; - } - - let mut start = 0_i64; - let mut end = -1_i64; - for i in 0..part_number { - let part = &oi.parts[i]; - start = end + 1; - end = start + part_plaintext_size(part) - 1; - } - - Some(HTTPRangeSpec { - is_suffix_length: false, - start, - end, - }) - } - - pub fn get_offset_length(&self, res_size: i64) -> Result<(usize, i64)> { - let len = self.get_length(res_size)?; - - let mut start = self.start; - if self.is_suffix_length { - let suffix_len = if self.start < 0 { - self.start - .checked_neg() - .ok_or_else(|| Error::InvalidRangeSpec("range value invalid: suffix length overflow".to_string()))? - } else { - self.start - }; - start = res_size - suffix_len; - if start < 0 { - start = 0; - } - } - Ok((start as usize, len)) - } - pub fn get_length(&self, res_size: i64) -> Result { - if res_size < 0 { - return Err(Error::InvalidRangeSpec("The requested range is not satisfiable".to_string())); - } - - if self.is_suffix_length { - let specified_len = if self.start < 0 { - self.start - .checked_neg() - .ok_or_else(|| Error::InvalidRangeSpec("range value invalid: suffix length overflow".to_string()))? - } else { - self.start - }; - let mut range_length = specified_len; - - if specified_len > res_size { - range_length = res_size; - } - - return Ok(range_length); - } - - if self.start >= res_size { - return Err(Error::InvalidRangeSpec("The requested range is not satisfiable".to_string())); - } - - if self.end > -1 { - let mut end = self.end; - if res_size <= end { - end = res_size - 1; - } - - let range_length = end - self.start + 1; - return Ok(range_length); - } - - if self.end == -1 { - let range_length = res_size - self.start; - return Ok(range_length); - } - - Err(Error::InvalidRangeSpec(format!( - "range value invalid: start={}, end={}, expected start <= end and end >= -1", - self.start, self.end - ))) - } -} - /// A streaming decompression reader that supports range requests by skipping data in the decompressed stream. /// This implementation acknowledges that compressed streams (like LZ4) must be decompressed sequentially /// from the beginning, so it streams and discards data until reaching the target offset. @@ -1818,12 +1723,12 @@ mod tests { ..Default::default() }; - let spec = HTTPRangeSpec::from_object_info(&object_info, 2).unwrap(); + let spec = http_range_spec_from_object_info(&object_info, 2).unwrap(); assert_eq!(spec.start, 100); assert_eq!(spec.end, 199); - assert!(HTTPRangeSpec::from_object_info(&object_info, 0).is_none()); - assert!(HTTPRangeSpec::from_object_info(&object_info, 4).is_none()); + assert!(http_range_spec_from_object_info(&object_info, 0).is_none()); + assert!(http_range_spec_from_object_info(&object_info, 4).is_none()); } #[test] @@ -1856,7 +1761,7 @@ mod tests { ..Default::default() }; - let spec = HTTPRangeSpec::from_object_info(&object_info, 2).unwrap(); + let spec = http_range_spec_from_object_info(&object_info, 2).unwrap(); assert_eq!(spec.start, 30); assert_eq!(spec.end, 69); } @@ -1891,7 +1796,7 @@ mod tests { ..Default::default() }; - let spec = HTTPRangeSpec::from_object_info(&object_info, 3).unwrap(); + let spec = http_range_spec_from_object_info(&object_info, 3).unwrap(); assert_eq!(spec.start, 60); assert_eq!(spec.end, 99); } diff --git a/crates/protocols/src/swift/dlo.rs b/crates/protocols/src/swift/dlo.rs index e359b19a9..410b16063 100644 --- a/crates/protocols/src/swift/dlo.rs +++ b/crates/protocols/src/swift/dlo.rs @@ -300,7 +300,7 @@ async fn create_dlo_stream( async move { let range_spec = if byte_start > 0 || byte_end < segment.size as u64 - 1 { - Some(rustfs_ecstore::store_api::HTTPRangeSpec { + Some(rustfs_storage_api::HTTPRangeSpec { is_suffix_length: false, start: byte_start as i64, end: byte_end as i64, diff --git a/crates/protocols/src/swift/object.rs b/crates/protocols/src/swift/object.rs index caaf2e5ff..1ba678fdd 100644 --- a/crates/protocols/src/swift/object.rs +++ b/crates/protocols/src/swift/object.rs @@ -533,7 +533,7 @@ pub async fn get_object( container: &str, object: &str, credentials: &Credentials, - range: Option, + range: Option, ) -> SwiftResult { use rustfs_ecstore::store_api::GetObjectReader; @@ -1032,8 +1032,8 @@ pub fn parse_copy_from_header(copy_from: &str) -> SwiftResult<(String, String)> /// assert_eq!(range.end, 1023); /// ``` #[allow(dead_code)] // Handler integration: Range header -pub fn parse_range_header(range_str: &str) -> SwiftResult { - use rustfs_ecstore::store_api::HTTPRangeSpec; +pub fn parse_range_header(range_str: &str) -> SwiftResult { + use rustfs_storage_api::HTTPRangeSpec; if !range_str.starts_with("bytes=") { return Err(SwiftError::BadRequest("Range header must start with 'bytes='".to_string())); diff --git a/crates/protocols/src/swift/slo.rs b/crates/protocols/src/swift/slo.rs index cbd42c20b..33c2e4dc4 100644 --- a/crates/protocols/src/swift/slo.rs +++ b/crates/protocols/src/swift/slo.rs @@ -420,7 +420,7 @@ async fn create_slo_stream( // Fetch segment with range let range_spec = if byte_start > 0 || byte_end < segment.size_bytes - 1 { - Some(rustfs_ecstore::store_api::HTTPRangeSpec { + Some(rustfs_storage_api::HTTPRangeSpec { is_suffix_length: false, start: byte_start as i64, end: byte_end as i64, diff --git a/crates/s3select-api/Cargo.toml b/crates/s3select-api/Cargo.toml index 9890975b1..13420b7e0 100644 --- a/crates/s3select-api/Cargo.toml +++ b/crates/s3select-api/Cargo.toml @@ -33,6 +33,7 @@ chrono.workspace = true rustfs-common.workspace = true datafusion = { workspace = true } rustfs-ecstore.workspace = true +rustfs-storage-api.workspace = true futures = { workspace = true } futures-core = { workspace = true } http.workspace = true diff --git a/crates/s3select-api/src/object_store.rs b/crates/s3select-api/src/object_store.rs index 79a7a3fdb..1308e1b1e 100644 --- a/crates/s3select-api/src/object_store.rs +++ b/crates/s3select-api/src/object_store.rs @@ -29,7 +29,8 @@ use rustfs_ecstore::error::{StorageError, is_err_bucket_not_found, is_err_object use rustfs_ecstore::resolve_object_store_handle; use rustfs_ecstore::set_disk::DEFAULT_READ_BUFFER_SIZE; use rustfs_ecstore::store::ECStore; -use rustfs_ecstore::store_api::{GetObjectReader, HTTPRangeSpec, ObjectIO, ObjectOperations, ObjectOptions}; +use rustfs_ecstore::store_api::{GetObjectReader, ObjectIO, ObjectOperations, ObjectOptions}; +use rustfs_storage_api::HTTPRangeSpec; use s3s::S3Result; use s3s::dto::SelectObjectContentInput; use s3s::header::{ diff --git a/crates/scanner/src/scanner.rs b/crates/scanner/src/scanner.rs index 961537397..4a6e36bc0 100644 --- a/crates/scanner/src/scanner.rs +++ b/crates/scanner/src/scanner.rs @@ -1124,7 +1124,7 @@ mod tests { &self, bucket: &str, object: &str, - _range: Option, + _range: Option, _h: http::HeaderMap, _opts: &ObjectOptions, ) -> rustfs_ecstore::error::Result { diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index bbbd5af24..923027368 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -24,4 +24,4 @@ pub use admin::{DiskSetSelector, StorageAdminApi}; pub use bucket::{BucketInfo, BucketOperations, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp}; pub use error::{StorageErrorCode, StorageResult}; pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo}; -pub use object::{HTTPPreconditions, ObjectLockRetentionOptions}; +pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions}; diff --git a/crates/storage-api/src/object.rs b/crates/storage-api/src/object.rs index 2c7141a02..f053ed914 100644 --- a/crates/storage-api/src/object.rs +++ b/crates/storage-api/src/object.rs @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::fmt; use time::OffsetDateTime; #[derive(Debug, Default, Clone)] @@ -39,6 +40,120 @@ pub struct ObjectLockRetentionOptions { pub bypass_governance: bool, } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum HTTPRangeError { + InvalidRangeSpec(String), +} + +impl fmt::Display for HTTPRangeError { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::InvalidRangeSpec(message) => f.write_str(message), + } + } +} + +impl std::error::Error for HTTPRangeError {} + +#[derive(Debug, Clone)] +pub struct HTTPRangeSpec { + pub is_suffix_length: bool, + pub start: i64, + pub end: i64, +} + +impl HTTPRangeSpec { + pub fn from_part_sizes(object_size: i64, part_number: usize, part_sizes: impl IntoIterator) -> Option { + if object_size == 0 || part_number == 0 { + return None; + } + + let mut start = 0_i64; + let mut end = -1_i64; + let mut parts = part_sizes.into_iter(); + for _ in 0..part_number { + let part_size = parts.next()?; + start = end.checked_add(1)?; + end = start.checked_add(part_size)?.checked_sub(1)?; + } + + Some(Self { + is_suffix_length: false, + start, + end, + }) + } + + pub fn get_offset_length(&self, res_size: i64) -> Result<(usize, i64), HTTPRangeError> { + let len = self.get_length(res_size)?; + + let mut start = self.start; + if self.is_suffix_length { + let suffix_len = if self.start < 0 { + self.start + .checked_neg() + .ok_or_else(|| HTTPRangeError::InvalidRangeSpec("range value invalid: suffix length overflow".to_string()))? + } else { + self.start + }; + start = res_size - suffix_len; + if start < 0 { + start = 0; + } + } + let offset = usize::try_from(start) + .map_err(|_| HTTPRangeError::InvalidRangeSpec("range value invalid: start offset overflow".to_string()))?; + Ok((offset, len)) + } + + pub fn get_length(&self, res_size: i64) -> Result { + if res_size < 0 { + return Err(HTTPRangeError::InvalidRangeSpec("The requested range is not satisfiable".to_string())); + } + + if self.is_suffix_length { + let specified_len = if self.start < 0 { + self.start + .checked_neg() + .ok_or_else(|| HTTPRangeError::InvalidRangeSpec("range value invalid: suffix length overflow".to_string()))? + } else { + self.start + }; + let mut range_length = specified_len; + + if specified_len > res_size { + range_length = res_size; + } + + return Ok(range_length); + } + + if self.start >= res_size { + return Err(HTTPRangeError::InvalidRangeSpec("The requested range is not satisfiable".to_string())); + } + + if self.end > -1 { + let mut end = self.end; + if res_size <= end { + end = res_size - 1; + } + + let range_length = end - self.start + 1; + return Ok(range_length); + } + + if self.end == -1 { + let range_length = res_size - self.start; + return Ok(range_length); + } + + Err(HTTPRangeError::InvalidRangeSpec(format!( + "range value invalid: start={}, end={}, expected start <= end and end >= -1", + self.start, self.end + ))) + } +} + fn non_empty_condition_value(value: Option<&str>) -> Option<&str> { value.map(str::trim).filter(|value| !value.is_empty()) } @@ -67,4 +182,33 @@ mod tests { assert!(opts.retain_until.is_none()); assert!(!opts.bypass_governance); } + + #[test] + fn http_range_spec_offset_length_handles_suffix_and_bounds() { + let range = HTTPRangeSpec { + is_suffix_length: false, + start: 5, + end: 10, + }; + + assert_eq!(range.get_offset_length(20).expect("range should fit"), (5, 6)); + + let suffix = HTTPRangeSpec { + is_suffix_length: true, + start: -5, + end: -1, + }; + + assert_eq!(suffix.get_offset_length(20).expect("suffix range should fit"), (15, 5)); + } + + #[test] + fn http_range_spec_from_part_sizes_keeps_part_boundaries() { + let spec = HTTPRangeSpec::from_part_sizes(100, 3, [10, 15, 20]).expect("third part should exist"); + + assert_eq!(spec.start, 25); + assert_eq!(spec.end, 44); + assert!(HTTPRangeSpec::from_part_sizes(100, 0, [10]).is_none()); + assert!(HTTPRangeSpec::from_part_sizes(100, 2, [10]).is_none()); + } } diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index c63717bc3..91517831d 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -94,7 +94,7 @@ Required `rustfs-storage-api` public re-exports: - `pub use bucket::{BucketInfo, BucketOperations, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp};` - `pub use error::{StorageErrorCode, StorageResult};` - `pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo};` -- `pub use object::{HTTPPreconditions, ObjectLockRetentionOptions};` +- `pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions};` ECStore must keep compile-time coverage for both `StorageAdminApi` and the separate `NamespaceLocking` operation group. diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 64a8b819c..6e596841d 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,17 +5,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Current Context - Issue: [`rustfs/backlog#660`](https://github.com/rustfs/backlog/issues/660) -- Branch: `overtrue/arch-object-option-contracts` -- Baseline: `origin/main` at `8d24d9133b31c0ca0de0fcd16ad06cd2fdc6f7ae` +- Branch: `overtrue/arch-range-contracts` +- Baseline: `overtrue/arch-object-option-contracts` at `5328e10a0f0cdda9cc17089956e17432f2875962` + over `origin/main` at `a9aba323c6512e6b99c3137258b97b6058075ce9` - PR type for this branch: `api-extraction` - Runtime behavior changes: no external behavior change expected. -- Rust code changes: move the pure `CompletePart`, `HTTPPreconditions`, and - `ObjectLockRetentionOptions` helper contracts from ECStore `store_api` into - `rustfs-storage-api`, then migrate in-repo consumers to the shared contract - path. -- CI/script changes: extend migration guards for multipart/object helper public - re-exports and reject restoring the old ECStore-owned helper definitions. -- Docs changes: record the object helper contract extraction slice. +- Rust code changes: move the pure `HTTPRangeSpec` range contract and + `HTTPRangeError` into `rustfs-storage-api`, then keep ECStore object-info + adaptation at the ECStore boundary. +- CI/script changes: extend migration guards for range helper public re-exports + and reject restoring the old ECStore-owned range helper definitions. +- Docs changes: record the range contract extraction slice. ## Phase 0 Tasks @@ -557,6 +557,26 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block migration/layer guards, formatting, diff hygiene, Rust risk scan, and required three-expert review passed. +- [x] `API-016` Move HTTP range helper contracts. + - Completed slice: move `HTTPRangeSpec` and `HTTPRangeError` from ECStore + `store_api/readers.rs` into `rustfs-storage-api`; keep `ObjectInfo` part + adaptation in ECStore and migrate RustFS, ECStore, Swift, scanner, and + S3-select consumers to import the shared range contract directly. + - Acceptance: `rustfs-storage-api` exports the range helper contracts, + in-repo consumers no longer use the old `rustfs_ecstore::store_api` path + for `HTTPRangeSpec`, and migration guards reject restoring old ECStore + definitions or public re-exports. + - Must preserve: S3 range semantics, suffix ranges, multipart part-range + boundaries, SSE/rio/compressed range planning, Swift/S3-select reads, and + ECStore-owned object-info/filemeta adaptation. + - Risk defense: only pure range contract behavior crosses into + `rustfs-storage-api`; ECStore keeps readers, `ObjectInfo`, part plaintext + size selection, encryption/compression planning, lifecycle/replication/rio + coupling, and storage implementation bodies. + - Verification: focused storage-api/ECStore/RustFS/downstream compile checks, + migration/layer guards, formatting, diff hygiene, Rust risk scan, and + required three-expert review passed. + ## Phase 8 Background Controller Tasks - [x] `BGC-001` Inventory background services. diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index 6385738f7..4d0f2fcc5 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -53,13 +53,13 @@ use rustfs_ecstore::rio::{DecryptReader, EncryptReader, HardLimitReader, boxed_r use rustfs_ecstore::rio::{HashReader, WritePlan}; use rustfs_ecstore::set_disk::is_valid_storage_class; use rustfs_ecstore::store::ECStore; -use rustfs_ecstore::store_api::{HTTPRangeSpec, ObjectIO, ObjectOptions, PutObjReader}; use rustfs_ecstore::store_api::{MultipartOperations, ObjectOperations}; +use rustfs_ecstore::store_api::{ObjectIO, ObjectOptions, PutObjReader}; use rustfs_filemeta::{ReplicationStatusType, ReplicationType}; use rustfs_s3_ops::S3Operation; #[cfg(test)] use rustfs_storage_api::HTTPPreconditions; -use rustfs_storage_api::{CompletePart, MultipartUploadResult}; +use rustfs_storage_api::{CompletePart, HTTPRangeSpec, MultipartUploadResult}; use rustfs_targets::EventName; use rustfs_utils::CompressionAlgorithm; use rustfs_utils::http::{ diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 7520ed325..0d31748f1 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -79,7 +79,7 @@ use rustfs_ecstore::rio::{DynReader, HashReader, WritePlan, wrap_reader}; use rustfs_ecstore::set_disk::{get_lock_acquire_timeout, is_valid_storage_class}; use rustfs_ecstore::store::ECStore; use rustfs_ecstore::store_api::{ - HTTPRangeSpec, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, PutObjReader, + NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, PutObjReader, }; use rustfs_filemeta::{ REPLICATE_INCOMING_DELETE, ReplicateDecision, ReplicateTargetDecision, ReplicationState, ReplicationStatusType, @@ -95,6 +95,7 @@ use rustfs_s3_ops::{S3Operation, delete_event_name_for_marker, put_event_name_fo use rustfs_s3select_api::object_store::bytes_stream; #[cfg(test)] use rustfs_storage_api::HTTPPreconditions; +use rustfs_storage_api::HTTPRangeSpec; use rustfs_targets::{ EventName, extract_params_header, extract_resp_elements, get_request_host, get_request_port, get_request_user_agent, }; @@ -1585,7 +1586,17 @@ impl DefaultObjectUsecase { if let Some(part_number) = part_number && rs.is_none() { - rs = HTTPRangeSpec::from_object_info(&info, part_number); + rs = HTTPRangeSpec::from_part_sizes( + info.size, + part_number, + info.parts.iter().map(|part| { + if part.actual_size > 0 { + part.actual_size + } else { + i64::try_from(part.size).unwrap_or(i64::MAX) + } + }), + ); } validate_sse_headers_for_read(&info.user_defined, &req.headers)?; diff --git a/rustfs/src/error.rs b/rustfs/src/error.rs index 39ed78ae5..07f074a8b 100644 --- a/rustfs/src/error.rs +++ b/rustfs/src/error.rs @@ -14,6 +14,7 @@ use rustfs_ecstore::bucket::quota::QuotaError; use rustfs_ecstore::error::StorageError; +use rustfs_storage_api::HTTPRangeError; use s3s::{S3Error, S3ErrorCode}; #[derive(Debug)] @@ -282,6 +283,16 @@ impl From for ApiError { } } +impl From for ApiError { + fn from(err: HTTPRangeError) -> Self { + ApiError { + code: S3ErrorCode::InvalidRange, + message: err.to_string(), + source: Some(Box::new(err)), + } + } +} + impl From for ApiError { fn from(err: std::io::Error) -> Self { // Check if the error is a ChecksumMismatch (BadDigest) diff --git a/rustfs/src/storage/options.rs b/rustfs/src/storage/options.rs index 0203023af..eaecd0601 100644 --- a/rustfs/src/storage/options.rs +++ b/rustfs/src/storage/options.rs @@ -31,9 +31,9 @@ use s3s::header::X_AMZ_OBJECT_LOCK_RETAIN_UNTIL_DATE; use crate::auth::UNSIGNED_PAYLOAD; use crate::auth::UNSIGNED_PAYLOAD_TRAILER; -use rustfs_ecstore::store_api::{HTTPRangeSpec, ObjectOptions}; +use rustfs_ecstore::store_api::ObjectOptions; use rustfs_policy::service_type::ServiceType; -use rustfs_storage_api::HTTPPreconditions; +use rustfs_storage_api::{HTTPPreconditions, HTTPRangeSpec}; use rustfs_utils::hash::EMPTY_STRING_SHA256_HASH; use rustfs_utils::http::AMZ_CONTENT_SHA256; use rustfs_utils::path::is_dir_object; diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index 6e51a3d8f..cd869e5ac 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -54,6 +54,7 @@ STORE_API_BUCKET_DTO_REEXPORTS_FILE="${TMP_DIR}/store_api_bucket_dto_reexports.t STORE_API_BUCKET_OPERATION_HITS_FILE="${TMP_DIR}/store_api_bucket_operation_hits.txt" STORE_API_MULTIPART_DTO_REEXPORTS_FILE="${TMP_DIR}/store_api_multipart_dto_reexports.txt" STORE_API_OBJECT_HELPER_REEXPORTS_FILE="${TMP_DIR}/store_api_object_helper_reexports.txt" +STORE_API_RANGE_HELPER_REEXPORTS_FILE="${TMP_DIR}/store_api_range_helper_reexports.txt" awk ' /^## PR Types$/ { @@ -192,7 +193,7 @@ require_source_line \ "storage-api public multipart DTO re-export" require_source_line \ "crates/storage-api/src/lib.rs" \ - "pub use object::{HTTPPreconditions, ObjectLockRetentionOptions};" \ + "pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions};" \ "storage-api public object helper contract re-export" require_source_line \ "crates/storage-api/src/lib.rs" \ @@ -240,7 +241,7 @@ fi ( cd "$ROOT_DIR" - rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api::\{[^}]*\b(?:HTTPPreconditions|ObjectLockRetentionOptions)\b|pub struct (?:HTTPPreconditions|ObjectLockRetentionOptions)\b' \ + rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:HTTPPreconditions|ObjectLockRetentionOptions)\b|::(?:HTTPPreconditions|ObjectLockRetentionOptions)\b)|pub struct (?:HTTPPreconditions|ObjectLockRetentionOptions)\b' \ crates/ecstore/src/store_api.rs crates/ecstore/src/store_api/types.rs || true ) >"$STORE_API_OBJECT_HELPER_REEXPORTS_FILE" @@ -248,6 +249,16 @@ if [[ -s "$STORE_API_OBJECT_HELPER_REEXPORTS_FILE" ]]; then report_failure "old ecstore store_api object helper path reintroduced: $(paste -sd '; ' "$STORE_API_OBJECT_HELPER_REEXPORTS_FILE")" fi +( + cd "$ROOT_DIR" + rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:HTTPRangeError|HTTPRangeSpec)\b|::(?:HTTPRangeError|HTTPRangeSpec)\b)|pub (?:enum HTTPRangeError|struct HTTPRangeSpec)\b' \ + crates/ecstore/src/store_api.rs crates/ecstore/src/store_api/types.rs crates/ecstore/src/store_api/readers.rs || true +) >"$STORE_API_RANGE_HELPER_REEXPORTS_FILE" + +if [[ -s "$STORE_API_RANGE_HELPER_REEXPORTS_FILE" ]]; then + report_failure "old ecstore store_api range helper path reintroduced: $(paste -sd '; ' "$STORE_API_RANGE_HELPER_REEXPORTS_FILE")" +fi + require_source_contains \ "crates/ecstore/src/store_api/traits.rs" \ "pub trait NamespaceLocking: Send + Sync + Debug + 'static" \