From 8d23ce06c6dba11f50f656af4c30b63036cef92f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=AE=89=E6=AD=A3=E8=B6=85?= Date: Sun, 14 Jun 2026 20:43:42 +0800 Subject: [PATCH] refactor: consolidate app object store fallback (#3445) --- crates/ecstore/src/tier/tier.rs | 1 - docs/architecture/migration-progress.md | 69 ++++++++++++++----------- rustfs/src/app/admin_usecase.rs | 8 +-- rustfs/src/app/bucket_usecase.rs | 10 ++-- rustfs/src/app/context/compat.rs | 9 +++- rustfs/src/app/multipart_usecase.rs | 8 +-- rustfs/src/app/object_usecase.rs | 10 ++-- 7 files changed, 58 insertions(+), 57 deletions(-) diff --git a/crates/ecstore/src/tier/tier.rs b/crates/ecstore/src/tier/tier.rs index 75dde0fd9..d42847960 100644 --- a/crates/ecstore/src/tier/tier.rs +++ b/crates/ecstore/src/tier/tier.rs @@ -994,7 +994,6 @@ impl TierConfigMgr { } pub async fn reload(&mut self, api: Arc) -> std::result::Result<(), std::io::Error> { - //let Some(api) = new_object_layer_fn() else { return Err(Error::msg("errServerNotInitialized")) }; let new_config = load_tier_config(api).await; match &new_config { diff --git a/docs/architecture/migration-progress.md b/docs/architecture/migration-progress.md index 24b9ce81b..8b98aa8ec 100644 --- a/docs/architecture/migration-progress.md +++ b/docs/architecture/migration-progress.md @@ -5,19 +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-ecstore-internal-object-store-resolver` -- Baseline: `upstream/main` at `8f6b1d47b5af689e0b3576ab4448cc4274786df1` +- Branch: `overtrue/arch-app-usecase-object-store-fallback-cleanup` +- Baseline: `origin/main` at `fc894c9b569229101f859a6c4097eb64d3f86f5c` - PR type for this branch: `consumer-migration` -- Runtime behavior changes: no external behavior change expected; ECStore - internal/background object-store lookups prefer the AppContext-owned object - store through the ECStore-owned resolver and keep the existing global - fallback. -- Rust code changes: migrate ECStore internal metrics, notification, tier, - decommission, admin info, bucket metadata, replication, lifecycle, and data - usage object-store lookups to the ECStore-owned resolver without touching app - compatibility fallbacks or the global fallback definition. +- Runtime behavior changes: no external behavior change expected; app usecase + object-store lookups share the same explicit-context resolver and keep the + existing legacy global object-layer fallback when no usecase context exists. +- Rust code changes: add an explicit AppContext object-store resolver helper, + migrate admin, bucket, multipart, and object usecases to it, and remove a + stale ECStore tier comment that referenced the old direct accessor. - CI/script changes: none. -- Docs changes: record `CTX-010` consumer migration scope and verification. +- Docs changes: record `CTX-011` compatibility fallback cleanup scope and + verification. ## Phase 0 Tasks @@ -232,6 +231,19 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block usage cache persistence, and existing storage error paths. - Verification: formatting, compile checks, migration guards, diff hygiene, Rust risk scan, and full `make pre-commit`. +- [x] `CTX-011` Consolidate app usecase object-store fallback. + - Do: migrate app admin, bucket, multipart, and object usecases away from + direct `new_object_layer_fn` calls and through an explicit-context resolver + helper. + - Acceptance: usecase lookups keep their injected AppContext precedence, + preserve `without_context()` legacy global object-layer fallback semantics, + and avoid consulting the global AppContext when a usecase intentionally has + no context. + - Must preserve: admin storage/data-usage reads, bucket create/delete/list + behavior, multipart object writes, object API reads/writes, lifecycle + transition tests, and existing "Not init" error paths. + - Verification: formatting, compile checks, migration guards, diff hygiene, + Rust risk scan, and full `make pre-commit`. ## Phase 1 Security Governance Tasks @@ -538,8 +550,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block ## Next PRs -1. `consumer-migration`: review app compatibility fallback call sites before - the final global-accessor cleanup phase. +1. `consumer-migration`: remove the final old global object-layer accessor + compatibility path once downstream/public API cleanup is accepted. 2. `pure-move`: start `R-009` boot wrapper with the IAM degraded readiness contract covered. @@ -547,13 +559,13 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block | Expert | Status | Notes | |---|---|---| -| Quality/architecture | passed | Single consumer-migration slice; ECStore internal/background consumers use the ECStore-owned resolver without adding application-layer dependencies or changing the fallback definition. | -| Migration preservation | passed | Metrics, notification, tier, decommission, admin info, bucket metadata, replication, lifecycle, and data-usage call sites keep the same missing-store/error behavior while preferring the AppContext-owned store when installed. | -| Testing/verification | passed | Formatting, compile checks, diff hygiene, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. | +| Quality/architecture | passed | Single consumer-migration slice; app usecases delegate object-store fallback to the AppContext compatibility helper instead of duplicating direct global accessor calls. | +| Migration preservation | passed | Admin, bucket, multipart, and object usecases keep injected context precedence and explicit no-context legacy global fallback behavior. | +| Testing/verification | passed | Formatting, compile checks, migration/layer guards, Rust risk scan, branch freshness check, and full `make pre-commit` passed. | ## Verification Notes -Passed on `8f6b1d47b5af689e0b3576ab4448cc4274786df1`: +Passed on `fc894c9b569229101f859a6c4097eb64d3f86f5c`: - `cargo fmt --all --check`. - `cargo check -p rustfs-ecstore`. @@ -563,26 +575,21 @@ Passed on `8f6b1d47b5af689e0b3576ab4448cc4274786df1`: - `./scripts/check_layer_dependencies.sh`. - `git rev-list --left-right --count HEAD...origin/main` returned `1 0` after rebase. -- Rust risk scan for changed Rust files; full-file matches were existing tests, - existing relaxed counters, existing numeric casts, existing print/debug - helpers, and existing unwrap/expect paths, and the added-line scan returned - no `unwrap`/`expect`, numeric cast, string error, boxed error, print macro, or - relaxed-ordering match. -- `make pre-commit`; all checks passed, including nextest 5954 passed and 111 - skipped, plus doctests. +- Rust risk scan for changed Rust files: full-file matches were existing tests, + existing numeric casts, existing string error signatures, and existing relaxed + counters; added-line scan returned no unwrap/expect, numeric cast, string + error, boxed error, print macro, or relaxed-ordering match. +- `make pre-commit`: all checks passed, including nextest with 5961 passed + and 111 skipped, plus doctests. Notes: -- This slice migrates ECStore internal/background object-store lookups to the - ECStore-owned resolver. -- App compatibility fallback call sites remain on `new_object_layer_fn` as an - explicit fallback path. +- This slice consolidates app usecase object-store fallback without changing + request behavior. - The old global accessor remains as the resolver fallback and public compatibility re-export for a later cleanup slice. ## Handoff Notes -- CTX-010 is complete. -- App compatibility fallback call sites remain on `new_object_layer_fn` as an - explicit fallback path. +- CTX-011 is complete. - The global fallback definition and re-export remain for a later cleanup slice. diff --git a/rustfs/src/app/admin_usecase.rs b/rustfs/src/app/admin_usecase.rs index 6bfa298f7..4352d244a 100644 --- a/rustfs/src/app/admin_usecase.rs +++ b/rustfs/src/app/admin_usecase.rs @@ -14,7 +14,7 @@ //! Admin application use-case contracts. -use crate::app::context::{AppContext, get_global_app_context}; +use crate::app::context::{AppContext, get_global_app_context, resolve_object_store_handle_for_context}; use crate::capacity::resolve_admin_used_capacity; use crate::error::ApiError; use crate::server::{DependencyReadiness, collect_dependency_readiness as collect_runtime_dependency_readiness}; @@ -22,7 +22,6 @@ use rustfs_data_usage::DataUsageInfo; use rustfs_ecstore::admin_server_info::get_server_info; use rustfs_ecstore::data_usage::{apply_bucket_usage_memory_overlay, load_data_usage_from_backend}; use rustfs_ecstore::endpoints::EndpointServerPools; -use rustfs_ecstore::new_object_layer_fn; use rustfs_ecstore::pools::{PoolDecommissionInfo, PoolStatus, get_total_usable_capacity, get_total_usable_capacity_free}; use rustfs_ecstore::store::ECStore; use rustfs_madmin::{InfoMessage, StorageInfo}; @@ -104,10 +103,7 @@ impl DefaultAdminUsecase { } fn object_store(&self) -> Option> { - self.context - .as_ref() - .map(|context| context.object_store()) - .or_else(new_object_layer_fn) + resolve_object_store_handle_for_context(self.context.as_deref()) } fn app_error(code: S3ErrorCode, message: impl Into) -> ApiError { diff --git a/rustfs/src/app/bucket_usecase.rs b/rustfs/src/app/bucket_usecase.rs index b92039407..10743ec62 100644 --- a/rustfs/src/app/bucket_usecase.rs +++ b/rustfs/src/app/bucket_usecase.rs @@ -17,7 +17,9 @@ use crate::admin::handlers::site_replication::{ site_replication_bucket_meta_hook, site_replication_delete_bucket_hook, site_replication_make_bucket_hook, }; -use crate::app::context::{AppContext, default_notify_interface, get_global_app_context}; +use crate::app::context::{ + AppContext, default_notify_interface, get_global_app_context, resolve_object_store_handle_for_context, +}; use crate::auth::get_condition_values_with_client_info; use crate::error::ApiError; use crate::server::RemoteAddr; @@ -53,7 +55,6 @@ use rustfs_ecstore::bucket::{ }; use rustfs_ecstore::client::object_api_utils::to_s3s_etag; use rustfs_ecstore::error::StorageError; -use rustfs_ecstore::new_object_layer_fn; use rustfs_ecstore::notification_sys::get_global_notification_sys; use rustfs_ecstore::store::ECStore; use rustfs_ecstore::store_api::{BucketOperations, ListObjectVersionsInfo, ListObjectsV2Info, ListOperations, ObjectInfo}; @@ -740,10 +741,7 @@ impl DefaultBucketUsecase { } fn object_store(&self) -> Option> { - self.context - .as_ref() - .map(|context| context.object_store()) - .or_else(new_object_layer_fn) + resolve_object_store_handle_for_context(self.context.as_deref()) } #[instrument( diff --git a/rustfs/src/app/context/compat.rs b/rustfs/src/app/context/compat.rs index 6883861a5..5ea61ba8d 100644 --- a/rustfs/src/app/context/compat.rs +++ b/rustfs/src/app/context/compat.rs @@ -41,7 +41,13 @@ pub fn resolve_bucket_metadata_handle() -> Option> /// Resolve object store handle using AppContext-first precedence. pub fn resolve_object_store_handle() -> Option> { - resolve_object_store_handle_with(get_global_app_context(), new_object_layer_fn) + let context = get_global_app_context(); + resolve_object_store_handle_for_context(context.as_deref()) +} + +/// Resolve object store handle using an explicit AppContext, falling back to the legacy global object layer. +pub fn resolve_object_store_handle_for_context(context: Option<&AppContext>) -> Option> { + context.map(|context| context.object_store()).or_else(new_object_layer_fn) } /// Resolve endpoints using AppContext-first precedence. @@ -77,6 +83,7 @@ fn resolve_bucket_metadata_handle_with( .or_else(fallback) } +#[cfg(test)] fn resolve_object_store_handle_with( context: Option>, fallback: impl FnOnce() -> Option>, diff --git a/rustfs/src/app/multipart_usecase.rs b/rustfs/src/app/multipart_usecase.rs index d009cd985..49dac5eb5 100644 --- a/rustfs/src/app/multipart_usecase.rs +++ b/rustfs/src/app/multipart_usecase.rs @@ -14,7 +14,7 @@ //! Multipart application use-case contracts. -use crate::app::context::{AppContext, get_global_app_context}; +use crate::app::context::{AppContext, get_global_app_context, resolve_object_store_handle_for_context}; use crate::app::object_usecase::{build_put_like_object_lock_metadata, validate_existing_object_lock_for_write}; use crate::capacity::record_capacity_write; use crate::error::ApiError; @@ -48,7 +48,6 @@ use rustfs_ecstore::bucket::{ use rustfs_ecstore::client::object_api_utils::to_s3s_etag; use rustfs_ecstore::compress::is_disk_compressible; use rustfs_ecstore::error::{StorageError, is_err_object_not_found, is_err_version_not_found}; -use rustfs_ecstore::new_object_layer_fn; #[cfg(test)] use rustfs_ecstore::rio::{DecryptReader, EncryptReader, HardLimitReader, boxed_reader, wrap_reader}; use rustfs_ecstore::rio::{HashReader, WritePlan}; @@ -230,10 +229,7 @@ impl DefaultMultipartUsecase { } fn object_store(&self) -> Option> { - self.context - .as_ref() - .map(|context| context.object_store()) - .or_else(new_object_layer_fn) + resolve_object_store_handle_for_context(self.context.as_deref()) } #[instrument(level = "debug", skip(self))] diff --git a/rustfs/src/app/object_usecase.rs b/rustfs/src/app/object_usecase.rs index 1562b214a..262c934dc 100644 --- a/rustfs/src/app/object_usecase.rs +++ b/rustfs/src/app/object_usecase.rs @@ -14,7 +14,9 @@ //! Object application use-case contracts. -use crate::app::context::{AppContext, default_notify_interface, get_global_app_context}; +use crate::app::context::{ + AppContext, default_notify_interface, get_global_app_context, resolve_object_store_handle_for_context, +}; use crate::config::RustFSBufferConfig; use crate::delete_tail_activity::{DeleteTailActivityGuard, DeleteTailStage}; use crate::error::ApiError; @@ -73,7 +75,6 @@ use rustfs_ecstore::compress::{MIN_DISK_COMPRESSIBLE_SIZE, is_disk_compressible} use rustfs_ecstore::config::storageclass; use rustfs_ecstore::disk::{error::DiskError, error_reduce::is_all_buckets_not_found}; use rustfs_ecstore::error::{StorageError, is_err_bucket_not_found, is_err_object_not_found, is_err_version_not_found}; -use rustfs_ecstore::new_object_layer_fn; 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; @@ -1158,10 +1159,7 @@ impl DefaultObjectUsecase { } fn object_store(&self) -> Option> { - self.context - .as_ref() - .map(|context| context.object_store()) - .or_else(new_object_layer_fn) + resolve_object_store_handle_for_context(self.context.as_deref()) } fn base_buffer_size(&self) -> usize {