[PM-39197] - improve vault sync performance

[jaasen-livefront]

🎟️ Tracking

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

📔 Objective

Reduce vault sync latency for users with very large direct collection assignments.

This PR optimizes the SQL Server collection-access query path used by sync by updating UserCollectionDetails to avoid the expensive join/OR shape that was causing high latency at scale.

Outcome from local verification on the same seeded dataset:

• Before: /sync?excludeDomains=true around 21s
• After: /sync?excludeDomains=true around 426ms

🔧 Performance approach

The sync bottleneck was isolated to collection access expansion in SQL Server (Collection_ReadByUserId via UserCollectionDetails), not API serialization or cipher loading.

To improve performance, UserCollectionDetails was reshaped from a broad left-join + OR filter pattern into two explicit branches:

1. Direct user-to-collection access branch (CollectionUser)
2. Group-derived access branch (GroupUser + CollectionGroup) with exclusion of directly-assigned rows

This reduces unnecessary row expansion and gives SQL Server a cleaner execution path for large collection cardinalities.

✅ Measured impact (same dataset)

• Before: /sync?excludeDomains=true ≈ 21s
• After: /sync?excludeDomains=true ≈ 426ms
• Approximate improvement: ~50x faster

📸 Screenshots

Not applicable (no UI changes).

[github-actions]

🤖 Bitwarden Claude Code Review

Overall Assessment: APPROVE

Reviewed the UserCollectionDetails SQL Server function refactor that reshapes the broad left-join + OR filter into two explicit UNION ALL branches (direct CollectionUser access and group-derived CollectionGroup access, with directly-assigned collections excluded from the group branch). Verified semantic equivalence with the prior implementation across the key authorization cases — direct-access precedence over group access, group-only multi-group permission aggregation, and the combined direct + group case — all of which are preserved by the CU.[CollectionId] IS NULL anti-join and the existing MIN/MIN/MAX aggregation in Collection_ReadByUserId. The SSDT source (src/Sql/dbo/...) and the dated migration match, and the function's return shape and only consumer remain unchanged, so no _V2 versioning or EF Core parity change is required.

Code Review Details

No blocking findings.

• 🎨 : This change reshapes authorization-resolution SQL but adds no integration test asserting the combined group + direct-access case resolves identically to the prior behavior. Consider a [DatabaseData] test in CollectionRepositoryTests covering a user with both a direct CollectionUser and group CollectionGroup assignment to the same collection to lock in the equivalence.
  • test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.96%. Comparing base (e8917a8) to head (4b0e0c8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7829      +/-   ##
==========================================
+ Coverage   62.50%   66.96%   +4.45%     
==========================================
  Files        2301     2300       -1     
  Lines      100156   100000     -156     
  Branches     9027     8992      -35     
==========================================
+ Hits        62605    66965    +4360     
+ Misses      35369    30769    -4600     
- Partials     2182     2266      +84     

☔ 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.

[sonarqubecloud]

Quality Gate passed

Issues
3 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
96.7% Duplication on New Code

See analysis details on SonarQube Cloud

[r-tome]

This is really great! The query changes don't seem to change the existing query logic.

Non-blocker: I'd love to see some tests for GetManyByUserIdAsync added to https://github.com/bitwarden/server/blob/main/test/Infrastructure.IntegrationTest/AdminConsole/Repositories/CollectionRepository/CollectionRepositoryTests.cs

[mkincaid-bw]

LGTM

[jaasen-livefront]

@mkincaid-bw @r-tome Had to bump the migration file if you wouldn't mind re-approving. Thanks!

[jaasen-livefront]

@mkincaid-bw Looks like a flakey test. Any idea why this may have cropped up?