mirror of
https://github.com/rustfs/rustfs.git
synced 2026-07-28 17:18:58 +00:00
refactor: move object precondition contracts (#3547)
This commit is contained in:
@@ -1,5 +1,8 @@
|
||||
use super::*;
|
||||
use rustfs_storage_api::{HTTPPreconditions, ObjectLockRetentionOptions, VersionMarker, WalkVersionsSortOrder};
|
||||
use rustfs_storage_api::{
|
||||
HTTPPreconditions, ObjectLockRetentionOptions, ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState,
|
||||
VersionMarker, WalkVersionsSortOrder,
|
||||
};
|
||||
|
||||
#[derive(Debug, Default, Clone)]
|
||||
pub struct ObjectOptions {
|
||||
@@ -102,76 +105,30 @@ impl ObjectOptions {
|
||||
}
|
||||
|
||||
pub fn precondition_check(&self, obj_info: &ObjectInfo) -> Result<()> {
|
||||
let has_valid_mod_time = obj_info.mod_time.is_some_and(|t| t != OffsetDateTime::UNIX_EPOCH);
|
||||
|
||||
if let Some(part_number) = self.part_number
|
||||
&& part_number > 1
|
||||
&& !obj_info.parts.is_empty()
|
||||
{
|
||||
let part_found = obj_info.parts.iter().any(|pi| pi.number == part_number);
|
||||
if !part_found {
|
||||
return Err(Error::InvalidPartNumber(part_number));
|
||||
let requested_part = self.part_number.and_then(|part_number| {
|
||||
if part_number > 1 && !obj_info.parts.is_empty() {
|
||||
Some(ObjectPreconditionPart {
|
||||
number: part_number,
|
||||
exists: obj_info.parts.iter().any(|pi| pi.number == part_number),
|
||||
})
|
||||
} else {
|
||||
None
|
||||
}
|
||||
}
|
||||
});
|
||||
let state = ObjectPreconditionState {
|
||||
etag: obj_info.etag.as_deref(),
|
||||
mod_time: obj_info.mod_time,
|
||||
requested_part,
|
||||
};
|
||||
|
||||
if let Some(pre) = &self.http_preconditions {
|
||||
let if_none_match = pre.if_none_match_value();
|
||||
let if_match = pre.if_match_value();
|
||||
|
||||
if let Some(if_none_match) = if_none_match
|
||||
&& let Some(etag) = &obj_info.etag
|
||||
&& is_etag_equal(etag, if_none_match)
|
||||
{
|
||||
return Err(Error::NotModified);
|
||||
}
|
||||
|
||||
if has_valid_mod_time
|
||||
&& let Some(if_modified_since) = &pre.if_modified_since
|
||||
&& let Some(mod_time) = &obj_info.mod_time
|
||||
&& !is_modified_since(mod_time, if_modified_since)
|
||||
{
|
||||
return Err(Error::NotModified);
|
||||
}
|
||||
|
||||
if let Some(if_match) = if_match {
|
||||
if let Some(etag) = &obj_info.etag {
|
||||
if !is_etag_equal(etag, if_match) {
|
||||
return Err(Error::PreconditionFailed);
|
||||
}
|
||||
} else {
|
||||
return Err(Error::PreconditionFailed);
|
||||
}
|
||||
}
|
||||
if has_valid_mod_time
|
||||
&& if_match.is_none()
|
||||
&& let Some(if_unmodified_since) = &pre.if_unmodified_since
|
||||
&& let Some(mod_time) = &obj_info.mod_time
|
||||
&& is_modified_since(mod_time, if_unmodified_since)
|
||||
{
|
||||
return Err(Error::PreconditionFailed);
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
state.check(self.http_preconditions.as_ref()).map_err(|err| match err {
|
||||
ObjectPreconditionError::InvalidPartNumber(part_number) => Error::InvalidPartNumber(part_number),
|
||||
ObjectPreconditionError::NotModified => Error::NotModified,
|
||||
ObjectPreconditionError::PreconditionFailed => Error::PreconditionFailed,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
fn is_etag_equal(etag1: &str, etag2: &str) -> bool {
|
||||
let e1 = etag1.trim_matches('"');
|
||||
let e2 = etag2.trim_matches('"');
|
||||
// Handle wildcard "*" - matches any ETag (per HTTP/1.1 RFC 7232)
|
||||
if e2 == "*" {
|
||||
return true;
|
||||
}
|
||||
e1 == e2
|
||||
}
|
||||
|
||||
fn is_modified_since(mod_time: &OffsetDateTime, given_time: &OffsetDateTime) -> bool {
|
||||
let mod_secs = mod_time.unix_timestamp();
|
||||
let given_secs = given_time.unix_timestamp();
|
||||
mod_secs > given_secs
|
||||
}
|
||||
|
||||
#[derive(Debug, Default)]
|
||||
pub struct ObjectInfo {
|
||||
pub bucket: String,
|
||||
|
||||
@@ -25,4 +25,5 @@ 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::{ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState};
|
||||
pub use object::{VersionMarker, WalkVersionsSortOrder};
|
||||
|
||||
@@ -43,6 +43,93 @@ pub struct ObjectLockRetentionOptions {
|
||||
pub bypass_governance: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct ObjectPreconditionPart {
|
||||
pub number: usize,
|
||||
pub exists: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, Default, Clone, Copy, PartialEq, Eq)]
|
||||
pub struct ObjectPreconditionState<'a> {
|
||||
pub etag: Option<&'a str>,
|
||||
pub mod_time: Option<OffsetDateTime>,
|
||||
pub requested_part: Option<ObjectPreconditionPart>,
|
||||
}
|
||||
|
||||
impl ObjectPreconditionState<'_> {
|
||||
pub fn check(self, preconditions: Option<&HTTPPreconditions>) -> Result<(), ObjectPreconditionError> {
|
||||
if let Some(part) = self.requested_part
|
||||
&& part.number > 1
|
||||
&& !part.exists
|
||||
{
|
||||
return Err(ObjectPreconditionError::InvalidPartNumber(part.number));
|
||||
}
|
||||
|
||||
let Some(preconditions) = preconditions else {
|
||||
return Ok(());
|
||||
};
|
||||
|
||||
let has_valid_mod_time = self.mod_time.is_some_and(|t| t != OffsetDateTime::UNIX_EPOCH);
|
||||
let if_none_match = preconditions.if_none_match_value();
|
||||
let if_match = preconditions.if_match_value();
|
||||
|
||||
if let Some(if_none_match) = if_none_match
|
||||
&& let Some(etag) = self.etag
|
||||
&& etag_matches(etag, if_none_match)
|
||||
{
|
||||
return Err(ObjectPreconditionError::NotModified);
|
||||
}
|
||||
|
||||
if has_valid_mod_time
|
||||
&& let Some(if_modified_since) = &preconditions.if_modified_since
|
||||
&& let Some(mod_time) = &self.mod_time
|
||||
&& !is_modified_since(mod_time, if_modified_since)
|
||||
{
|
||||
return Err(ObjectPreconditionError::NotModified);
|
||||
}
|
||||
|
||||
if let Some(if_match) = if_match {
|
||||
if let Some(etag) = self.etag {
|
||||
if !etag_matches(etag, if_match) {
|
||||
return Err(ObjectPreconditionError::PreconditionFailed);
|
||||
}
|
||||
} else {
|
||||
return Err(ObjectPreconditionError::PreconditionFailed);
|
||||
}
|
||||
}
|
||||
|
||||
if has_valid_mod_time
|
||||
&& if_match.is_none()
|
||||
&& let Some(if_unmodified_since) = &preconditions.if_unmodified_since
|
||||
&& let Some(mod_time) = &self.mod_time
|
||||
&& is_modified_since(mod_time, if_unmodified_since)
|
||||
{
|
||||
return Err(ObjectPreconditionError::PreconditionFailed);
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum ObjectPreconditionError {
|
||||
InvalidPartNumber(usize),
|
||||
NotModified,
|
||||
PreconditionFailed,
|
||||
}
|
||||
|
||||
impl fmt::Display for ObjectPreconditionError {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
match self {
|
||||
Self::InvalidPartNumber(part_number) => write!(f, "invalid part number {part_number}"),
|
||||
Self::NotModified => f.write_str("object not modified"),
|
||||
Self::PreconditionFailed => f.write_str("object precondition failed"),
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl std::error::Error for ObjectPreconditionError {}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
pub enum VersionMarker {
|
||||
Null,
|
||||
@@ -185,6 +272,16 @@ fn non_empty_condition_value(value: Option<&str>) -> Option<&str> {
|
||||
value.map(str::trim).filter(|value| !value.is_empty())
|
||||
}
|
||||
|
||||
fn etag_matches(object_etag: &str, condition_etag: &str) -> bool {
|
||||
let object_etag = object_etag.trim_matches('"');
|
||||
let condition_etag = condition_etag.trim_matches('"');
|
||||
condition_etag == "*" || object_etag == condition_etag
|
||||
}
|
||||
|
||||
fn is_modified_since(mod_time: &OffsetDateTime, given_time: &OffsetDateTime) -> bool {
|
||||
mod_time.unix_timestamp() > given_time.unix_timestamp()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::*;
|
||||
@@ -210,6 +307,69 @@ mod tests {
|
||||
assert!(!opts.bypass_governance);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_precondition_state_rejects_missing_requested_part() {
|
||||
let state = ObjectPreconditionState {
|
||||
requested_part: Some(ObjectPreconditionPart {
|
||||
number: 3,
|
||||
exists: false,
|
||||
}),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(state.check(None), Err(ObjectPreconditionError::InvalidPartNumber(3)));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_precondition_state_keeps_existing_etag_priority() {
|
||||
let preconditions = HTTPPreconditions {
|
||||
if_match: Some("\"other\"".to_owned()),
|
||||
if_none_match: Some("\"abc\"".to_owned()),
|
||||
..Default::default()
|
||||
};
|
||||
let state = ObjectPreconditionState {
|
||||
etag: Some("\"abc\""),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(state.check(Some(&preconditions)), Err(ObjectPreconditionError::NotModified));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_precondition_state_requires_etag_for_if_match() {
|
||||
let preconditions = HTTPPreconditions {
|
||||
if_match: Some("\"abc\"".to_owned()),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(
|
||||
ObjectPreconditionState::default().check(Some(&preconditions)),
|
||||
Err(ObjectPreconditionError::PreconditionFailed)
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn object_precondition_state_checks_modification_dates() {
|
||||
let mod_time = OffsetDateTime::UNIX_EPOCH + time::Duration::seconds(100);
|
||||
let preconditions = HTTPPreconditions {
|
||||
if_modified_since: Some(OffsetDateTime::UNIX_EPOCH + time::Duration::seconds(100)),
|
||||
..Default::default()
|
||||
};
|
||||
let state = ObjectPreconditionState {
|
||||
mod_time: Some(mod_time),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(state.check(Some(&preconditions)), Err(ObjectPreconditionError::NotModified));
|
||||
|
||||
let preconditions = HTTPPreconditions {
|
||||
if_unmodified_since: Some(OffsetDateTime::UNIX_EPOCH + time::Duration::seconds(99)),
|
||||
..Default::default()
|
||||
};
|
||||
|
||||
assert_eq!(state.check(Some(&preconditions)), Err(ObjectPreconditionError::PreconditionFailed));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn version_marker_parses_null_and_uuid_markers() {
|
||||
assert_eq!(VersionMarker::parse("null").expect("null marker should parse"), VersionMarker::Null);
|
||||
|
||||
@@ -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::{ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState};`
|
||||
- `pub use object::{VersionMarker, WalkVersionsSortOrder};`
|
||||
|
||||
ECStore must keep compile-time coverage for both `StorageAdminApi` and the
|
||||
|
||||
@@ -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-object-list-helper-contracts`
|
||||
- Baseline: `overtrue/arch-range-contracts` at `829bc62095f0fd853747cd6d7d77f8558baf24a2`
|
||||
over `origin/main` at `a9aba323c6512e6b99c3137258b97b6058075ce9`
|
||||
- Branch: `overtrue/arch-object-precondition-contracts`
|
||||
- Baseline: `overtrue/arch-object-list-helper-contracts` at `65b56e1fb4fa4c65c167e2f434bf5fcb2a0b0cd4f`
|
||||
over `overtrue/arch-range-contracts` at `829bc62095f0fd853747cd6d7d77f8558baf24a2`
|
||||
- PR type for this branch: `api-extraction`
|
||||
- Runtime behavior changes: no external behavior change expected.
|
||||
- Rust code changes: move the pure `VersionMarker` and
|
||||
`WalkVersionsSortOrder` list helper contracts into `rustfs-storage-api`,
|
||||
then keep ECStore list/walk implementations and filemeta adaptation at the
|
||||
ECStore boundary.
|
||||
- CI/script changes: extend migration guards for list helper public re-exports
|
||||
and reject restoring the old ECStore-owned list helper definitions or
|
||||
- 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 list helper contract extraction slice.
|
||||
- Docs changes: record the object precondition contract extraction slice.
|
||||
|
||||
## Phase 0 Tasks
|
||||
|
||||
@@ -599,6 +598,27 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
migration/layer guards, formatting, diff hygiene, Rust risk scan, and
|
||||
required three-expert review passed.
|
||||
|
||||
- [x] `API-018` Move object precondition helper contracts.
|
||||
- Completed slice: add `ObjectPreconditionState`,
|
||||
`ObjectPreconditionPart`, and `ObjectPreconditionError` to
|
||||
`rustfs-storage-api`; make ECStore `ObjectOptions::precondition_check`
|
||||
adapt `ObjectInfo` into the shared pure contract and map the contract
|
||||
result back to the existing ECStore errors.
|
||||
- Acceptance: `rustfs-storage-api` exports the precondition helper contracts,
|
||||
ECStore keeps `ObjectOptions` and `ObjectInfo`, and migration guards reject
|
||||
dropping the public precondition contract re-export.
|
||||
- Must preserve: requested-part validation, empty condition handling,
|
||||
`If-None-Match`/`If-Modified-Since` `NotModified` behavior,
|
||||
`If-Match`/`If-Unmodified-Since` `PreconditionFailed` behavior, wildcard
|
||||
ETag matching, and ECStore error mapping.
|
||||
- Risk defense: only pure precondition decision state and result contracts
|
||||
cross into `rustfs-storage-api`; ECStore keeps object metadata adaptation,
|
||||
storage error types, `ObjectOptions`, `ObjectInfo`, readers,
|
||||
lifecycle/replication coupling, and storage implementation bodies.
|
||||
- Verification: focused storage-api tests, ECStore/RustFS/downstream compile
|
||||
checks, migration/layer guards, formatting, diff hygiene, Rust risk scan,
|
||||
and required three-expert review passed.
|
||||
|
||||
## Phase 8 Background Controller Tasks
|
||||
|
||||
- [x] `BGC-001` Inventory background services.
|
||||
@@ -875,39 +895,43 @@ Status values: `[ ]` not started, `[~]` in progress, `[x]` complete, `[!]` block
|
||||
|
||||
| Expert | Status | Notes |
|
||||
|---|---|---|
|
||||
| Quality/architecture | passed | Pure object list helper contracts now live in `rustfs-storage-api`; implementation-heavy list/walk contracts and ECStore behavior stay in ECStore. |
|
||||
| Migration preservation | passed | The slice changes helper type ownership and import paths only; null version marker parsing, first-entry version marker behavior, walk sort default, and ECStore filemeta/list behavior are unchanged. |
|
||||
| Testing/verification | passed | Focused storage-api/ECStore/RustFS/downstream compile checks, migration/layer guards, formatting, diff hygiene, Rust risk scan, and full `make pre-commit` passed. |
|
||||
| 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. |
|
||||
|
||||
## 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.
|
||||
- `cargo fmt --all --check`: passed.
|
||||
- `git diff --check`: passed.
|
||||
- Rust risk scan: no new production `unwrap`/`expect`, panic/todo markers,
|
||||
`unsafe`, process-spawning calls, lossy casts, println/eprintln, or relaxed
|
||||
ordering in added Rust lines; new `expect` calls are test-only assertions.
|
||||
- `make pre-commit`: passed.
|
||||
- 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
|
||||
skipped, and doctests passed.
|
||||
|
||||
Notes:
|
||||
|
||||
- This slice follows the range helper contract branch and keeps the old
|
||||
aggregate facade, bucket DTO, multipart DTO, bucket operation contract, object
|
||||
helper, range helper, and list helper contract guards active.
|
||||
- The shared list marker/sort helper contracts are now owned by
|
||||
`rustfs-storage-api`; ECStore keeps implementation-heavy object/list result
|
||||
DTOs, walk options with filemeta filters, readers, lifecycle/replication, rio,
|
||||
filemeta, and storage error contracts.
|
||||
- The slice does not alter list, walk, object, multipart, or bucket runtime
|
||||
behavior.
|
||||
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.
|
||||
|
||||
## Handoff Notes
|
||||
|
||||
- List helper contract cleanup is stacked on the range helper contract branch.
|
||||
- Object precondition contract cleanup is stacked on the list helper contract
|
||||
branch.
|
||||
- After this lands, remaining storage work can continue by extracting larger
|
||||
low-coupling DTO slices or by narrowing remaining operation-group consumers.
|
||||
|
||||
@@ -196,6 +196,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::{ObjectPreconditionError, ObjectPreconditionPart, ObjectPreconditionState};" \
|
||||
"storage-api public object precondition contract re-export"
|
||||
require_source_line \
|
||||
"crates/storage-api/src/lib.rs" \
|
||||
"pub use object::{VersionMarker, WalkVersionsSortOrder};" \
|
||||
@@ -246,7 +250,7 @@ fi
|
||||
|
||||
(
|
||||
cd "$ROOT_DIR"
|
||||
rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:HTTPPreconditions|ObjectLockRetentionOptions)\b|::(?:HTTPPreconditions|ObjectLockRetentionOptions)\b)|pub struct (?:HTTPPreconditions|ObjectLockRetentionOptions)\b' \
|
||||
rg -n --no-heading 'pub(?:\(crate\))? use rustfs_storage_api(?:::\{[^}]*\b(?:HTTPPreconditions|ObjectLockRetentionOptions|ObjectPreconditionError|ObjectPreconditionPart|ObjectPreconditionState)\b|::(?:HTTPPreconditions|ObjectLockRetentionOptions|ObjectPreconditionError|ObjectPreconditionPart|ObjectPreconditionState)\b)|pub (?:enum ObjectPreconditionError|struct (?:HTTPPreconditions|ObjectLockRetentionOptions|ObjectPreconditionPart|ObjectPreconditionState))\b' \
|
||||
crates/ecstore/src/store_api.rs crates/ecstore/src/store_api/types.rs || true
|
||||
) >"$STORE_API_OBJECT_HELPER_REEXPORTS_FILE"
|
||||
|
||||
|
||||
Reference in New Issue
Block a user