Skip to content

Conversation

dani-garcia
Copy link
Member

🎟️ 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

  • 👍 (:+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

Copy link
Contributor

github-actions bot commented Aug 25, 2025

Logo
Checkmarx One – Scan Summary & Detailse4710954-8b58-4a11-a0c6-9ffc1fb48ca9

Great job! No new security vulnerabilities introduced in this pull request

Copy link

codecov bot commented Aug 25, 2025

Codecov Report

❌ Patch coverage is 53.84615% with 36 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.26%. Comparing base (745287f) to head (748fa09).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
crates/bitwarden-state/src/repository.rs 33.33% 12 Missing ⚠️
crates/bitwarden-state-migrations/src/lib.rs 0.00% 8 Missing ⚠️
crates/bitwarden-wasm-internal/src/platform/mod.rs 0.00% 5 Missing ⚠️
crates/bitwarden-state/src/sdk_managed/sqlite.rs 90.00% 4 Missing ⚠️
crates/bitwarden-state/src/registry.rs 0.00% 3 Missing ⚠️
crates/bitwarden-core/src/platform/state_client.rs 0.00% 2 Missing ⚠️
crates/bitwarden-uniffi/src/platform/mod.rs 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #410      +/-   ##
==========================================
- Coverage   78.27%   78.26%   -0.02%     
==========================================
  Files         274      275       +1     
  Lines       27394    27441      +47     
==========================================
+ Hits        21444    21476      +32     
- Misses       5950     5965      +15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@dani-garcia dani-garcia requested a review from coroiu August 25, 2025 15:18
@dani-garcia dani-garcia marked this pull request as ready for review August 25, 2025 15:18
@dani-garcia dani-garcia requested a review from a team as a code owner August 25, 2025 15:18
@dani-garcia dani-garcia force-pushed the ps/state-impl-migrations branch from b67679a to dd0f274 Compare August 26, 2025 10:35
dani-garcia added a commit that referenced this pull request Sep 4, 2025
## 🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-12612

## 📔 Objective

A continuation of #213,
this PR implements some simple SDK managed data store based on SQLite
(on non-wasm) and IndexedDB (on wasm).

Both databases are wrapped in a `Database` trait and conditionally
compiled based on platform. Then from each database we can get multiple
`Repository` implementations that can be used to read and write data
persistently. Each repository is mapped to a separate table in SQLite
and Object Store in IndexedDb.

Some limitations of the current system:
- The database currently needs to be initialized as a separate step to
both the SdkClient and the client-managed repositories, I feel like
ideally we can do that as part of Client initialization, but that would
require us to make initialization async. I've left that for the future.
- Currently we don't have any indexes beyond the main key, but both
sqlite and indexeddb support adding more.
- The current structure doesn't have any migration capabilities, which
are required for indexeddb to create the object stores, this PR
temporarily adds version numbers to the `register_repository_item`
calls, just to get something working, but note that this is removed in
#410 and replaced with a
better migration system.

## ⏰ 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

---------

Co-authored-by: Andreas Coroiu <[email protected]>
Base automatically changed from ps/state-impl to main September 4, 2025 09:52
@dani-garcia dani-garcia force-pushed the ps/state-impl-migrations branch from dd0f274 to 6e65633 Compare September 4, 2025 10:15
Copy link

@dani-garcia dani-garcia merged commit 0f493c2 into main Sep 17, 2025
50 checks passed
@dani-garcia dani-garcia deleted the ps/state-impl-migrations branch September 17, 2025 12:06
bw-ghapp bot pushed a commit to bitwarden/sdk-swift that referenced this pull request Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants