[PM-32069] Add ExtendedProviderAbilityCacheService by JimmyVo16 · Pull Request #7447 · bitwarden/server

JimmyVo16 · 2026-04-10T19:45:11Z

🎟️ Tracking

https://bitwarden.atlassian.net/browse/PM-32069
https://bitwarden.atlassian.net/browse/PM-32112

📔 Objective

Add extended cache for the provider service (IProviderAbilityCacheService).
Inject IProviderAbilityCacheService into FeatureRoutedCacheService behind a feature flag.
Add unit tests.

📸 Screenshots

This code isn't wired up yet, so we can't manually test it, but I feel like the automated test coverage is sufficient.

github-actions · 2026-04-10T20:02:20Z

Checkmarx One – Scan Summary & Details – 8903f213-00a0-4634-853e-e308455ba11d

Fixed Issues (3)

Great job! The following issues were fixed in this Pull Request

Severity	Issue	Source File / Package
	~~CSRF~~	src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 145
	~~CSRF~~	src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 97
	~~CSRF~~	src/Api/KeyManagement/Controllers/AccountsKeyManagementController.cs: 173

github-actions · 2026-04-10T21:17:05Z

Bitwarden Claude Code Review

Overall Assessment: APPROVE

This PR adds ExtendedProviderAbilityCacheService behind the ProviderAbilityExtendedCache feature flag, mirroring the existing organization ability cache pattern. The implementation includes a new IProviderAbilityCacheService interface, a DI registration extension method, feature flag routing in FeatureRoutedCacheService, and both unit and integration tests. The code follows established codebase patterns and provides appropriate test coverage for all three routed operations (get, upsert, delete).

Code Review Details

No new findings. The active discussion threads on singleton vs scoped registration and cache key formatting are already being addressed by the author and reviewers.

codecov · 2026-04-10T21:36:40Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.87%. Comparing base (28902ac) to head (c600a4d).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #7447      +/-   ##
==========================================
- Coverage   63.27%   58.87%   -4.40%     
==========================================
  Files        2074     2076       +2     
  Lines       91391    91424      +33     
  Branches     8140     8143       +3     
==========================================
- Hits        57825    53829    -3996     
- Misses      31565    35672    +4107     
+ Partials     2001     1923      -78

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

JimmyVo16 · 2026-04-14T14:40:40Z

I looked at the failed Build Docker images (SeederApi, ./util, linux/amd64, linux/arm64, true) job. It looks like a permission issue when logging into Azure. I don't think this is related to my change.

JimmyVo16 · 2026-04-15T13:37:27Z

Added @jrmccannon as a reviewer because he has context working with the organization cache. @BTreston, feel free to add feedback, though.

jrmccannon

Just a few things @eliykat had mentioned to me and we should be consistent across the two ability caches.

jrmccannon · 2026-04-15T15:40:02Z

        services.AddOptionality();
        services.AddTokenizers();

+        services.AddDistributedCache(globalSettings);


Did you run into an error with this?

I would say there's a chance this would error as it attempts to add more than 1 Distributed cache. It is In several startup files.

jrmccannon · 2026-04-15T15:40:49Z

+        services.AddExtendedCache(ExtendedProviderAbilityCacheService.CacheName, globalSettings);
+        services.AddSingleton<IProviderAbilityCacheService, ExtendedProviderAbilityCacheService>();


Move this to an extension method similar to OrgAbility.

jrmccannon · 2026-04-15T15:41:06Z

+        services.AddExtendedCache(ExtendedProviderAbilityCacheService.CacheName, globalSettings);
+        services.AddSingleton<IProviderAbilityCacheService, ExtendedProviderAbilityCacheService>();


Use extension method instead

jrmccannon · 2026-04-15T15:42:51Z

+namespace Bit.Core.Test.AdminConsole.AbilitiesCache;
+
+[SutProviderCustomize]
+public class ExtendedProviderAbilityCacheServiceTests


Is this testing anything meaningful? I'd say remove these unit tests and replace with some integration tests to validate.

eliykat · 2026-04-16T00:59:01Z

