refactor: client-first architecture, extended builder API, and CLI improvements#68
Merged
refactor: client-first architecture, extended builder API, and CLI improvements#68
Conversation
Reorganize the codebase around OneIo as the central type. Free-standing
functions (get_reader, download, etc.) are now thin wrappers over a shared
default OneIo client via default_oneio().
Module layout:
- Remove src/oneio/ sub-directory; all modules moved to src/
- Split client.rs (OneIo impl) and builder.rs (OneIoBuilder + default_oneio)
- Merge src/oneio/compressions/*.rs into single src/compression.rs
- Extract ProgressReader into src/progress.rs
- Extract async functions into src/async_reader.rs
OneIoBuilder additions:
- header(HeaderName, HeaderValue) and header_str(&str, &str) — infallible header API
- user_agent(HeaderValue) — infallible, previously returned Result
- configure_http(f) — escape hatch for any reqwest::blocking::ClientBuilder option
- timeout(), connect_timeout(), proxy(), no_proxy(), redirect()
- add_root_certificate_pem(), add_root_certificate_der()
- ONEIO_CA_BUNDLE env var support
OneIo additions:
- get_reader_with_type(path, compression) — explicit compression override
- from_client(Client) — construct from existing reqwest client
- download_with_retry() uses exponential backoff between attempts
Error type updates:
- OneIoError is now #[non_exhaustive]
- Add NetworkWithContext { url, source } for network errors with URL context
- Add InvalidHeader and InvalidCertificate variants
Bug fixes:
- Compression detection strips URL query params and fragments before
reading the file extension (e.g. file.gz?token=x now detects gz correctly)
- Replace Arc<ProgressBar> with ProgressBar::clone(); indicatif's
ProgressBar is already Clone + Send + Sync
- Remove double HEAD request for --download: get_content_length() was
called to gate the progress bar, then called again inside
get_reader_with_progress(); now always use the progress path on a terminal
- Align --download and s3 download progress behavior: both try the
progress bar on a terminal regardless of whether file size is known
- Progress bar no longer uses hidden/reveal dance; bar writes to stderr
immediately with a spinner when total size is unknown
- Fix parse_header to split on first ':' only, accepting both
"Name: Value" and "Name:Value" formats
- Document that --compression is ignored when --download is used
- Extract build_oneio() and s3_credentials_or_exit() helpers
- Shorten success messages ("uploaded to ...", "downloaded to ...")
- Fix get_reader_with_progress callback signature in README (Fn(u64, u64))
- Update README CLI examples to match actual output
Add 29 new tests covering compression formats, progress tracking, cache reader, JSON parsing, content-length detection, error variants, environment variables, SHA256 digest, and raw writer behavior. Bug fix: LZ4 compressed writes were silently truncated because lz4::Encoder has no Drop impl and requires an explicit finish() call to write the end-of-stream frame marker. Fixed with a Lz4Writer wrapper in compression.rs that calls finish() on drop. The write tests caught this regression. New tests: - LZ4/XZ/Zstd: read, write round-trip, get_reader_with_type override - Progress: callback fires with correct bytes/total on HTTP and local files - Cache reader: creation, reuse, force-refresh, nested directory creation - JSON: struct deserialization, invalid input returns error - Content length: local file via metadata, HTTP with Content-Length header - Error variants: InvalidCertificate (PEM + DER), network error display - Env vars: ONEIO_CA_BUNDLE (valid + missing path), ONEIO_ACCEPT_INVALID_CERTS - SHA256 digest: known hash assertion, missing file returns error - get_writer_raw: creates plain uncompressed file, creates parent directories
Removed cargo-readme workflow check as lib.rs and README now serve different audiences with distinct content. Updated copilot instructions to reflect separate maintenance model. Fixed Duration import to use full path in builder methods.
reqwest::Certificate::from_pem/from_der do not validate certificate data at parse time - they only validate when used in a TLS connection. These tests incorrectly assumed parsing would fail for invalid data, but the functions return Ok for any input and defer validation to connection time.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
OneIois now the central type; free-standing functions (get_reader,download, etc.) are thin wrappers over a shared default client viadefault_oneio()src/oneio/sub-directory; all modules now live atsrc/OneIoBuilder: addedtimeout,connect_timeout,proxy,no_proxy,redirect,configure_httpescape hatch,add_root_certificate_pem/der,header_str, andONEIO_CA_BUNDLEenv var supportOneIomethods:get_reader_with_type(explicit compression override),from_client,download_with_retrywith exponential backoffOneIoErroris now#[non_exhaustive]; addedNetworkWithContext,InvalidHeader,InvalidCertificatevariantslz4::Encoderhas noDropimpl and requires an explicitfinish()call; fixed with aLz4Writerwrapper-H/--header,--compression,s3 downloadsubcommand, download progress bar, fixed S3 upload syntaxBreaking changes
OneIoErroris now#[non_exhaustive]—matchwithout a wildcard_arm will not compileOneIoBuilder::header()now takes typedHeaderName/HeaderValue(infallible) instead of(K, V) -> Result<Self>OneIoBuilder::user_agent()now takes typedHeaderValue(infallible)oneio::download()no longer acceptsOption<reqwest::blocking::Client>oneio::remotemodule is nowpub(crate);create_client_with_headersis#[deprecated]ProgressReaderandProgressCallbackremoved from public APIOneIoinstance methods)Test plan
cargo test— default features (gz, bz, https)cargo test --all-features— all 51 tests passcargo clippy --all-features -- -D warnings— cleancargo build --no-default-features— clean