Skip to content

PM-42388: chore: Separate Cipher flows from VaultRepository - #7297

Open
david-livefront wants to merge 1 commit into
PM-42381-decouple-orgsfrom
PM-42388-decouple-cipher-flows
Open

PM-42388: chore: Separate Cipher flows from VaultRepository#7297
david-livefront wants to merge 1 commit into
PM-42381-decouple-orgsfrom
PM-42388-decouple-cipher-flows

Conversation

@david-livefront

@david-livefront david-livefront commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

🎟️ Tracking

PM-42388

📔 Objective

This PR moves the Cipher flows from the VaultRepository to its own specialized manager. This is needed to avoid a circular dependency in the OrganizationEventManager since we will need this manager in new places in the future.

@david-livefront
david-livefront requested a review from a team as a code owner August 24, 2026 19:35
@github-actions github-actions Bot added app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt labels Aug 24, 2026
@codecov

codecov Bot commented Aug 24, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 85.84906% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.62%. Comparing base (91e3624) to head (c5c6c02).

Files with missing lines Patch % Lines
...twarden/data/vault/manager/VaultDataManagerImpl.kt 86.40% 9 Missing and 5 partials ⚠️
...form/manager/event/OrganizationEventManagerImpl.kt 50.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                    Coverage Diff                     @@
##           PM-42381-decouple-orgs    #7297      +/-   ##
==========================================================
+ Coverage                   86.32%   86.62%   +0.30%     
==========================================================
  Files                         926      927       +1     
  Lines                       66715    66628      -87     
  Branches                     9877     9878       +1     
==========================================================
+ Hits                        57592    57718     +126     
+ Misses                       5647     5433     -214     
- Partials                     3476     3477       +1     
Flag Coverage Δ
app-data 17.58% <85.84%> (-0.12%) ⬇️
app-ui-auth-tools 18.98% <0.00%> (+0.02%) ⬆️
app-ui-platform 16.68% <0.00%> (+0.30%) ⬆️
app-ui-vault 27.83% <0.00%> (+0.03%) ⬆️
authenticator 6.07% <0.00%> (+0.01%) ⬆️
lib-core-network-bridge 4.08% <0.00%> (-0.01%) ⬇️
lib-data-ui 1.20% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 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.

@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR extracts the cipher/folder/send/list-item state flows and deleteVaultData out of VaultRepository into a new VaultDataManager, and repoints OrganizationEventManagerImpl at the narrower dependency to break the future circular-dependency risk. The extraction is a faithful move: VaultRepository still extends VaultDataManager and VaultRepositoryImpl delegates via VaultDataManager by vaultDataManager, so no call sites change behavior. I traced the new Hilt graph (VaultDataManagerCipherManager, VaultDiskSource, VaultSyncManager) and confirmed none of those depend back on VaultRepository or OrganizationEventManager, so no new cycle is introduced. Tests were relocated with parity into VaultDataManagerTest, and the Codecov delta is a reporting artifact from missing HEAD uploads rather than a real coverage regression.

Code Review Details
  • ♻️ : Carried over from the original implementation — the .find { it.id == itemId } result is discarded by the following .let { cipherManager.getCipher(...) }, so the list scan is wasted work and getCipher is invoked even when the item is absent from the synced list. Behavior is unchanged by this PR, so no action needed here; noting it since the lines were re-authored in a new file.
    • app/src/main/kotlin/com/x8bit/bitwarden/data/vault/manager/VaultDataManagerImpl.kt:159-163

@david-livefront
david-livefront force-pushed the PM-42388-decouple-cipher-flows branch from 1fbeb6c to c5c6c02 Compare August 24, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

app:password-manager Bitwarden Password Manager app context t:tech-debt Change Type - Tech debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant