From 24443c829d435b216b0387f3644d312a44d0f64d 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 11:57:46 +0800 Subject: [PATCH] refactor: clean shared list operation consumers (#3563) --- crates/iam/src/store/object.rs | 10 +-- docs/architecture/crate-boundaries.md | 5 ++ docs/architecture/migration-progress.md | 86 ++++++++++++------- rustfs/src/app/bucket_usecase.rs | 11 ++- rustfs/src/storage/s3_api/bucket.rs | 16 ++-- scripts/check_architecture_migration_rules.sh | 11 +++ 6 files changed, 93 insertions(+), 46 deletions(-) diff --git a/crates/iam/src/store/object.rs b/crates/iam/src/store/object.rs index 6709b12e9..c28cc149e 100644 --- a/crates/iam/src/store/object.rs +++ b/crates/iam/src/store/object.rs @@ -22,8 +22,7 @@ use crate::{ }; use futures::future::join_all; use rustfs_credentials::get_global_action_cred; -use rustfs_ecstore::error::{StorageError, classify_system_path_failure_reason}; -use rustfs_ecstore::store_api::{ObjectInfoOrErr, WalkOptions}; +use rustfs_ecstore::error::{Error as EcstoreError, StorageError, classify_system_path_failure_reason}; use rustfs_ecstore::{ config::{ RUSTFS_CONFIG_PREFIX, @@ -34,8 +33,7 @@ use rustfs_ecstore::{ }; use rustfs_io_metrics::record_system_path_failure; use rustfs_policy::{auth::UserIdentity, policy::PolicyDoc}; -use rustfs_storage_api::HTTPPreconditions; -use rustfs_storage_api::ListOperations as _; +use rustfs_storage_api::{HTTPPreconditions, ListOperations as _, ObjectInfoOrErr as StorageObjectInfoOrErr}; use rustfs_utils::path::{SLASH_SEPARATOR, path_join_buf}; use serde::{Serialize, de::DeserializeOwned}; use std::sync::{LazyLock, Mutex}; @@ -62,6 +60,8 @@ pub static IAM_CONFIG_POLICY_DB_SERVICE_ACCOUNTS_PREFIX: LazyLock = pub static IAM_CONFIG_POLICY_DB_GROUPS_PREFIX: LazyLock = LazyLock::new(|| format!("{RUSTFS_CONFIG_PREFIX}/iam/policydb/groups/")); +type ObjectInfoOrErr = StorageObjectInfoOrErr; + const IAM_IDENTITY_FILE: &str = "identity.json"; const IAM_POLICY_FILE: &str = "policy.json"; const IAM_GROUP_MEMBERS_FILE: &str = "members.json"; @@ -367,7 +367,7 @@ impl ObjectStore { let sender_on_error = sender.clone(); tokio::spawn(async move { if let Err(err) = store - .walk(ctx.clone(), Self::BUCKET_NAME, &path, tx, WalkOptions::default()) + .walk(ctx.clone(), Self::BUCKET_NAME, &path, tx, Default::default()) .await { let reason = classify_system_path_failure_reason(&err); diff --git a/docs/architecture/crate-boundaries.md b/docs/architecture/crate-boundaries.md index b08738711..b77a75da5 100644 --- a/docs/architecture/crate-boundaries.md +++ b/docs/architecture/crate-boundaries.md @@ -106,3 +106,8 @@ and the separate `NamespaceLocking` operation group. The old `StorageAPI` aggregate facade must not reappear in production `crates/ecstore/src` or `rustfs/src` code after the storage operation groups have been made explicit. + +Outer RustFS/IAM consumers must use `rustfs-storage-api` generic list response +contracts directly for `ListObjectsV2Info`, `ListObjectVersionsInfo`, and +`ObjectInfoOrErr`; ECStore keeps the concrete aliases only for internal +implementation and compatibility. diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index df163e0d4..e00f4c590 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,19 +5,19 @@ 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-storage-operation-consumer-cleanup` -- Baseline: `main` at `e5cad7ed207265f5312c58d8dac6042e31ba95a3` - after the object and multipart operation contract merge. -- PR type for this branch: `api-extraction` +- Branch: `overtrue/arch-storage-shared-operation-bounds` +- Baseline: `main` at `46e200c290e3fc0887f4c19172bcc146e019d737` + after the heal/namespace-lock operation contract merge. +- PR type for this branch: `consumer-migration` - Runtime behavior changes: no external behavior change expected. -- Rust code changes: move `HealOperations` and `NamespaceLocking` into - `rustfs-storage-api` as generic operation contracts with associated - ECStore-bound types, then keep ECStore's existing public names as fixed - associated-type compatibility subtraits. -- CI/script changes: extend migration guards for the heal/namespace-lock - operation public re-exports and ECStore local-method regressions. -- Docs changes: record the heal and namespace-lock operation contract - extraction slice. +- Rust code changes: migrate RustFS bucket response builders and IAM walk + channel typing away from ECStore list response aliases to the generic + `rustfs-storage-api` list contracts while keeping ECStore-owned + `ObjectInfo`, `ObjectOptions`, readers, delete DTOs, and implementation + behavior in ECStore. +- CI/script changes: extend migration guards so outer RustFS/IAM list response + consumers do not drift back to ECStore alias imports. +- Docs changes: record the shared list operation consumer cleanup slice. ## Phase 0 Tasks @@ -732,6 +732,27 @@ 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-024` Clean shared list operation consumer bounds. + - Completed slice: migrate RustFS S3/bucket usecase list response builders from + ECStore `ListObjectVersionsInfo`/`ListObjectsV2Info` aliases to + `rustfs-storage-api` generic list response contracts bound to ECStore + `ObjectInfo`; migrate IAM walk channel typing from ECStore + `ObjectInfoOrErr` alias to the shared generic item contract. + - Acceptance: outer RustFS/IAM consumers use storage-api list response + contracts directly, ECStore keeps concrete aliases for internal + implementation and compatibility, and migration guards reject restoring the + old outer-consumer imports. + - Must preserve: S3 list v2/version output mapping, IAM config walk channel + item/error handling, ECStore concrete object metadata shape, walk options + inference, and storage error conversion behavior. + - Risk defense: this slice moves only low-coupling generic response/channel + typing; ECStore still owns `ObjectInfo`, `ObjectOptions`, readers, + filemeta-bound walk filter type, delete DTOs, and list/walk implementation + bodies. + - Verification: focused RustFS/IAM compile and tests, 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. @@ -1008,17 +1029,17 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | passed | Generic heal and namespace-lock traits now live in `rustfs-storage-api`; ECStore still owns concrete type bindings and implementations. | -| Migration preservation | passed | Existing ECStore public trait names remain as fixed associated-type compatibility subtraits while method resolution moves to shared traits where needed. | -| Testing/verification | passed | Focused storage-api tests, ECStore compat tests, downstream compile checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and full `make pre-commit` passed. | +| Quality/architecture | passed | Outer RustFS/IAM list response consumers now use the generic storage-api contracts; ECStore keeps concrete aliases only for implementation and compatibility. | +| Migration preservation | passed | S3 list response mapping and IAM walk item/error handling keep the same ECStore `ObjectInfo`, error conversion, and walk option inference behavior. | +| Testing/verification | passed | Focused RustFS/IAM compile/tests, 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 --test ecstore_contract_compat_test`: passed. -- `cargo check --tests -p rustfs-storage-api -p rustfs-ecstore -p rustfs -p rustfs-heal -p rustfs-scanner`: passed. +- `cargo check --tests -p rustfs -p rustfs-iam`: passed. +- `cargo test -p rustfs --lib storage::s3_api::bucket`: passed; 22 passed. +- `cargo test -p rustfs-iam`: passed; 150 passed. - `./scripts/check_architecture_migration_rules.sh`: passed. - `./scripts/check_layer_dependencies.sh`: passed. - `cargo fmt --all --check`: passed. @@ -1031,21 +1052,20 @@ Passed before push: Notes: -- This slice follows the object and multipart operation contract branch and keeps the old - aggregate facade, bucket DTO, multipart DTO, bucket operation contract, object - helper, range helper, list helper, object precondition, list response, and - walk/list/object/multipart operation contract guards active. -- The shared heal and namespace-lock operation contracts are now owned by - `rustfs-storage-api`; ECStore keeps the concrete associated type bindings, - `HealOpts`, `HealResultItem`, `NamespaceLockWrapper`, lock implementation, - peer heal behavior, set/pool dispatch, and storage error mapping. -- The slice does not alter heal, namespace-lock, object, list, walk, - multipart, bucket, delete, reader, or tag/metadata runtime behavior. +- This slice follows the heal and namespace-lock operation contract branch and + keeps the old aggregate facade, DTO/helper, response, and operation contract + guards active. +- Outer RustFS/IAM consumers now bind low-coupling list response/channel + containers through `rustfs-storage-api`; ECStore keeps the concrete aliases, + object metadata, filemeta-bound walk filter, readers, delete DTOs, and list + implementation behavior. +- The slice does not alter list, walk, bucket, object, delete, reader, + tag/metadata, heal, namespace-lock, multipart, or storage error runtime + behavior. ## Handoff Notes -- Heal and namespace-lock operation contract cleanup is based on the merged - object and multipart operation contract branch. -- After this lands, remaining storage work can continue by removing low-value - ECStore compatibility imports or extracting larger low-coupling object - metadata/option/reader slices. +- Shared list operation consumer cleanup is rebased onto `main` after + `rustfs/rustfs#3560` merged. +- After this lands, continue with larger consumer-migration batches instead of + single-alias slices. diff --git a/rustfs/src/app/bucket_usecase.rs b/rustfs/src/app/bucket_usecase.rs index 2bc886d27..ee71ce235 100644 --- a/rustfs/src/app/bucket_usecase.rs +++ b/rustfs/src/app/bucket_usecase.rs @@ -57,15 +57,17 @@ use rustfs_ecstore::client::object_api_utils::to_s3s_etag; use rustfs_ecstore::error::StorageError; use rustfs_ecstore::notification_sys::get_global_notification_sys; use rustfs_ecstore::store::ECStore; -use rustfs_ecstore::store_api::{ListObjectVersionsInfo, ListObjectsV2Info, ObjectInfo}; +use rustfs_ecstore::store_api::ObjectInfo; use rustfs_madmin::{SITE_REPL_API_VERSION, SRBucketMeta}; use rustfs_policy::policy::{ action::{Action, S3Action}, {BucketPolicy, BucketPolicyArgs, Effect, Validator}, }; use rustfs_s3_ops::S3Operation; -use rustfs_storage_api::ListOperations as _; -use rustfs_storage_api::{BucketOperations, BucketOptions, DeleteBucketOptions, MakeBucketOptions}; +use rustfs_storage_api::{ + BucketOperations, BucketOptions, DeleteBucketOptions, ListObjectVersionsInfo as StorageListObjectVersionsInfo, + ListObjectsV2Info as StorageListObjectsV2Info, ListOperations as _, MakeBucketOptions, +}; use rustfs_targets::{ EventName, arn::{ARN, TargetIDError}, @@ -89,6 +91,9 @@ const LOG_COMPONENT_APP: &str = "app"; const LOG_SUBSYSTEM_BUCKET: &str = "bucket"; use urlencoding::encode; +type ListObjectVersionsInfo = StorageListObjectVersionsInfo; +type ListObjectsV2Info = StorageListObjectsV2Info; + fn serialize_config(value: &T) -> S3Result> { serialize(value).map_err(to_internal_error) } diff --git a/rustfs/src/storage/s3_api/bucket.rs b/rustfs/src/storage/s3_api/bucket.rs index 6660a1c64..4cf1dee94 100644 --- a/rustfs/src/storage/s3_api/bucket.rs +++ b/rustfs/src/storage/s3_api/bucket.rs @@ -15,8 +15,10 @@ use crate::storage::s3_api::common::rustfs_owner; use percent_encoding::percent_decode_str; use rustfs_ecstore::client::object_api_utils::to_s3s_etag; -use rustfs_ecstore::store_api::{ListObjectVersionsInfo, ListObjectsV2Info}; -use rustfs_storage_api::BucketInfo; +use rustfs_ecstore::store_api::ObjectInfo; +use rustfs_storage_api::{ + BucketInfo, ListObjectVersionsInfo as StorageListObjectVersionsInfo, ListObjectsV2Info as StorageListObjectsV2Info, +}; use s3s::dto::{ Bucket, CommonPrefix, DeleteMarkerEntry, EncodingType, ListBucketsOutput, ListObjectVersionsOutput, ListObjectsOutput, ListObjectsV2Output, Object, ObjectStorageClass, ObjectVersion, ObjectVersionStorageClass, Timestamp, @@ -27,6 +29,9 @@ use urlencoding::encode; const S3_MAX_KEYS: i32 = 1000; +type ListObjectVersionsInfo = StorageListObjectVersionsInfo; +type ListObjectsV2Info = StorageListObjectsV2Info; + fn normalize_max_keys(max_keys: i32) -> i32 { max_keys.min(S3_MAX_KEYS) } @@ -384,11 +389,12 @@ fn calculate_next_marker(v2: &ListObjectsV2Output) -> Option { #[cfg(test)] mod tests { use super::{ - ListObjectVersionsParams, build_list_buckets_output, build_list_object_versions_output, build_list_objects_output, - build_list_objects_v2_output, parse_list_object_versions_params, parse_list_objects_v2_params, + ListObjectVersionsInfo, ListObjectVersionsParams, ListObjectsV2Info, build_list_buckets_output, + build_list_object_versions_output, build_list_objects_output, build_list_objects_v2_output, + parse_list_object_versions_params, parse_list_objects_v2_params, }; use crate::storage::s3_api::common::rustfs_owner; - use rustfs_ecstore::store_api::{ListObjectVersionsInfo, ListObjectsV2Info, ObjectInfo}; + use rustfs_ecstore::store_api::ObjectInfo; use rustfs_storage_api::BucketInfo; use s3s::S3ErrorCode; use s3s::dto::{CommonPrefix, EncodingType, ListObjectsV2Output, Object}; diff --git a/scripts/check_architecture_migration_rules.sh b/scripts/check_architecture_migration_rules.sh index a22ad0973..27234f0ed 100755 --- a/scripts/check_architecture_migration_rules.sh +++ b/scripts/check_architecture_migration_rules.sh @@ -57,6 +57,7 @@ STORE_API_OBJECT_HELPER_REEXPORTS_FILE="${TMP_DIR}/store_api_object_helper_reexp 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" +STORE_API_EXTERNAL_LIST_CONSUMER_HITS_FILE="${TMP_DIR}/store_api_external_list_consumer_hits.txt" STORE_API_OBJECT_OPERATION_LOCAL_METHOD_HITS_FILE="${TMP_DIR}/store_api_object_operation_local_method_hits.txt" awk ' @@ -298,6 +299,16 @@ 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 +( + cd "$ROOT_DIR" + rg -n --no-heading 'rustfs_ecstore::store_api(?:::\{[^}]*\b(?:ListObjectVersionsInfo|ListObjectsV2Info|ObjectInfoOrErr)\b|::(?:ListObjectVersionsInfo|ListObjectsV2Info|ObjectInfoOrErr)\b)' \ + rustfs/src crates/iam/src || true +) >"$STORE_API_EXTERNAL_LIST_CONSUMER_HITS_FILE" + +if [[ -s "$STORE_API_EXTERNAL_LIST_CONSUMER_HITS_FILE" ]]; then + report_failure "external list response consumers must use rustfs-storage-api contracts: $(paste -sd '; ' "$STORE_API_EXTERNAL_LIST_CONSUMER_HITS_FILE")" +fi + ( cd "$ROOT_DIR" rg -n --no-heading 'async fn (get_object_reader|put_object|get_object_info|verify_object_integrity|copy_object|delete_object_version|delete_object|delete_objects|put_object_metadata|get_object_tags|put_object_tags|delete_object_tags|add_partial|transition_object|restore_transitioned_object|list_multipart_uploads|new_multipart_upload|copy_object_part|put_object_part|get_multipart_info|list_object_parts|abort_multipart_upload|complete_multipart_upload|heal_format|heal_bucket|heal_object|get_pool_and_set|check_abandoned_parts|new_ns_lock)\b' \