Skip to content

Commit 0f493c2

Browse files
authored
[PM-22589] Implement basic db migrations for indexeddb (#410)
## 🎟️ Tracking https://bitwarden.atlassian.net/browse/PM-22589 ## 📔 Objective Builds on top of #301 to add some basic state migration support for SDK-managed state. This is needed for IndexedDB as we can only run migrations there on version upgrades. ## ⏰ Reminders before review - Contributor guidelines followed - All formatters and local linters executed and passed - Written new unit and / or integration tests where applicable - Protected functional changes with optionality (feature flags) - Used internationalization (i18n) for all UI strings - CI builds passed - Communicated to DevOps any deployment requirements - Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team ## 🦮 Reviewer guidelines <!-- Suggested interactions but feel free to use (or not) as you desire! --> - 👍 (`:+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 confirmed issue 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
1 parent 745287f commit 0f493c2

File tree

16 files changed

+199
-111
lines changed

16 files changed

+199
-111
lines changed

Cargo.lock

Lines changed: 10 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ bitwarden-send = { path = "crates/bitwarden-send", version = "=1.0.0" }
3838
bitwarden-sm = { path = "bitwarden_license/bitwarden-sm", version = "=1.0.0" }
3939
bitwarden-ssh = { path = "crates/bitwarden-ssh", version = "=1.0.0" }
4040
bitwarden-state = { path = "crates/bitwarden-state", version = "=1.0.0" }
41+
bitwarden-state-migrations = { path = "crates/bitwarden-state-migrations", version = "=1.0.0" }
4142
bitwarden-test = { path = "crates/bitwarden-test", version = "=1.0.0" }
4243
bitwarden-threading = { path = "crates/bitwarden-threading", version = "=1.0.0" }
4344
bitwarden-uniffi-error = { path = "crates/bitwarden-uniffi-error", version = "=1.0.0" }

crates/bitwarden-core/src/platform/state_client.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use std::sync::Arc;
22

33
use bitwarden_state::{
44
registry::{RepositoryNotFoundError, StateRegistryError},
5-
repository::{Repository, RepositoryItem, RepositoryItemData},
5+
repository::{Repository, RepositoryItem, RepositoryMigrations},
66
DatabaseConfiguration,
77
};
88

@@ -36,12 +36,12 @@ impl StateClient {
3636
pub async fn initialize_database(
3737
&self,
3838
configuration: DatabaseConfiguration,
39-
repositories: Vec<RepositoryItemData>,
39+
migrations: RepositoryMigrations,
4040
) -> Result<(), StateRegistryError> {
4141
self.client
4242
.internal
4343
.repository_map
44-
.initialize_database(configuration, repositories)
44+
.initialize_database(configuration, migrations)
4545
.await
4646
}
4747

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
[package]
2+
name = "bitwarden-state-migrations"
3+
version.workspace = true
4+
authors.workspace = true
5+
edition.workspace = true
6+
rust-version.workspace = true
7+
homepage.workspace = true
8+
repository.workspace = true
9+
license-file.workspace = true
10+
keywords.workspace = true
11+
12+
[features]
13+
uniffi = ["bitwarden-vault/uniffi"]
14+
wasm = ["bitwarden-vault/wasm"]
15+
16+
[dependencies]
17+
bitwarden-state = { workspace = true }
18+
bitwarden-vault = { workspace = true }
19+
20+
[lints]
21+
workspace = true
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# bitwarden-state-migrations
2+
3+
This crate contains migrations for the SDK-managed state framework. It should only be imported by
4+
the final application crates (`bitwarden-wasm-internal` and `bitwarden-uniffi`)
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#![doc = include_str!("../README.md")]
2+
3+
use bitwarden_state::repository::{RepositoryItem, RepositoryMigrationStep, RepositoryMigrations};
4+
use bitwarden_vault::{Cipher, Folder};
5+
6+
/// Returns a list of all SDK-managed repository migrations.
7+
pub fn get_sdk_managed_migrations() -> RepositoryMigrations {
8+
use RepositoryMigrationStep::*;
9+
RepositoryMigrations::new(vec![
10+
// Add any new migrations here. Note that order matters, and that removing a repository
11+
// requires a separate migration step using `Remove(...)`.
12+
Add(Cipher::data()),
13+
Add(Folder::data()),
14+
])
15+
}

crates/bitwarden-state/README.md

Lines changed: 15 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -182,65 +182,23 @@ getClient(userId = userId).platform().store().registerCipherStore(CipherStoreImp
182182

183183
With `SDK-Managed State`, the SDK will be exclusively responsible for the data storage. This means
184184
that the clients don't need to make any changes themselves, as the implementation is internal to the
185-
SDK. To add support for an SDK managed `Repository`, it needs to be added to the initialization code
186-
for WASM and UniFFI. This example shows how to add support for `Cipher`s.
185+
SDK. To add support for an SDK managed `Repository`, a new migration step needs to be added to the
186+
`bitwarden-state-migrations` crate.
187187

188-
### How to initialize SDK-Managed State on WASM
188+
### How to initialize SDK-Managed State
189189

190-
Go to `crates/bitwarden-wasm-internal/src/platform/mod.rs` and add a line with your type, as shown
191-
below. In this example we're registering `Cipher` as both client and SDK managed to show how both
192-
are done, but you can also just do one or the other.
190+
Go to `crates/bitwarden-state-migrations/src/lib.rs` and add a line with your type, as shown below.
191+
In this example we're registering `Cipher` and `Folder` as SDK managed.
193192

194193
```rust,ignore
195-
pub async fn initialize_state(
196-
&self,
197-
cipher_repository: CipherRepository,
198-
) -> Result<(), bitwarden_state::registry::StateRegistryError> {
199-
let cipher = cipher_repository.into_channel_impl();
200-
// Register the provided repository as client managed state
201-
self.0.platform().state().register_client_managed(cipher);
202-
203-
let sdk_managed_repositories = vec![
204-
// This should list all the SDK-managed repositories
205-
<Cipher as RepositoryItem>::data(),
206-
// Add your type here
207-
];
208-
209-
self.0
210-
.platform()
211-
.state()
212-
.initialize_database(sdk_managed_repositories)
213-
.await
214-
}
215-
```
216-
217-
### How to initialize SDK-Managed State on UniFFI
218-
219-
Go to `crates/bitwarden-uniffi/src/platform/mod.rs` and add a line with your type, as shown below.
220-
In this example we're registering `Cipher` as both client and SDK managed to show how both are done,
221-
but you can also just do one or the other.
222-
223-
```rust,ignore
224-
pub async fn initialize_state(
225-
&self,
226-
cipher_repository: Arc<dyn CipherRepository>,
227-
) -> Result<()> {
228-
let cipher = UniffiRepositoryBridge::new(cipher_repository);
229-
// Register the provided repository as client managed state
230-
self.0.platform().state().register_client_managed(cipher);
231-
232-
let sdk_managed_repositories = vec![
233-
// This should list all the SDK-managed repositories
234-
<Cipher as RepositoryItem>::data(),
235-
// Add your type here
236-
];
237-
238-
self.0
239-
.platform()
240-
.state()
241-
.initialize_database(sdk_managed_repositories)
242-
.await
243-
.map_err(Error::StateRegistry)?;
244-
Ok(())
245-
}
194+
/// Returns a list of all SDK-managed repository migrations.
195+
pub fn get_sdk_managed_migrations() -> RepositoryMigrations {
196+
use RepositoryMigrationStep::*;
197+
RepositoryMigrations::new(vec![
198+
// Add any new migrations here. Note that order matters, and that removing a repository
199+
// requires a separate migration step using `Remove(...)`.
200+
Add(Cipher::data()),
201+
Add(Folder::data()),
202+
])
203+
}
246204
```

crates/bitwarden-state/src/registry.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use serde::{de::DeserializeOwned, Serialize};
99
use thiserror::Error;
1010

1111
use crate::{
12-
repository::{Repository, RepositoryItem, RepositoryItemData},
12+
repository::{Repository, RepositoryItem, RepositoryItemData, RepositoryMigrations},
1313
sdk_managed::{Database, DatabaseConfiguration, SystemDatabase},
1414
};
1515

@@ -71,19 +71,19 @@ impl StateRegistry {
7171
pub async fn initialize_database(
7272
&self,
7373
configuration: DatabaseConfiguration,
74-
repositories: Vec<RepositoryItemData>,
74+
migrations: RepositoryMigrations,
7575
) -> Result<(), StateRegistryError> {
7676
if self.database.get().is_some() {
7777
return Err(StateRegistryError::DatabaseAlreadyInitialized);
7878
}
7979
let _ = self
8080
.database
81-
.set(SystemDatabase::initialize(configuration, &repositories).await?);
81+
.set(SystemDatabase::initialize(configuration, migrations.clone()).await?);
8282

8383
*self
8484
.sdk_managed
8585
.write()
86-
.expect("RwLock should not be poisoned") = repositories.clone();
86+
.expect("RwLock should not be poisoned") = migrations.into_repository_items();
8787

8888
Ok(())
8989
}

crates/bitwarden-state/src/repository.rs

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,51 @@ pub const fn validate_registry_name(name: &str) -> bool {
100100
true
101101
}
102102

103+
/// Represents a set of migrations for multiple repositories in a database migration process.
104+
#[derive(Debug, Clone)]
105+
pub struct RepositoryMigrations {
106+
pub(crate) steps: Vec<RepositoryMigrationStep>,
107+
// This is used only by indexedDB
108+
#[allow(dead_code)]
109+
pub(crate) version: u32,
110+
}
111+
112+
/// Represents a single step for a repository in a database migration process.
113+
#[derive(Debug, Clone, Copy)]
114+
pub enum RepositoryMigrationStep {
115+
/// Add a new repository.
116+
Add(RepositoryItemData),
117+
/// Remove an existing repository.
118+
Remove(RepositoryItemData),
119+
}
120+
121+
impl RepositoryMigrations {
122+
/// Create a new `RepositoryMigrations` with the given steps. The version is derived from the
123+
/// number of steps.
124+
pub fn new(steps: Vec<RepositoryMigrationStep>) -> Self {
125+
Self {
126+
version: steps.len() as u32,
127+
steps,
128+
}
129+
}
130+
131+
/// Converts the migration steps into a list of unique repository item data.
132+
pub fn into_repository_items(self) -> Vec<RepositoryItemData> {
133+
let mut map = std::collections::HashMap::new();
134+
for step in self.steps {
135+
match step {
136+
RepositoryMigrationStep::Add(data) => {
137+
map.insert(data.type_id, data);
138+
}
139+
RepositoryMigrationStep::Remove(data) => {
140+
map.remove(&data.type_id);
141+
}
142+
}
143+
}
144+
map.into_values().collect()
145+
}
146+
}
147+
103148
/// Register a type for use in a repository. The type must only be registered once in the crate
104149
/// where it's defined. The provided name must be unique and not be changed.
105150
#[macro_export]

crates/bitwarden-state/src/sdk_managed/indexed_db.rs

Lines changed: 17 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1+
use indexed_db::Error;
12
use js_sys::JsString;
23
use serde::{de::DeserializeOwned, ser::Serialize};
34

45
use crate::{
5-
repository::{RepositoryItem, RepositoryItemData},
6+
repository::{RepositoryItem, RepositoryMigrationStep, RepositoryMigrations},
67
sdk_managed::{Database, DatabaseConfiguration, DatabaseError},
78
};
89

@@ -22,27 +23,32 @@ pub struct IndexedDbDatabase(
2223
impl Database for IndexedDbDatabase {
2324
async fn initialize(
2425
configuration: DatabaseConfiguration,
25-
registrations: &[RepositoryItemData],
26+
migrations: RepositoryMigrations,
2627
) -> Result<Self, DatabaseError> {
2728
let DatabaseConfiguration::IndexedDb { db_name } = configuration else {
2829
return Err(DatabaseError::UnsupportedConfiguration(configuration));
2930
};
3031

3132
let factory = indexed_db::Factory::get()?;
3233

33-
let registrations = registrations.to_vec();
34-
35-
// TODO: This version will be replaced by a proper migration system in a followup PR:
36-
// https://github.com/bitwarden/sdk-internal/pull/410
37-
let version: u32 = 1;
38-
3934
// Open the database, creating it if needed
4035
let db = factory
41-
.open(&db_name, version, async move |evt| {
36+
.open(&db_name, migrations.version, async move |evt| {
4237
let db = evt.database();
4338

44-
for reg in registrations {
45-
db.build_object_store(reg.name()).create()?;
39+
for step in &migrations.steps {
40+
match step {
41+
RepositoryMigrationStep::Add(data) => {
42+
db.build_object_store(data.name()).create()?;
43+
}
44+
RepositoryMigrationStep::Remove(data) => {
45+
match db.delete_object_store(data.name()) {
46+
// If the store doesn't exist, we can ignore the error
47+
Ok(_) | Err(Error::DoesNotExist) => {}
48+
Err(e) => return Err(e),
49+
}
50+
}
51+
}
4652
}
4753

4854
Ok(())

0 commit comments

Comments
 (0)