From 36f7ad6936d03b9593880b5c4958f47b2a039fc9 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 08:18:26 +0800 Subject: [PATCH] refactor: move walk options contract (#3549) --- crates/ecstore/src/store_api/types.rs | 14 +--- crates/storage-api/src/lib.rs | 2 +- crates/storage-api/src/object.rs | 40 ++++++++++++ docs/architecture/crate-boundaries.md | 2 +- docs/architecture/migration-progress.md | 65 ++++++++++++------- scripts/check_architecture_migration_rules.sh | 4 +- 6 files changed, 88 insertions(+), 39 deletions(-) diff --git a/crates/ecstore/src/store_api/types.rs b/crates/ecstore/src/store_api/types.rs index e53e638ea..7421fa91f 100644 --- a/crates/ecstore/src/store_api/types.rs +++ b/crates/ecstore/src/store_api/types.rs @@ -1,13 +1,14 @@ use super::*; use rustfs_storage_api::{ HTTPPreconditions, ObjectLockRetentionOptions, ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState, - VersionMarker, WalkVersionsSortOrder, + VersionMarker, }; 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; +pub type WalkOptions = rustfs_storage_api::WalkOptions; #[derive(Debug, Default, Clone)] pub struct ObjectOptions { @@ -768,17 +769,6 @@ impl DeletedObject { type WalkFilter = fn(&FileInfo) -> bool; -#[derive(Clone, Default)] -pub struct WalkOptions { - pub filter: Option, // return WalkFilter returns 'true/false' - pub marker: Option, // set to skip until this object - pub latest_only: bool, // returns only latest versions for all matching objects - pub ask_disks: String, // dictates how many disks are being listed - pub versions_sort: WalkVersionsSortOrder, // sort order for versions of the same object; default: Ascending order in ModTime - pub limit: usize, // maximum number of items, 0 means no limit - pub include_free_versions: bool, // include persisted tier free-version cleanup records -} - #[cfg(test)] mod tests { use super::*; diff --git a/crates/storage-api/src/lib.rs b/crates/storage-api/src/lib.rs index 4b5bfc3d9..804ca660f 100644 --- a/crates/storage-api/src/lib.rs +++ b/crates/storage-api/src/lib.rs @@ -27,4 +27,4 @@ pub use multipart::{CompletePart, ListMultipartsInfo, ListPartsInfo, MultipartIn 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}; +pub use object::{VersionMarker, WalkOptions, WalkVersionsSortOrder}; diff --git a/crates/storage-api/src/object.rs b/crates/storage-api/src/object.rs index 6861eff88..4519b0142 100644 --- a/crates/storage-api/src/object.rs +++ b/crates/storage-api/src/object.rs @@ -154,6 +154,31 @@ pub enum WalkVersionsSortOrder { Descending, } +#[derive(Clone)] +pub struct WalkOptions { + pub filter: Option, + pub marker: Option, + pub latest_only: bool, + pub ask_disks: String, + pub versions_sort: WalkVersionsSortOrder, + pub limit: usize, + pub include_free_versions: bool, +} + +impl Default for WalkOptions { + fn default() -> Self { + Self { + filter: None, + marker: None, + latest_only: false, + ask_disks: String::new(), + versions_sort: WalkVersionsSortOrder::default(), + limit: 0, + include_free_versions: false, + } + } +} + #[derive(Debug, Default)] pub struct ListObjectsInfo { pub is_truncated: bool, @@ -418,6 +443,21 @@ mod tests { assert!(matches!(WalkVersionsSortOrder::default(), WalkVersionsSortOrder::Ascending)); } + #[test] + fn walk_options_defaults_preserve_existing_contract() { + type TestFilter = fn(&str) -> bool; + + let opts = WalkOptions::::default(); + + assert!(opts.filter.is_none()); + assert!(opts.marker.is_none()); + assert!(!opts.latest_only); + assert!(opts.ask_disks.is_empty()); + assert!(matches!(opts.versions_sort, WalkVersionsSortOrder::Ascending)); + assert_eq!(opts.limit, 0); + assert!(!opts.include_free_versions); + } + #[test] fn object_list_response_contracts_default_to_empty_collections() { let v1 = ListObjectsInfo::<()>::default(); diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index edf9e9916..44718d5ea 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -97,7 +97,7 @@ Required `rustfs-storage-api` public re-exports: - `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};` +- `pub use object::{VersionMarker, WalkOptions, WalkVersionsSortOrder};` 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 fb175c57b..39c9b4ba6 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,18 +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-list-response-contracts` -- Baseline: `main` at `80ed63484bf62b916ceda17fd76b1092261fda77` - after the object precondition contract merge. +- Branch: `overtrue/arch-walk-options-contracts` +- Baseline: `main` at `604379d62fab481b73d1848bfbd045a684031441` + after the table catalog row-level conflict hardening merge. - PR type for this branch: `api-extraction` - Runtime behavior changes: no external behavior change expected. -- 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. +- Rust code changes: move `WalkOptions` into `rustfs-storage-api` as a generic + filter-container contract, then keep ECStore's existing public name as a + `FileInfo` filter alias. +- CI/script changes: extend migration guards for the `WalkOptions` public + re-export and ECStore local-definition regressions. +- Docs changes: record the walk options contract extraction slice. ## Phase 0 Tasks @@ -642,6 +641,26 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, full pre-commit, and required three-expert review passed. +- [x] `API-020` Move walk options contract. + - Completed slice: move `WalkOptions` from ECStore `store_api/types.rs` into + `rustfs-storage-api` as a generic public contract over the filter type, + then keep ECStore's old public `WalkOptions` name as a type alias bound to + the existing `fn(&FileInfo) -> bool` filter shape. + - Acceptance: `rustfs-storage-api` exports `WalkOptions`, ECStore no longer + defines a local `WalkOptions` struct, existing ECStore consumers keep their + old import path, and migration guards reject dropping the public + storage-api re-export or reintroducing a local ECStore definition. + - Must preserve: walk filter optionality, marker, latest-only flag, ask-disks + string, version sort default, limit semantics, include-free-versions flag, + and ECStore list/walk runtime behavior. + - Risk defense: only the generic options container crosses into + `rustfs-storage-api`; ECStore keeps the concrete `FileInfo` filter binding, + list/walk implementations, metadata conversion, readers, storage errors, + lifecycle/replication coupling, and operation traits. + - 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. @@ -918,8 +937,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| 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. | +| Quality/architecture | passed | Generic `WalkOptions` now lives in `rustfs-storage-api`; ECStore still owns the concrete `FileInfo` filter binding and list/walk implementation behavior. | +| Migration preservation | passed | Filter optionality, marker/default fields, version sort default, include-free-versions flag, and existing ECStore public import path are preserved through an alias. | | 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 @@ -927,7 +946,7 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block Passed before push: - `cargo test -p rustfs-storage-api`: passed. -- `cargo check --tests -p rustfs-storage-api -p rustfs-ecstore -p rustfs -p rustfs-iam -p rustfs-scanner`: passed. +- `cargo check --tests -p rustfs-storage-api -p rustfs-ecstore -p rustfs -p rustfs-iam -p rustfs-scanner -p rustfs-protocols`: passed. - `./scripts/check_architecture_migration_rules.sh`: passed. - `./scripts/check_layer_dependencies.sh`: passed. - `cargo fmt --all --check`: passed. @@ -935,26 +954,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 6191 tests passed and 111 +- `make pre-commit`: passed; nextest reported 6203 tests passed and 111 skipped, and doctests passed. Notes: -- This slice follows the object precondition contract branch and keeps the old +- This slice follows the object list response contract branch and keeps the old aggregate facade, bucket DTO, multipart DTO, bucket operation contract, object - 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. + helper, range helper, list helper, object precondition, list response, and + walk options contract guards active. +- The shared walk options container is now owned by `rustfs-storage-api`; + ECStore keeps the concrete `FileInfo` filter alias, `ObjectInfo`, storage + `Error`, `ObjectOptions`, 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 list response contract cleanup is stacked on the object precondition - contract branch. +- Walk options contract cleanup is stacked on the object list response contract + branch. - After this lands, remaining storage work can continue by extracting larger 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 c8c697efe..002a4fefa 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -207,7 +207,7 @@ require_source_line \ "storage-api public object precondition contract re-export" require_source_line \ "crates/storage-api/src/lib.rs" \ - "pub use object::{VersionMarker, WalkVersionsSortOrder};" \ + "pub use object::{VersionMarker, WalkOptions, WalkVersionsSortOrder};" \ "storage-api public list helper contract re-export" require_source_line \ "crates/storage-api/src/lib.rs" \ @@ -275,7 +275,7 @@ fi ( cd "$ROOT_DIR" - rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:VersionMarker|WalkVersionsSortOrder)\b|::(?:VersionMarker|WalkVersionsSortOrder)\b)|pub enum (?:VersionMarker|WalkVersionsSortOrder)\b' \ + rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:VersionMarker|WalkOptions|WalkVersionsSortOrder)\b|::(?:VersionMarker|WalkOptions|WalkVersionsSortOrder)\b)|pub (?:enum (?:VersionMarker|WalkVersionsSortOrder)|struct WalkOptions)\b' \ crates/ecstore/src/store_api.rs crates/ecstore/src/store_api/types.rs || true ) >"$STORE_API_LIST_HELPER_REEXPORTS_FILE"