refactor(client): Introduce ClientBuilder pattern to fix constructor telescoping#902
refactor(client): Introduce ClientBuilder pattern to fix constructor telescoping#902addisonbeck wants to merge 6 commits intomainfrom
Conversation
Introduces ClientBuilder in crates/bitwarden-core/src/client/builder.rs with new(), with_settings(), with_token_handler(), and build() methods. The build() body is the exact logic from Client::new_internal, moved here as the first step of the builder pattern refactor (PM-34617). Includes Default impl and 4 unit tests. Helper functions new_http_client_builder() and build_default_headers() moved from client.rs into this file. Module not yet wired into mod.rs — that happens in Commit 3.
Client::new() and Client::new_with_token_handler() are now thin wrappers delegating to ClientBuilder::build(). Adds Client::builder() factory method. Removes Client::new_internal() entirely. Wires pub mod builder into client/mod.rs so ClientBuilder is resolvable. Part of PM-34617 builder pattern refactor.
Adds pub use builder::ClientBuilder in client/mod.rs and includes
ClientBuilder in the pub use client::{...} re-export in lib.rs.
ClientBuilder is now importable as bitwarden_core::ClientBuilder.
All 4 builder unit tests now compile and pass. Part of PM-34617.
Introduces PasswordManagerClientBuilder in crates/bitwarden-pm/src/builder.rs with new(), with_settings(), build(), and Default impl. Delegates to ClientBuilder::new().with_token_handler(PasswordManagerTokenHandler::default()). Includes 2 unit tests. Module not yet wired into lib.rs — that happens in the next commit. Part of PM-34617.
|
New Issues (1)Checkmarx found the following issues in this Pull Request
|
🔍 SDK Breaking Change Detection ResultsSDK Version:
Breaking change detection completed. View SDK workflow |
The project enforces #![warn(missing_docs)] escalated to errors via RUSTFLAGS=-D warnings. All new public items introduced by the ClientBuilder refactor (PR #902) were missing doc comments, causing 10 CI job failures across all platforms (build + clippy). Adds /// doc comments to: - ClientBuilder struct and its new(), with_settings(), with_token_handler(), and build() methods - pub mod builder declaration in client/mod.rs - Client::builder() factory method - PasswordManagerClientBuilder struct and its new(), with_settings(), and build() methods - PasswordManagerClient::builder() factory method Refs: PM-34617
Bitwarden Claude Code ReviewOverall Assessment: APPROVE This PR introduces a |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #902 +/- ##
=======================================
Coverage 82.45% 82.46%
=======================================
Files 356 358 +2
Lines 42817 42893 +76
=======================================
+ Hits 35306 35370 +64
- Misses 7511 7523 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
The breaking change reported here is unrelated, typechecking on clients is failing on main. |
The project enforces #![warn(missing_docs)] escalated to errors via RUSTFLAGS=-D warnings. All new public items introduced by the ClientBuilder refactor (PR #902) were missing doc comments, causing 10 CI job failures across all platforms (build + clippy). Adds /// doc comments to: - ClientBuilder struct and its new(), with_settings(), with_token_handler(), and build() methods - pub mod builder declaration in client/mod.rs - Client::builder() factory method - PasswordManagerClientBuilder struct and its new(), with_settings(), and build() methods - PasswordManagerClient::builder() factory method Refs: PM-34617
Co-authored-by: Daniel García <dani-garcia@users.noreply.github.com>
|
2 participants
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-34617
📔 Objective
We've got multiple in flight SDK features where part of the design is to create new constructors for clients with different options for provided args.
This PR sets up a builder pattern to replace these. I've left the old constructors and have not marked them deprecated, but all new work will follow this new pattern.