feat: adapt to s3s typed etag support #653
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
@Nugine Please review these changes. |
There was a problem hiding this comment.
Pull Request Overview
This PR adapts the codebase to use s3s library's native typed ETag support, eliminating manual ETag quoting that was previously implemented. The update bumps s3s to a specific commit that provides RFC 7232-compliant entity tag formatting and refactors the DNS resolution utilities to support testable mock resolvers.
- Updates s3s dependency to use typed ETag support from upstream
- Refactors ETag handling to use strongly-typed
s3s::dto::ETaginstead of manual string formatting - Adds injectable DNS resolver functionality for improved test reliability
Reviewed Changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| Cargo.toml | Updates s3s dependency to specific git commit with typed ETag support |
| crates/ecstore/src/client/object_api_utils.rs | Refactors ETag functions to return/handle typed ETag instead of quoted strings |
| crates/ecstore/src/store_api.rs | Adds conversion from typed ETag back to quoted string format for backward compatibility |
| crates/utils/src/net.rs | Adds mock DNS resolver infrastructure for testing and refactors DNS resolution logic |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| pub fn extract_etag(metadata: &HashMap<String, String>) -> String { | ||
| let etag = if let Some(etag) = metadata.get("etag") { | ||
| etag.clone() | ||
| } else { | ||
| metadata["md5Sum"].clone() | ||
| }; | ||
| format_etag(&etag) | ||
| metadata | ||
| .get("etag") | ||
| .cloned() | ||
| .or_else(|| metadata.get("md5Sum").cloned()) | ||
| .unwrap_or_default() | ||
| } |
There was a problem hiding this comment.
[nitpick] The extract_etag function now returns the raw stored string instead of a formatted ETag, but the function name suggests it should extract and format an ETag. Consider renaming to get_raw_etag or get_stored_etag to clarify that it returns the unprocessed value.
2 participants
Summary
s3sto commit f76013a52de2c8d44f6aa43182f1f1a938d2ad03 (upstream PR feat(s3s): typed etag s3s-project/s3s#349) so the SDK natively formats RFC 7232 entity tags; this supersedes the manual quoting we added in Fix ETag format to comply with HTTP standards by wrapping with quotes #592 and fix: normalize ETag comparison in multipart upload and replication #627crates/ecstore/src/client/object_api_utils.rs:format_etagnow returns a typeds3s::dto::ETag, handling both strong ("abc") and weak (W/"abc") tags without double quotingextract_etagkeeps the existing precedence (etagmetadata →md5Sum) but leaves the stored string untouched; the caller receives the typed ETagcrates/ecstore/src/store_api.rsto preserve backward compatibility:s3s::dto::CompletedPart, convertETag::Strong/Weakback to the quoted string format expected by the current metadata/persistence code pathsset_mock_dns_resolver/reset_dns_resolver) incrates/utils/src/net.rsType of Change
Related Issues
s3s-project/s3s#349, #592 and #627
Summary of Changes
Checklist
make pre-commitImpact
Additional Notes
Thank you for your contribution! Please ensure your PR follows the community standards (CODE_OF_CONDUCT.md) and sign the CLA if this is your first contribution.