-
Notifications
You must be signed in to change notification settings - Fork 11
[PM-19479] Client-managed Repository traits #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
==========================================
- Coverage 71.77% 71.20% -0.58%
==========================================
Files 224 230 +6
Lines 18403 18632 +229
==========================================
+ Hits 13208 13266 +58
- Misses 5195 5366 +171 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Great job, no security vulnerabilities found in this Pull Request |
5f13cc1
to
778bc50
Compare
778bc50
to
34daa77
Compare
Implement demo cipher repository
34daa77
to
9d3ce7f
Compare
# Conflicts: # Cargo.lock # crates/bitwarden-uniffi/Cargo.toml # crates/bitwarden-vault/src/vault_client.rs # crates/bitwarden-wasm-internal/Cargo.toml # crates/bitwarden-wasm-internal/src/client.rs # crates/bitwarden-wasm-internal/src/vault/mod.rs
# Conflicts: # Cargo.lock # crates/bitwarden-wasm-internal/Cargo.toml # crates/bitwarden-wasm-internal/src/client.rs
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Vault ✅
🎟️ Tracking
https://bitwarden.atlassian.net/browse/PM-19479
📔 Objective
Implement a generic trait for accessing the client application's data storage directly. Because we want the store access to be typed, but
bitwarden_core
isn't aware of the models,bitwarden_core
only implements a generic way to set and retrieve genericimpl Repository<T>
instances, somewhat like dependency injection. Then it's up to each team/feature crates to define which models and which stores are available.This feature is created in a new
bitwarden-state
, which will be expanded by a separate PR with the addition of SDK-managed state (Sqlite+IndexedDB). At the moment this crate contains:Repository
trait which will be implemented by the clients (and the SDK in the future), and aRepositoryItem
marker trait which will be used to mark which models are meant to be used with the repositories.StateRegistry
which stores all the client-managed Repositories, and in the future will also handle the SDK-managed repositories.StateClient
subclient under platform that will be used by the applications to register their Repositories. Both the WASM and UniFFI crates also need some conversion code to implement theRepository
traits. I've tried to simplify it as much as possible, and hide it behind a macro when that wasn't possible.Some limitations on the current design:
UserKeyDefinition<Record<string, T>>
to match the key-value pattern inRepository
. This usually matches with what is being used for vault (encrypted ciphers/folders, etc), but it might fall short on other domains, like profile data, or user keys.inventory
crate to keep a global list of all the existing implementations and then validating that they've all been registered, but that doesn't work for WASM. We might be able to use theinventory
crate and just run it in tests though.For some documentation on how to use it, you can check the README: https://github.com/bitwarden/sdk-internal/blob/ps/state-traits/crates/bitwarden-state/README.md
The continuation of this is in #301, which contains SDK managed repository support. The client implementation of both these PRs is in bitwarden/clients#14839
⏰ Reminders before review
team
🦮 Reviewer guidelines
:+1:
) or similar for great changes:memo:
) or ℹ️ (:information_source:
) for notes or general info:question:
) for questions:thinking:
) or 💭 (:thought_balloon:
) for more open inquiry that's not quite a confirmedissue and could potentially benefit from discussion
:art:
) for suggestions / improvements:x:
) or:warning:
) for more significant problems or concerns needing attention:seedling:
) or ♻️ (:recycle:
) for future improvements or indications of technical debt:pick:
) for minor or nitpick changes