From 24fa03e04b84ab23759e89a865b8effd6293bcf1 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 07:19:07 +0800 Subject: [PATCH] refactor: move object list response contracts (#3548) --- crates/ecstore/src/store_api/types.rs | 64 ++------------- crates/storage-api/src/lib.rs | 1 + crates/storage-api/src/object.rs | 66 ++++++++++++++++ docs/architecture/crate-boundaries.md | 1 + docs/architecture/migration-progress.md | 77 ++++++++++++------- scripts/check_architecture_migration_rules.sh | 15 ++++ 6 files changed, 138 insertions(+), 86 deletions(-) diff --git a/crates/ecstore/src/store_api/types.rs b/crates/ecstore/src/store_api/types.rs index a54bcd461..e53e638ea 100644 --- a/crates/ecstore/src/store_api/types.rs +++ b/crates/ecstore/src/store_api/types.rs @@ -4,6 +4,11 @@ use rustfs_storage_api::{ VersionMarker, WalkVersionsSortOrder, }; +pub type ListObjectsInfo = rustfs_storage_api::ListObjectsInfo; +pub type ListObjectsV2Info = rustfs_storage_api::ListObjectsV2Info; +pub type ListObjectVersionsInfo = rustfs_storage_api::ListObjectVersionsInfo; +pub type ObjectInfoOrErr = rustfs_storage_api::ObjectInfoOrErr; + #[derive(Debug, Default, Clone)] pub struct ObjectOptions { // Use the maximum parity (N/2), used when saving server configuration files @@ -708,50 +713,6 @@ fn versions_after_marker(file_infos: &rustfs_filemeta::FileInfoVersions, marker: .unwrap_or(&file_infos.versions) } -#[derive(Debug, Default)] -pub struct ListObjectsInfo { - // Indicates whether the returned list objects response is truncated. A - // value of true indicates that the list was truncated. The list can be truncated - // if the number of objects exceeds the limit allowed or specified - // by max keys. - pub is_truncated: bool, - - // When response is truncated (the IsTruncated element value in the response - // is true), you can use the key name in this field as marker in the subsequent - // request to get next set of objects. - pub next_marker: Option, - - // List of objects info for this request. - pub objects: Vec, - - // List of prefixes for this request. - pub prefixes: Vec, -} - -#[derive(Debug, Default)] -pub struct ListObjectsV2Info { - // Indicates whether the returned list objects response is truncated. A - // value of true indicates that the list was truncated. The list can be truncated - // if the number of objects exceeds the limit allowed or specified - // by max keys. - pub is_truncated: bool, - - // When response is truncated (the IsTruncated element value in the response - // is true), you can use the key name in this field as marker in the subsequent - // request to get next set of objects. - // - // NOTE: This element is returned only if you have delimiter request parameter - // specified. - pub continuation_token: Option, - pub next_continuation_token: Option, - - // List of objects info for this request. - pub objects: Vec, - - // List of prefixes for this request. - pub prefixes: Vec, -} - #[derive(Debug, Default, Clone)] pub struct ObjectToDelete { pub object_name: String, @@ -805,15 +766,6 @@ impl DeletedObject { } } -#[derive(Debug, Default, Clone)] -pub struct ListObjectVersionsInfo { - pub is_truncated: bool, - pub next_marker: Option, - pub next_version_idmarker: Option, - pub objects: Vec, - pub prefixes: Vec, -} - type WalkFilter = fn(&FileInfo) -> bool; #[derive(Clone, Default)] @@ -827,12 +779,6 @@ pub struct WalkOptions { pub include_free_versions: bool, // include persisted tier free-version cleanup records } -#[derive(Debug)] -pub struct ObjectInfoOrErr { - pub item: Option, - pub err: Option, -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index 2e037065b..4b5bfc3d9 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -25,5 +25,6 @@ pub use bucket::{BucketInfo, BucketOperations, BucketOptions, DeleteBucketOption pub use error::{StorageErrorCode, StorageResult}; pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo}; pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions}; +pub use object::{ListObjectVersionsInfo, ListObjectsInfo, ListObjectsV2Info, ObjectInfoOrErr}; pub use object::{ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState}; pub use object::{VersionMarker, WalkVersionsSortOrder}; diff --git a/crates/storage-api/src/object.rs b/crates/storage-api/src/object.rs index 7b41e9a1c..6861eff88 100644 --- a/crates/storage-api/src/object.rs +++ b/crates/storage-api/src/object.rs @@ -154,6 +154,38 @@ pub enum WalkVersionsSortOrder { Descending, } +#[derive(Debug, Default)] +pub struct ListObjectsInfo { + pub is_truncated: bool, + pub next_marker: Option, + pub objects: Vec, + pub prefixes: Vec, +} + +#[derive(Debug, Default)] +pub struct ListObjectsV2Info { + pub is_truncated: bool, + pub continuation_token: Option, + pub next_continuation_token: Option, + pub objects: Vec, + pub prefixes: Vec, +} + +#[derive(Debug, Default, Clone)] +pub struct ListObjectVersionsInfo { + pub is_truncated: bool, + pub next_marker: Option, + pub next_version_idmarker: Option, + pub objects: Vec, + pub prefixes: Vec, +} + +#[derive(Debug)] +pub struct ObjectInfoOrErr { + pub item: Option, + pub err: Option, +} + #[derive(Debug, Clone, PartialEq, Eq)] pub enum HTTPRangeError { InvalidRangeSpec(String), @@ -386,6 +418,40 @@ mod tests { assert!(matches!(WalkVersionsSortOrder::default(), WalkVersionsSortOrder::Ascending)); } + #[test] + fn object_list_response_contracts_default_to_empty_collections() { + let v1 = ListObjectsInfo::<()>::default(); + assert!(!v1.is_truncated); + assert!(v1.next_marker.is_none()); + assert!(v1.objects.is_empty()); + assert!(v1.prefixes.is_empty()); + + let v2 = ListObjectsV2Info::<()>::default(); + assert!(!v2.is_truncated); + assert!(v2.continuation_token.is_none()); + assert!(v2.next_continuation_token.is_none()); + assert!(v2.objects.is_empty()); + assert!(v2.prefixes.is_empty()); + + let versions = ListObjectVersionsInfo::<()>::default(); + assert!(!versions.is_truncated); + assert!(versions.next_marker.is_none()); + assert!(versions.next_version_idmarker.is_none()); + assert!(versions.objects.is_empty()); + assert!(versions.prefixes.is_empty()); + } + + #[test] + fn object_info_or_err_keeps_optional_item_and_error_slots() { + let item = ObjectInfoOrErr:: { + item: Some(7), + err: None, + }; + + assert_eq!(item.item, Some(7)); + assert!(item.err.is_none()); + } + #[test] fn http_range_spec_offset_length_handles_suffix_and_bounds() { let range = HTTPRangeSpec { diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index 6e1dbff66..edf9e9916 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -95,6 +95,7 @@ Required `rustfs-storage-api` public re-exports: - `pub use error::{StorageErrorCode, StorageResult};` - `pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartInfo, MultipartUploadResult, PartInfo};` - `pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions};` +- `pub use object::{ListObjectVersionsInfo, ListObjectsInfo, ListObjectsV2Info, ObjectInfoOrErr};` - `pub use object::{ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState};` - `pub use object::{VersionMarker, WalkVersionsSortOrder};` diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 3598ded28..fb175c57b 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,18 +5,18 @@ 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-precondition-contracts` -- Baseline: `overtrue/arch-object-list-helper-contracts` at `65b56e1fb4fa4c65c167e2f434bf5fcb2a0b0cd4f` - over `overtrue/arch-range-contracts` at `829bc62095f0fd853747cd6d7d77f8558baf24a2` +- Branch: `overtrue/arch-list-response-contracts` +- Baseline: `main` at `80ed63484bf62b916ceda17fd76b1092261fda77` + after the object precondition contract merge. - PR type for this branch: `api-extraction` - Runtime behavior changes: no external behavior change expected. -- Rust code changes: move pure object HTTP precondition evaluation into - `rustfs-storage-api` as `ObjectPreconditionState`, - `ObjectPreconditionPart`, and `ObjectPreconditionError`, then keep - `ObjectOptions`, `ObjectInfo`, and ECStore error mapping in ECStore. -- CI/script changes: extend migration guards for object precondition contract - public re-exports. -- Docs changes: record the object precondition contract extraction slice. +- Rust code changes: move object list response DTO contracts into + `rustfs-storage-api` as generic `ListObjectsInfo`, + `ListObjectsV2Info`, `ListObjectVersionsInfo`, and `ObjectInfoOrErr`, + then keep ECStore's existing public names as `ObjectInfo`/`Error` aliases. +- CI/script changes: extend migration guards for list response contract public + re-exports and ECStore local-definition regressions. +- Docs changes: record the object list response contract extraction slice. ## Phase 0 Tasks @@ -619,6 +619,29 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and required three-expert review passed. +- [x] `API-019` Move object list response contracts. + - Completed slice: move `ListObjectsInfo`, `ListObjectsV2Info`, + `ListObjectVersionsInfo`, and `ObjectInfoOrErr` from ECStore + `store_api/types.rs` into `rustfs-storage-api` as generic public + contracts, then keep ECStore's old public names as type aliases bound to + `ObjectInfo` and `Error`. + - Acceptance: `rustfs-storage-api` exports the generic list response + contracts, ECStore no longer defines local response structs for these + contracts, existing ECStore consumers keep their old import path, and + migration guards reject dropping the public storage-api re-export or + reintroducing local ECStore definitions. + - Must preserve: list v1/v2 truncation and marker fields, list-object-version + marker fields, object/prefix vectors, walk item/error channel shape, and + ECStore list/walk runtime behavior. + - Risk defense: only generic response containers cross into + `rustfs-storage-api`; ECStore keeps `ObjectInfo`, `ObjectOptions`, + `WalkOptions`, filemeta filters, object metadata adaptation, storage errors, + readers, lifecycle/replication coupling, and list/walk implementation + bodies. + - Verification: focused storage-api tests, ECStore/RustFS/downstream compile + checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, + full pre-commit, and required three-expert review passed. + ## Phase 8 Background Controller Tasks - [x] `BGC-001` Inventory background services. @@ -895,16 +918,15 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | passed | Pure precondition decision state and result contracts now live in `rustfs-storage-api`; ECStore still owns object metadata adaptation and storage error mapping. | -| Migration preservation | passed | Requested-part validation, ETag/date precondition priority, empty condition handling, and existing ECStore `Error` mapping are preserved. | -| Testing/verification | passed | Focused storage-api/ECStore tests, compile checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and full `make pre-commit` passed. | +| Quality/architecture | passed | Generic list response containers now live in `rustfs-storage-api`; ECStore still owns `ObjectInfo`, `Error`, walk options, and list implementation behavior. | +| Migration preservation | passed | List v1/v2 fields, version-list fields, walk item/error channel shape, and existing ECStore public import paths are preserved through aliases. | +| Testing/verification | passed | Focused storage-api tests, downstream compile checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and full `make pre-commit` passed. | ## Verification Notes Passed before push: - `cargo test -p rustfs-storage-api`: passed. -- `cargo test -p rustfs-ecstore precondition_check_ignores_empty_etag_conditions`: passed. - `cargo check --tests -p rustfs-storage-api -p rustfs-ecstore -p rustfs -p rustfs-iam -p rustfs-scanner`: passed. - `./scripts/check_architecture_migration_rules.sh`: passed. - `./scripts/check_layer_dependencies.sh`: passed. @@ -913,25 +935,26 @@ Passed before push: - Rust risk scan: no new `unwrap`/`expect`, panic/todo markers, `unsafe`, process-spawning calls, lossy casts, println/eprintln, or relaxed ordering in added Rust lines. -- `make pre-commit`: passed; nextest reported 6164 tests passed and 111 +- `make pre-commit`: passed; nextest reported 6191 tests passed and 111 skipped, and doctests passed. Notes: -- This slice follows the range helper contract branch and keeps the old +- This slice follows the object precondition contract branch and keeps the old aggregate facade, bucket DTO, multipart DTO, bucket operation contract, object - helper, range helper, list helper, and object precondition contract guards - active. -- The shared precondition helper contract is now owned by - `rustfs-storage-api`; ECStore keeps `ObjectOptions`, `ObjectInfo`, object - metadata adaptation, storage error mapping, readers, lifecycle/replication, - rio, filemeta, and implementation behavior. -- The slice does not alter object, list, walk, multipart, bucket, or reader - runtime behavior. + helper, range helper, list helper, object precondition, and list response + contract guards active. +- The shared list response containers are now owned by `rustfs-storage-api`; + ECStore keeps `ObjectInfo`, storage `Error`, `ObjectOptions`, `WalkOptions`, + object metadata adaptation, storage error mapping, readers, + lifecycle/replication, rio, filemeta, and implementation behavior. +- The slice does not alter object, list, walk, multipart, bucket, delete, or + reader runtime behavior. ## Handoff Notes -- Object precondition contract cleanup is stacked on the list helper contract - branch. +- Object list response contract cleanup is stacked on the object precondition + contract branch. - After this lands, remaining storage work can continue by extracting larger - low-coupling DTO slices or by narrowing remaining operation-group consumers. + low-coupling DTO/consumer slices or by narrowing remaining operation-group + consumers. diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index 9b32e64a9..c8c697efe 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -56,6 +56,7 @@ STORE_API_MULTIPART_DTO_REEXPORTS_FILE="${TMP_DIR}/store_api_multipart_dto_reexp 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" STORE_API_LIST_HELPER_REEXPORTS_FILE="${TMP_DIR}/store_api_list_helper_reexports.txt" +STORE_API_LIST_RESPONSE_REEXPORTS_FILE="${TMP_DIR}/store_api_list_response_reexports.txt" awk ' /^## PR Types$/ { @@ -196,6 +197,10 @@ require_source_line \ "crates/storage-api/src/lib.rs" \ "pub use object::{HTTPPreconditions, HTTPRangeError, HTTPRangeSpec, ObjectLockRetentionOptions};" \ "storage-api public object helper contract re-export" +require_source_line \ + "crates/storage-api/src/lib.rs" \ + "pub use object::{ListObjectVersionsInfo, ListObjectsInfo, ListObjectsV2Info, ObjectInfoOrErr};" \ + "storage-api public list response contract re-export" require_source_line \ "crates/storage-api/src/lib.rs" \ "pub use object::{ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState};" \ @@ -278,6 +283,16 @@ if [[ -s "$STORE_API_LIST_HELPER_REEXPORTS_FILE" ]]; then report_failure "old ecstore store_api list helper path reintroduced: $(paste -sd '; ' "$STORE_API_LIST_HELPER_REEXPORTS_FILE")" fi +( + cd "$ROOT_DIR" + rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:ListObjectVersionsInfo|ListObjectsInfo|ListObjectsV2Info|ObjectInfoOrErr)\b|::(?:ListObjectVersionsInfo|ListObjectsInfo|ListObjectsV2Info|ObjectInfoOrErr)\b)|pub struct (?:ListObjectVersionsInfo|ListObjectsInfo|ListObjectsV2Info|ObjectInfoOrErr)\b' \ + crates/ecstore/src/store_api.rs crates/ecstore/src/store_api/types.rs || true +) >"$STORE_API_LIST_RESPONSE_REEXPORTS_FILE" + +if [[ -s "$STORE_API_LIST_RESPONSE_REEXPORTS_FILE" ]]; then + report_failure "old ecstore store_api list response path reintroduced: $(paste -sd '; ' "$STORE_API_LIST_RESPONSE_REEXPORTS_FILE")" +fi + require_source_contains \ "crates/ecstore/src/store_api/traits.rs" \ "pub trait NamespaceLocking: Send + Sync + Debug + 'static" \