+    public async Task<ProviderAbility?> GetProviderAbilityAsync(Guid providerId)
+    {
+        return await cache.GetOrSetAsync<ProviderAbility?>(
+            $"{providerId}",


@jrmccannon this is also a different way of constructing the cache key compared to yours - while the guid should be unique, the prefix seems like better practice to me if required to namespace it. Or does the CacheName already namespace it? I don't mind, as long as the approach is consistent.

Yes per @justindbaur's comment

We should update the Org cache since I think not having unnecessary prefixes is a better path.

Thanks for the info! I agree.

claude · 2026-04-16T16:26:09Z

+    public static IServiceCollection AddProviderAbilityCache(this IServiceCollection services,
+        GlobalSettings globalSettings) =>
+        services.AddExtendedCache(ExtendedProviderAbilityCacheService.CacheName, globalSettings)
+            .AddSingleton<IProviderAbilityCacheService, ExtendedProviderAbilityCacheService>();


💡 Suggestion (non-blocking): This registers as AddSingleton, while the analogous OrganizationAbilityServiceCollectionsExtension uses AddScoped. Both work correctly since all dependencies (IFusionCache, IProviderRepository) are singletons, but the inconsistency is worth calling out — was this an intentional change from the org pattern?

@jrmccannon , I think we should use AddSingleton for this since it’s a stateless service. Let me know what you think.

I know you have a PR open for the multiple use case. I can add a request for changes on that one, if you’d like.

claude · 2026-04-16T16:26:13Z

+        return await cache.GetOrSetAsync<ProviderAbility?>(
+            $"{providerId}",
+            async _ => await providerRepository.GetAbilityAsync(providerId)
+        );


💡 Suggestion (non-blocking): The cache key here is a bare GUID ($"{providerId}"), while the org equivalent uses a prefixed, formatted key ($"org-ability:{organizationId:N}"). Since these live in separate named caches there's no collision risk, but a prefixed key like $"provider-ability:{providerId:N}" would improve consistency and make cache entries easier to identify when debugging.

Suggested change

return await cache.GetOrSetAsync<ProviderAbility?>(

$"{providerId}",

async _ => await providerRepository.GetAbilityAsync(providerId)

);

return await cache.GetOrSetAsync<ProviderAbility?>(

$"provider-ability:{providerId:N}",

async _ => await providerRepository.GetAbilityAsync(providerId)

);

…iceTests.cs Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

sonarqubecloud · 2026-04-16T17:04:09Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarQube Cloud

jrmccannon

Looks good. I took recommendations from this and made mine consistent as well.

eliykat

Nothing more needed from me, you're all over it 👍

[PM-32069] Add ExtendedProviderAbilityCacheService

76d5c4f

JimmyVo16 self-assigned this Apr 10, 2026

JimmyVo16 added the ai-review Request a Claude code review label Apr 10, 2026

merge main

28a5089

[PM-32069] Add DI for the event service

ff79dbf

Merge branch 'main' into ac/pm-32069/add-extended-provider-cache

43bfc80

JimmyVo16 marked this pull request as ready for review April 14, 2026 14:39

JimmyVo16 requested review from a team as code owners April 14, 2026 14:39

JimmyVo16 requested a review from BTreston April 14, 2026 14:39

JimmyVo16 requested a review from jrmccannon April 15, 2026 13:37

jrmccannon requested changes Apr 15, 2026

View reviewed changes

JimmyVo16 added 3 commits April 15, 2026 16:13

[PM-32069] Move provider cache DI to an extension method.

e5e43c2

[PM-32069] merge main without FeatureRoutedCacheService.cs

ec86310

[PM-32069] Fix FeatureRoutedCacheService merge conflicts.

9f7ba68

eliykat reviewed Apr 16, 2026

View reviewed changes

JimmyVo16 added 2 commits April 16, 2026 12:22

[PM-32069] Fix AddDistributedCache config

0f919b5

[PM-32069] wip

4841381

claude Bot reviewed Apr 16, 2026

View reviewed changes

Comment thread test/Core.Test/Services/Implementations/FeatureRoutedCacheServiceTests.cs Outdated

JimmyVo16 and others added 3 commits April 16, 2026 12:49

[PM-32069] Move provider cache tests to integration tests.

dc83325

Update test/Core.Test/Services/Implementations/FeatureRoutedCacheServ…

3829fb5

…iceTests.cs Co-authored-by: claude[bot] <209825114+claude[bot]@users.noreply.github.com>

Merge branch 'main' into ac/pm-32069/add-extended-provider-cache

c600a4d

JimmyVo16 mentioned this pull request Apr 16, 2026

[PM-32073] - Added Bulk Get Org Ability #7476

Merged

JimmyVo16 requested review from eliykat and jrmccannon April 16, 2026 17:57

jrmccannon approved these changes Apr 16, 2026

View reviewed changes

Banrion approved these changes Apr 16, 2026

View reviewed changes

eliykat reviewed Apr 16, 2026

View reviewed changes

JimmyVo16 merged commit 914d412 into main Apr 17, 2026
64 checks passed

JimmyVo16 deleted the ac/pm-32069/add-extended-provider-cache branch April 17, 2026 12:45

		services.AddExtendedCache(ExtendedProviderAbilityCacheService.CacheName, globalSettings);
		services.AddSingleton<IProviderAbilityCacheService, ExtendedProviderAbilityCacheService>();

Conversation

JimmyVo16 commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎟️ Tracking

📔 Objective

📸 Screenshots

Uh oh!

github-actions Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Bitwarden Claude Code Review

Uh oh!

codecov Bot commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

JimmyVo16 commented Apr 14, 2026

Uh oh!

JimmyVo16 commented Apr 15, 2026

Uh oh!

jrmccannon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eliykat Apr 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude Bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

claude Bot Apr 16, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sonarqubecloud Bot commented Apr 16, 2026

Quality Gate passed

Uh oh!

jrmccannon left a comment

Choose a reason for hiding this comment

Uh oh!

eliykat left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

JimmyVo16 commented Apr 10, 2026 •

edited

Loading

github-actions Bot commented Apr 10, 2026 •

edited

Loading

github-actions Bot commented Apr 10, 2026 •

edited

Loading

codecov Bot commented Apr 10, 2026 •

edited

Loading

eliykat Apr 16, 2026 •

edited

Loading