mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-26 16:28:15 +00:00
refactor: remove storage api facade (#3490)
This commit is contained in:
@@ -60,7 +60,6 @@ pub use global::{get_global_lock_client, get_global_lock_clients, set_global_loc
|
||||
pub use global::{new_object_layer_fn, resolve_object_store_handle, set_object_store_resolver};
|
||||
|
||||
pub use global::GLOBAL_Endpoints;
|
||||
pub use store_api::StorageAPI;
|
||||
|
||||
#[cfg(test)]
|
||||
mod rio_tests {
|
||||
|
||||
@@ -55,7 +55,7 @@ use crate::{
|
||||
BucketInfo, BucketOperations, BucketOptions, CompletePart, DeleteBucketOptions, DeletedObject, GetObjectReader,
|
||||
HTTPRangeSpec, HealOperations, ListMultipartsInfo, ListObjectsV2Info, ListOperations, MakeBucketOptions, MultipartInfo,
|
||||
MultipartOperations, MultipartUploadResult, NamespaceLocking, ObjectIO, ObjectInfo, ObjectOperations, PartInfo,
|
||||
PutObjReader, StorageAPI,
|
||||
PutObjReader,
|
||||
},
|
||||
store_init::load_format_erasure,
|
||||
};
|
||||
@@ -1633,9 +1633,6 @@ impl SetDisks {
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl StorageAPI for SetDisks {}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl NamespaceLocking for SetDisks {
|
||||
#[tracing::instrument(skip(self))]
|
||||
|
||||
@@ -31,7 +31,7 @@ use crate::{
|
||||
BucketInfo, BucketOperations, BucketOptions, CompletePart, DeleteBucketOptions, DeletedObject, GetObjectReader,
|
||||
HTTPRangeSpec, HealOperations, ListMultipartsInfo, ListObjectVersionsInfo, ListObjectsV2Info, ListOperations,
|
||||
MakeBucketOptions, MultipartInfo, MultipartOperations, MultipartUploadResult, NamespaceLocking, ObjectIO, ObjectInfo,
|
||||
ObjectOperations, ObjectOptions, ObjectToDelete, PartInfo, PutObjReader, StorageAPI,
|
||||
ObjectOperations, ObjectOptions, ObjectToDelete, PartInfo, PutObjReader,
|
||||
},
|
||||
store_init::{check_format_erasure_values, get_format_erasure_in_quorum, load_format_erasure_all, save_format_file},
|
||||
};
|
||||
@@ -896,9 +896,6 @@ impl HealOperations for Sets {
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl StorageAPI for Sets {}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl NamespaceLocking for Sets {
|
||||
async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper> {
|
||||
|
||||
@@ -68,7 +68,7 @@ use crate::{
|
||||
BucketInfo, BucketOperations, BucketOptions, CompletePart, DeleteBucketOptions, DeletedObject, GetObjectReader,
|
||||
HTTPRangeSpec, HealOperations, ListObjectsV2Info, ListOperations, MakeBucketOptions, MultipartOperations,
|
||||
MultipartUploadResult, NamespaceLocking, ObjectInfo, ObjectOperations, ObjectOptions, ObjectToDelete, PartInfo,
|
||||
PutObjReader, StorageAPI,
|
||||
PutObjReader,
|
||||
},
|
||||
store_init,
|
||||
};
|
||||
@@ -703,9 +703,6 @@ impl HealOperations for ECStore {
|
||||
}
|
||||
}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl StorageAPI for ECStore {}
|
||||
|
||||
#[async_trait::async_trait]
|
||||
impl NamespaceLocking for ECStore {
|
||||
async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper> {
|
||||
|
||||
@@ -178,13 +178,3 @@ pub trait HealOperations: Send + Sync + Debug {
|
||||
pub trait NamespaceLocking: Send + Sync + Debug + 'static {
|
||||
async fn new_ns_lock(&self, bucket: &str, object: &str) -> Result<NamespaceLockWrapper>;
|
||||
}
|
||||
|
||||
/// Unified storage API combining all operation groups.
|
||||
///
|
||||
/// Consumers can depend on specific sub-traits (e.g., `BucketOperations`)
|
||||
/// when they don't need the full API surface.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub trait StorageAPI:
|
||||
ObjectIO + BucketOperations + ObjectOperations + ListOperations + MultipartOperations + HealOperations + Debug
|
||||
{
|
||||
}
|
||||
|
||||
@@ -12,12 +12,7 @@
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
use rustfs_ecstore::{
|
||||
disk::DiskStore,
|
||||
error::Error,
|
||||
store::ECStore,
|
||||
store_api::{NamespaceLocking, StorageAPI},
|
||||
};
|
||||
use rustfs_ecstore::{disk::DiskStore, error::Error, store::ECStore, store_api::NamespaceLocking};
|
||||
use rustfs_storage_api::StorageAdminApi;
|
||||
|
||||
fn storage_admin_api_type_name<T>() -> &'static str
|
||||
@@ -32,9 +27,9 @@ where
|
||||
std::any::type_name::<T>()
|
||||
}
|
||||
|
||||
fn storage_api_with_namespace_locking_type_name<T>() -> &'static str
|
||||
fn namespace_locking_type_name<T>() -> &'static str
|
||||
where
|
||||
T: StorageAPI + NamespaceLocking,
|
||||
T: NamespaceLocking,
|
||||
{
|
||||
std::any::type_name::<T>()
|
||||
}
|
||||
@@ -45,6 +40,6 @@ fn ecstore_implements_storage_admin_api_contract() {
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ecstore_implements_storage_api_and_namespace_locking_contracts() {
|
||||
assert!(storage_api_with_namespace_locking_type_name::<ECStore>().ends_with("::ECStore"));
|
||||
fn ecstore_implements_namespace_locking_contract() {
|
||||
assert!(namespace_locking_type_name::<ECStore>().ends_with("::ECStore"));
|
||||
}
|
||||
|
||||
@@ -85,8 +85,8 @@ the first extraction.
|
||||
|
||||
## Loss-Prevention Coverage
|
||||
|
||||
Architecture migration checks must keep public contract re-exports and storage
|
||||
trait coverage from silently drifting during cleanup PRs.
|
||||
Architecture migration checks must keep public contract re-exports and ECStore
|
||||
compatibility coverage from silently drifting during cleanup PRs.
|
||||
|
||||
Required `rustfs-storage-api` public re-exports:
|
||||
|
||||
@@ -94,16 +94,5 @@ Required `rustfs-storage-api` public re-exports:
|
||||
- `pub use bucket::{BucketInfo, BucketOptions, DeleteBucketOptions, MakeBucketOptions, SRBucketDeleteOp};`
|
||||
- `pub use error::{StorageErrorCode, StorageResult};`
|
||||
|
||||
Required `StorageAPI` operation groups:
|
||||
|
||||
- `ObjectIO`
|
||||
- `BucketOperations`
|
||||
- `ObjectOperations`
|
||||
- `ListOperations`
|
||||
- `MultipartOperations`
|
||||
- `HealOperations`
|
||||
- `Debug`
|
||||
|
||||
`NamespaceLocking` must remain a separate operation group from the full
|
||||
`StorageAPI` facade. ECStore must keep compile-time coverage for both
|
||||
`StorageAdminApi` and `StorageAPI + NamespaceLocking`.
|
||||
ECStore must keep compile-time coverage for both `StorageAdminApi` and the
|
||||
separate `NamespaceLocking` operation group.
|
||||
|
||||
@@ -5,16 +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-config-storage-boundary-cleanup`
|
||||
- Baseline: `origin/main` at `fbab160c2b09075f5e2503a669d82917ef82d40e`
|
||||
- Branch: `overtrue/arch-remove-storage-api-facade`
|
||||
- Baseline: `origin/main` at `c26593fa7a4b55849e98832cde657c6d4b167262`
|
||||
- PR type for this branch: `consumer-migration`
|
||||
- Runtime behavior changes: no external behavior change expected.
|
||||
- Rust code changes: remove stale full `StorageAPI` coupling from config
|
||||
persistence tests, an unused S3 remove-client import, and an obsolete storage
|
||||
list comment.
|
||||
- CI/script changes: none.
|
||||
- Docs changes: refresh the ECStore config persistence inventory and current
|
||||
verification state.
|
||||
- Rust code changes: remove the old unused `StorageAPI` facade, its ECStore
|
||||
implementation blocks, its public re-export, and the stale compile-time
|
||||
compatibility test coverage.
|
||||
- CI/script changes: adjust architecture migration guardrails to keep the
|
||||
remaining storage-admin and namespace-lock contracts covered.
|
||||
- Docs changes: record the final old-facade cleanup slice and its verification
|
||||
state.
|
||||
|
||||
## Phase 0 Tasks
|
||||
|
||||
@@ -35,13 +36,12 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
- Completed slices: add a mechanical admin route matrix guard from
|
||||
[`admin-route-action-snapshot.md`](admin-route-action-snapshot.md) and
|
||||
`rustfs/src/admin/route_registration_test.rs`; add migration rules for
|
||||
public storage-api re-export coverage, StorageAPI operation-group coverage,
|
||||
NamespaceLocking separation, and ECStore compatibility-test coverage.
|
||||
public storage-api re-export coverage and ECStore compatibility-test
|
||||
coverage.
|
||||
- Acceptance: architecture migration rules fail if the public storage-api
|
||||
contract re-export surface drifts, if `StorageAPI` stops covering the
|
||||
documented storage operation groups, if `NamespaceLocking` is folded back
|
||||
into the full storage facade, or if ECStore compile-time compatibility tests
|
||||
for these contracts are removed.
|
||||
contract re-export surface drifts or if ECStore compile-time compatibility
|
||||
tests for the remaining storage-admin and namespace-lock contracts are
|
||||
removed.
|
||||
- [x] `G-007` Create startup timeline table.
|
||||
- Acceptance: [`startup-timeline.md`](startup-timeline.md) records current
|
||||
binary startup order, side effects, fatal boundaries, and readiness stages.
|
||||
@@ -410,7 +410,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
- Completed slice: `rustfs/rustfs#3340` removed duplicate admin-read methods
|
||||
from the old `StorageAPI` trait and its ECStore/Sets/SetDisks/test
|
||||
implementations after API-007 migrated their consumers.
|
||||
- Acceptance: old `StorageAPI` keeps storage operation traits while admin
|
||||
- Final cleanup slice: remove the old `StorageAPI` facade after all real
|
||||
consumers moved to concrete operation groups.
|
||||
- Acceptance: storage operation traits remain available directly while admin
|
||||
inventory surfaces live only on `StorageAdminApi`.
|
||||
|
||||
- [x] `API-009` Narrow metadata helper storage bounds.
|
||||
@@ -422,6 +424,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
- Cleanup slice: remove stale full `StorageAPI` dependencies from config
|
||||
persistence test support after the server-config persistence helpers moved
|
||||
to their actual object I/O and storage-admin bounds.
|
||||
- Completed cleanup slice: `rustfs/rustfs#3489` removed the stale full
|
||||
facade dependency from config persistence test support.
|
||||
- Acceptance: metadata helper contracts express the actual operation group
|
||||
they need, while callers and persistence behavior remain unchanged.
|
||||
|
||||
@@ -431,8 +435,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
helper only needs `ObjectIO`.
|
||||
- Acceptance: resync metadata helpers express object-I/O-only persistence
|
||||
requirements, while replication execution, delete replication, multipart
|
||||
replication, object lookups, and scheduling behavior remain on full
|
||||
`StorageAPI` where needed.
|
||||
replication, object lookups, and scheduling behavior remain on the concrete
|
||||
operation groups they need.
|
||||
|
||||
- [x] `API-011` Narrow scanner cache helper storage bounds.
|
||||
- Completed slice: `rustfs/rustfs#3348` narrowed scanner data-usage cache
|
||||
@@ -445,9 +449,9 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
cache paths, retry and timeout behavior, cache-save metrics, publish/update
|
||||
channel behavior, scanner cycle scheduling, disk scan concurrency, bucket
|
||||
scan semantics, lifecycle/replication decisions, and storage hot paths.
|
||||
- Risk defense: do not move traits to `rustfs-storage-api`, do not remove
|
||||
`StorageAPI`, do not alter helper bodies, and do not narrow scanner paths
|
||||
that need bucket operations, disk inventory, or full storage orchestration.
|
||||
- Risk defense: do not move traits to `rustfs-storage-api`, do not alter
|
||||
helper bodies, and do not narrow scanner paths that need bucket operations,
|
||||
disk inventory, or full storage orchestration.
|
||||
- Verification: focused compile/tests, migration guards, Rust risk scan, and
|
||||
required quality/architecture, migration-preservation, and
|
||||
testing/verification review passed.
|
||||
@@ -470,6 +474,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
resync leader-lock, delete replication, object replication, and multipart
|
||||
replication helpers away from full `StorageAPI` where they only need object
|
||||
I/O, object operations, list operations, and namespace locking.
|
||||
- Final cleanup slice: remove the unused old `StorageAPI` facade, its
|
||||
implementation blocks, public re-export, and stale guard coverage.
|
||||
- Acceptance: table catalog object backend contracts express the actual
|
||||
object read/write, metadata/delete, list, and namespace-lock capabilities
|
||||
they need; namespace-lock consumers depend on `NamespaceLocking` instead of
|
||||
@@ -480,8 +486,8 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
scanner/heal/replication/config persistence, and storage hot paths.
|
||||
- Risk defense: do not move traits into `rustfs-storage-api`, do not change
|
||||
lock implementation code, do not alter table catalog method bodies, and do
|
||||
not retain stale API-012 compatibility markers after the old `StorageAPI`
|
||||
lock method is removed.
|
||||
not leave stale full-facade compatibility coverage after consumers move to
|
||||
concrete operation groups.
|
||||
- Verification: focused compile/tests, migration guards, Rust risk scan, and
|
||||
required quality/architecture, migration-preservation, and
|
||||
testing/verification review passed.
|
||||
@@ -755,45 +761,44 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
## Next PRs
|
||||
|
||||
1. `pure-move`/`consumer-migration`: continue larger cleanup slices with the
|
||||
loss-prevention guards active for public re-exports and storage trait
|
||||
coverage.
|
||||
loss-prevention guards active for public re-exports and remaining storage
|
||||
compatibility contracts.
|
||||
|
||||
## Pre-Push Review Log
|
||||
|
||||
| Expert | Status | Notes |
|
||||
|---|---|---|
|
||||
| Quality/architecture | passed | Config persistence tests now express their actual object I/O, namespace-lock, and storage-admin requirements instead of a full storage facade. |
|
||||
| Migration preservation | passed | Config object encoding/decoding, metadata reads, namespace-lock behavior, and S3 remove-client behavior are unchanged. |
|
||||
| Testing/verification | passed | Focused config tests, compile checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and full `make pre-commit` passed. |
|
||||
| Quality/architecture | passed | Old `StorageAPI` facade removal leaves concrete operation traits and remaining storage-admin/namespace-lock contracts explicit. |
|
||||
| Migration preservation | passed | ECStore/Sets/SetDisks operation implementations remain in place; only the unused aggregate facade and stale guard coverage are removed. |
|
||||
| Testing/verification | passed | Focused compatibility test, compile checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and full `make pre-commit` passed. |
|
||||
|
||||
## Verification Notes
|
||||
|
||||
Passed on `fbab160c2b09075f5e2503a669d82917ef82d40e`:
|
||||
Passed on `c26593fa7a4b55849e98832cde657c6d4b167262`:
|
||||
|
||||
- `cargo check -p rustfs-ecstore`: passed.
|
||||
- `cargo test -p rustfs-ecstore config::com --no-fail-fast`: passed.
|
||||
- `cargo test -p rustfs-ecstore --test storage_api_compat_test --no-fail-fast`:
|
||||
passed.
|
||||
- `cargo check -p rustfs -p rustfs-ecstore`: passed.
|
||||
- `./scripts/check_architecture_migration_rules.sh`: passed.
|
||||
- `./scripts/check_layer_dependencies.sh`: passed.
|
||||
- `cargo fmt --all --check`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Rust risk scan: no new production `unwrap`/`expect`, lossy casts, string
|
||||
errors, public boxed errors, production `println`/`eprintln`, or relaxed
|
||||
atomics in added Rust lines.
|
||||
- Rust risk scan: no new production `unwrap`/`expect`, panic/todo markers,
|
||||
`unsafe`, or process-spawning calls in added Rust lines.
|
||||
- `make pre-commit`: passed.
|
||||
|
||||
Notes:
|
||||
|
||||
- This slice removes test-only full facade scaffolding after config persistence
|
||||
helpers already moved to narrower object I/O and storage-admin contracts.
|
||||
- The S3 remove client had a stale full facade import only; behavior remains
|
||||
unchanged.
|
||||
- The slice does not remove the full storage facade or move traits across crate
|
||||
boundaries.
|
||||
- This slice removes the old full storage facade after no real code consumers
|
||||
remain.
|
||||
- The concrete storage operation traits, `StorageAdminApi`, and
|
||||
`NamespaceLocking` remain available and covered.
|
||||
- The slice does not move traits across crate boundaries.
|
||||
|
||||
## Handoff Notes
|
||||
|
||||
- Config storage boundary cleanup is locally verified and current with
|
||||
- Old storage facade removal is locally verified on a branch current with
|
||||
`origin/main`.
|
||||
- Remaining storage-facade cleanup can continue by migrating other consumers
|
||||
that no longer need the full storage facade.
|
||||
- After this lands, remaining storage work can focus on concrete operation
|
||||
contracts instead of the aggregate facade.
|
||||
|
||||
@@ -49,7 +49,6 @@ PR_TYPE_HITS_FILE="${TMP_DIR}/pr_type_hits.txt"
|
||||
SOURCE_MARKERS_FILE="${TMP_DIR}/source_markers.txt"
|
||||
SOURCE_IDS_FILE="${TMP_DIR}/source_ids.txt"
|
||||
REGISTER_IDS_FILE="${TMP_DIR}/register_ids.txt"
|
||||
STORAGE_API_SUPERTRAITS_FILE="${TMP_DIR}/storage_api_supertraits.txt"
|
||||
|
||||
awk '
|
||||
/^## PR Types$/ {
|
||||
@@ -187,30 +186,6 @@ require_source_line \
|
||||
"pub use error::{StorageErrorCode, StorageResult};" \
|
||||
"storage-api public error contract re-export"
|
||||
|
||||
perl -0ne '
|
||||
if (/pub trait StorageAPI:\s*(.*?)\s*\{\s*\}/s) {
|
||||
my $body = $1;
|
||||
while ($body =~ /\b([A-Z][A-Za-z0-9_]*)\b/g) {
|
||||
print "$1\n";
|
||||
}
|
||||
}
|
||||
' "${ROOT_DIR}/crates/ecstore/src/store_api/traits.rs" |
|
||||
sort -u >"$STORAGE_API_SUPERTRAITS_FILE"
|
||||
|
||||
if [[ ! -s "$STORAGE_API_SUPERTRAITS_FILE" ]]; then
|
||||
report_failure "StorageAPI supertrait declaration not found in crates/ecstore/src/store_api/traits.rs"
|
||||
fi
|
||||
|
||||
for required_trait in ObjectIO BucketOperations ObjectOperations ListOperations MultipartOperations HealOperations Debug; do
|
||||
if ! contains_line "$required_trait" "$STORAGE_API_SUPERTRAITS_FILE"; then
|
||||
report_failure "StorageAPI no longer covers required operation group ${required_trait}"
|
||||
fi
|
||||
done
|
||||
|
||||
if contains_line "NamespaceLocking" "$STORAGE_API_SUPERTRAITS_FILE"; then
|
||||
report_failure "NamespaceLocking must remain separate from the full StorageAPI facade"
|
||||
fi
|
||||
|
||||
require_source_contains \
|
||||
"crates/ecstore/src/store_api/traits.rs" \
|
||||
"pub trait NamespaceLocking: Send + Sync + Debug + 'static" \
|
||||
@@ -221,8 +196,8 @@ require_source_contains \
|
||||
"ECStore StorageAdminApi compile-time coverage test"
|
||||
require_source_contains \
|
||||
"crates/ecstore/tests/storage_api_compat_test.rs" \
|
||||
"fn ecstore_implements_storage_api_and_namespace_locking_contracts()" \
|
||||
"ECStore StorageAPI and NamespaceLocking compile-time coverage test"
|
||||
"fn ecstore_implements_namespace_locking_contract()" \
|
||||
"ECStore NamespaceLocking compile-time coverage test"
|
||||
|
||||
if (( FAILURES > 0 )); then
|
||||
exit 1
|
||||
|
||||
Reference in New Issue
Block a user