From e487d9562312e8e7b00e3babe44051565fcd4988 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Fri, 10 Apr 2026 09:55:59 -0500 Subject: [PATCH 1/3] Added extended cache implementation for org ability. --- ...ExtendedOrganizationAbilityCacheService.cs | 33 ++++ .../IOrganizationAbilityCacheService.cs | 11 ++ .../OrganizationAbilityCacheConstants.cs | 9 + .../FeatureRoutedCacheService.cs | 28 ++- .../Utilities/ServiceCollectionExtensions.cs | 2 + ...dedOrganizationAbilityCacheServiceTests.cs | 122 +++++++++++++ .../FeatureRoutedCacheServiceTests.cs | 171 +++++++++++++++++- 7 files changed, 363 insertions(+), 13 deletions(-) create mode 100644 src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs create mode 100644 src/Core/AdminConsole/AbilitiesCache/IOrganizationAbilityCacheService.cs create mode 100644 src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs create mode 100644 test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs diff --git a/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs b/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs new file mode 100644 index 000000000000..0e206bcad79c --- /dev/null +++ b/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs @@ -0,0 +1,33 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Models.Data.Organizations; +using Bit.Core.Repositories; +using Microsoft.Extensions.DependencyInjection; +using ZiggyCreatures.Caching.Fusion; + +namespace Bit.Core.AdminConsole.AbilitiesCache; + +public class ExtendedOrganizationAbilityCacheService( + [FromKeyedServices(OrganizationAbilityCacheConstants.CacheName)] IFusionCache cache, + IOrganizationRepository organizationRepository) + : IOrganizationAbilityCacheService +{ + public async Task GetOrganizationAbilityAsync(Guid orgId) + { + var cacheKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(orgId); + return await cache.GetOrSetAsync( + cacheKey, + async _ => await organizationRepository.GetAbilityAsync(orgId)); + } + + public async Task UpsertOrganizationAbilityAsync(Organization organization) + { + var cacheKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organization.Id); + await cache.SetAsync(cacheKey, new OrganizationAbility(organization)); + } + + public async Task DeleteOrganizationAbilityAsync(Guid organizationId) + { + var cacheKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organizationId); + await cache.RemoveAsync(cacheKey); + } +} diff --git a/src/Core/AdminConsole/AbilitiesCache/IOrganizationAbilityCacheService.cs b/src/Core/AdminConsole/AbilitiesCache/IOrganizationAbilityCacheService.cs new file mode 100644 index 000000000000..9a1b9d30af89 --- /dev/null +++ b/src/Core/AdminConsole/AbilitiesCache/IOrganizationAbilityCacheService.cs @@ -0,0 +1,11 @@ +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Models.Data.Organizations; + +namespace Bit.Core.AdminConsole.AbilitiesCache; + +public interface IOrganizationAbilityCacheService +{ + Task GetOrganizationAbilityAsync(Guid orgId); + Task UpsertOrganizationAbilityAsync(Organization organization); + Task DeleteOrganizationAbilityAsync(Guid organizationId); +} diff --git a/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs b/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs new file mode 100644 index 000000000000..34643560bc5e --- /dev/null +++ b/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs @@ -0,0 +1,9 @@ +namespace Bit.Core.AdminConsole.AbilitiesCache; + +public static class OrganizationAbilityCacheConstants +{ + public const string CacheName = "OrganizationAbilities"; + + public static string BuildCacheKeyForOrganizationAbility(Guid organizationId) + => $"org-ability:{organizationId:N}"; +} diff --git a/src/Core/Services/Implementations/FeatureRoutedCacheService.cs b/src/Core/Services/Implementations/FeatureRoutedCacheService.cs index 2631296e22c0..1dd74b4e5bf5 100644 --- a/src/Core/Services/Implementations/FeatureRoutedCacheService.cs +++ b/src/Core/Services/Implementations/FeatureRoutedCacheService.cs @@ -7,14 +7,18 @@ namespace Bit.Core.Services.Implementations; public class FeatureRoutedCacheService( - IVCurrentInMemoryApplicationCacheService inMemoryApplicationCacheService) + IVCurrentInMemoryApplicationCacheService inMemoryApplicationCacheService, + IOrganizationAbilityCacheService extendedCacheService, + IFeatureService featureService) : IApplicationCacheService { public Task> GetOrganizationAbilitiesAsync() => inMemoryApplicationCacheService.GetOrganizationAbilitiesAsync(); public Task GetOrganizationAbilityAsync(Guid orgId) => - inMemoryApplicationCacheService.GetOrganizationAbilityAsync(orgId); + featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + ? extendedCacheService.GetOrganizationAbilityAsync(orgId) + : inMemoryApplicationCacheService.GetOrganizationAbilityAsync(orgId); public Task> GetProviderAbilitiesAsync() => inMemoryApplicationCacheService.GetProviderAbilitiesAsync(); @@ -44,19 +48,29 @@ public async Task> GetOrganizationAbiliti } public Task UpsertOrganizationAbilityAsync(Organization organization) => - inMemoryApplicationCacheService.UpsertOrganizationAbilityAsync(organization); + featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + ? extendedCacheService.UpsertOrganizationAbilityAsync(organization) + : inMemoryApplicationCacheService.UpsertOrganizationAbilityAsync(organization); public Task UpsertProviderAbilityAsync(Provider provider) => inMemoryApplicationCacheService.UpsertProviderAbilityAsync(provider); public Task DeleteOrganizationAbilityAsync(Guid organizationId) => - inMemoryApplicationCacheService.DeleteOrganizationAbilityAsync(organizationId); + featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + ? extendedCacheService.DeleteOrganizationAbilityAsync(organizationId) + : inMemoryApplicationCacheService.DeleteOrganizationAbilityAsync(organizationId); public Task DeleteProviderAbilityAsync(Guid providerId) => inMemoryApplicationCacheService.DeleteProviderAbilityAsync(providerId); public async Task BaseUpsertOrganizationAbilityAsync(Organization organization) { + if (featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache)) + { + await extendedCacheService.UpsertOrganizationAbilityAsync(organization); + return; + } + if (inMemoryApplicationCacheService is InMemoryServiceBusApplicationCacheService serviceBusCache) { await serviceBusCache.BaseUpsertOrganizationAbilityAsync(organization); @@ -69,6 +83,12 @@ public async Task BaseUpsertOrganizationAbilityAsync(Organization organization) public async Task BaseDeleteOrganizationAbilityAsync(Guid organizationId) { + if (featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache)) + { + await extendedCacheService.DeleteOrganizationAbilityAsync(organizationId); + return; + } + if (inMemoryApplicationCacheService is InMemoryServiceBusApplicationCacheService serviceBusCache) { await serviceBusCache.BaseDeleteOrganizationAbilityAsync(organizationId); diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 85886027ac2d..44934e2b1bec 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -295,6 +295,8 @@ public static void AddDefaultServices(this IServiceCollection services, GlobalSe services.AddTokenizers(); services.AddScoped(); + services.AddExtendedCache(OrganizationAbilityCacheConstants.CacheName, globalSettings); + services.AddScoped(); if (CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ConnectionString) && CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ApplicationCacheTopicName)) diff --git a/test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs b/test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs new file mode 100644 index 000000000000..62d5793890ba --- /dev/null +++ b/test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs @@ -0,0 +1,122 @@ +using Bit.Core.AdminConsole.AbilitiesCache; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Models.Data.Organizations; +using Bit.Core.Repositories; +using Bit.Test.Common.AutoFixture; +using Bit.Test.Common.AutoFixture.Attributes; +using NSubstitute; +using Xunit; +using ZiggyCreatures.Caching.Fusion; + +namespace Bit.Core.Test.AdminConsole.AbilitiesCache; + +[SutProviderCustomize] +public class ExtendedOrganizationAbilityCacheServiceTests +{ + [Theory, BitAutoData] + public async Task GetOrganizationAbilityAsync_CallsGetOrSetAsyncWithCorrectKey( + SutProvider sutProvider, + Guid orgId, + OrganizationAbility expectedAbility) + { + // Arrange + var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(orgId); + sutProvider.GetDependency() + .GetAbilityAsync(orgId) + .Returns(expectedAbility); + + sutProvider.GetDependency() + .GetOrSetAsync( + key: Arg.Is(expectedKey), + factory: Arg.Any>>(), + options: Arg.Any(), + tags: Arg.Any>()) + .Returns(callInfo => + { + var factory = callInfo + .ArgAt, CancellationToken, + Task>>(1); + return new ValueTask(factory.Invoke(null!, CancellationToken.None)); + }); + + // Act + var result = await sutProvider.Sut.GetOrganizationAbilityAsync(orgId); + + // Assert + Assert.Equal(expectedAbility, result); + await sutProvider.GetDependency() + .Received(1) + .GetAbilityAsync(orgId); + } + + [Theory, BitAutoData] + public async Task GetOrganizationAbilityAsync_WhenOrgDoesNotExist_ReturnsNull( + SutProvider sutProvider, + Guid orgId) + { + // Arrange + var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(orgId); + sutProvider.GetDependency() + .GetAbilityAsync(orgId) + .Returns((OrganizationAbility?)null); + + sutProvider.GetDependency() + .GetOrSetAsync( + key: Arg.Is(expectedKey), + factory: Arg.Any>>(), + options: Arg.Any(), + tags: Arg.Any>()) + .Returns(callInfo => + { + var factory = callInfo + .ArgAt, CancellationToken, + Task>>(1); + return new ValueTask(factory.Invoke(null!, CancellationToken.None)); + }); + + // Act + var result = await sutProvider.Sut.GetOrganizationAbilityAsync(orgId); + + // Assert + Assert.Null(result); + } + + [Theory, BitAutoData] + public async Task UpsertOrganizationAbilityAsync_CallsSetAsyncWithCorrectKey( + SutProvider sutProvider, + Organization organization) + { + // Arrange + var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organization.Id); + + // Act + await sutProvider.Sut.UpsertOrganizationAbilityAsync(organization); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .SetAsync( + Arg.Is(expectedKey), + Arg.Is(a => a != null && a.Id == organization.Id), + Arg.Any(), + Arg.Any>(), + Arg.Any()); + } + + [Theory, BitAutoData] + public async Task DeleteOrganizationAbilityAsync_CallsRemoveAsyncWithCorrectKey( + SutProvider sutProvider, + Guid organizationId) + { + // Arrange + var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organizationId); + + // Act + await sutProvider.Sut.DeleteOrganizationAbilityAsync(organizationId); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .RemoveAsync(expectedKey); + } +} diff --git a/test/Core.Test/Services/Implementations/FeatureRoutedCacheServiceTests.cs b/test/Core.Test/Services/Implementations/FeatureRoutedCacheServiceTests.cs index a66c44d4133a..af70ff79a2f7 100644 --- a/test/Core.Test/Services/Implementations/FeatureRoutedCacheServiceTests.cs +++ b/test/Core.Test/Services/Implementations/FeatureRoutedCacheServiceTests.cs @@ -39,12 +39,15 @@ await sutProvider.GetDependency() } [Theory, BitAutoData] - public async Task GetOrganizationAbilityAsync_ReturnsFromInMemoryService( + public async Task GetOrganizationAbilityAsync_WhenFlagOff_ReturnsFromInMemoryService( SutProvider sutProvider, Guid orgId, OrganizationAbility expectedResult) { // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(false); sutProvider.GetDependency() .GetOrganizationAbilityAsync(orgId) .Returns(expectedResult); @@ -57,6 +60,36 @@ public async Task GetOrganizationAbilityAsync_ReturnsFromInMemoryService( await sutProvider.GetDependency() .Received(1) .GetOrganizationAbilityAsync(orgId); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .GetOrganizationAbilityAsync(default); + } + + [Theory, BitAutoData] + public async Task GetOrganizationAbilityAsync_WhenFlagOn_ReturnsFromExtendedCacheService( + SutProvider sutProvider, + Guid orgId, + OrganizationAbility expectedResult) + { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(true); + sutProvider.GetDependency() + .GetOrganizationAbilityAsync(orgId) + .Returns(expectedResult); + + // Act + var result = await sutProvider.Sut.GetOrganizationAbilityAsync(orgId); + + // Assert + Assert.Equal(expectedResult, result); + await sutProvider.GetDependency() + .Received(1) + .GetOrganizationAbilityAsync(orgId); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .GetOrganizationAbilityAsync(default); } [Theory, BitAutoData] @@ -236,10 +269,15 @@ public async Task GetOrganizationAbilitiesAsync_WhenNoIdsMatched_ReturnsEmptyDic } [Theory, BitAutoData] - public async Task UpsertOrganizationAbilityAsync_CallsInMemoryService( + public async Task UpsertOrganizationAbilityAsync_WhenFlagOff_CallsInMemoryService( SutProvider sutProvider, Organization organization) { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(false); + // Act await sutProvider.Sut.UpsertOrganizationAbilityAsync(organization); @@ -247,6 +285,31 @@ public async Task UpsertOrganizationAbilityAsync_CallsInMemoryService( await sutProvider.GetDependency() .Received(1) .UpsertOrganizationAbilityAsync(organization); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpsertOrganizationAbilityAsync(default); + } + + [Theory, BitAutoData] + public async Task UpsertOrganizationAbilityAsync_WhenFlagOn_CallsExtendedCacheService( + SutProvider sutProvider, + Organization organization) + { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(true); + + // Act + await sutProvider.Sut.UpsertOrganizationAbilityAsync(organization); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .UpsertOrganizationAbilityAsync(organization); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .UpsertOrganizationAbilityAsync(default); } [Theory, BitAutoData] @@ -264,10 +327,15 @@ await sutProvider.GetDependency() } [Theory, BitAutoData] - public async Task DeleteOrganizationAbilityAsync_CallsInMemoryService( + public async Task DeleteOrganizationAbilityAsync_WhenFlagOff_CallsInMemoryService( SutProvider sutProvider, Guid organizationId) { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(false); + // Act await sutProvider.Sut.DeleteOrganizationAbilityAsync(organizationId); @@ -275,6 +343,31 @@ public async Task DeleteOrganizationAbilityAsync_CallsInMemoryService( await sutProvider.GetDependency() .Received(1) .DeleteOrganizationAbilityAsync(organizationId); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteOrganizationAbilityAsync(default); + } + + [Theory, BitAutoData] + public async Task DeleteOrganizationAbilityAsync_WhenFlagOn_CallsExtendedCacheService( + SutProvider sutProvider, + Guid organizationId) + { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(true); + + // Act + await sutProvider.Sut.DeleteOrganizationAbilityAsync(organizationId); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .DeleteOrganizationAbilityAsync(organizationId); + await sutProvider.GetDependency() + .DidNotReceiveWithAnyArgs() + .DeleteOrganizationAbilityAsync(default); } [Theory, BitAutoData] @@ -292,12 +385,15 @@ await sutProvider.GetDependency() } [Theory, BitAutoData] - public async Task BaseUpsertOrganizationAbilityAsync_CallsServiceBusCache( + public async Task BaseUpsertOrganizationAbilityAsync_WhenFlagOff_CallsServiceBusCache( Organization organization) { // Arrange var currentCacheService = CreateCurrentCacheMockService(); - var sut = new FeatureRoutedCacheService(currentCacheService); + var extendedCacheService = Substitute.For(); + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache).Returns(false); + var sut = new FeatureRoutedCacheService(currentCacheService, extendedCacheService, featureService); // Act await sut.BaseUpsertOrganizationAbilityAsync(organization); @@ -306,13 +402,40 @@ public async Task BaseUpsertOrganizationAbilityAsync_CallsServiceBusCache( await currentCacheService .Received(1) .BaseUpsertOrganizationAbilityAsync(organization); + await extendedCacheService + .DidNotReceiveWithAnyArgs() + .UpsertOrganizationAbilityAsync(default); } [Theory, BitAutoData] - public async Task BaseUpsertOrganizationAbilityAsync_WhenServiceIsNotServiceBusCache_ThrowsException( + public async Task BaseUpsertOrganizationAbilityAsync_WhenFlagOn_CallsExtendedCacheService( SutProvider sutProvider, Organization organization) { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(true); + + // Act + await sutProvider.Sut.BaseUpsertOrganizationAbilityAsync(organization); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .UpsertOrganizationAbilityAsync(organization); + } + + [Theory, BitAutoData] + public async Task BaseUpsertOrganizationAbilityAsync_WhenFlagOff_AndServiceIsNotServiceBusCache_ThrowsException( + SutProvider sutProvider, + Organization organization) + { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(false); + // Act var ex = await Assert.ThrowsAsync( () => sutProvider.Sut.BaseUpsertOrganizationAbilityAsync(organization)); @@ -322,12 +445,15 @@ public async Task BaseUpsertOrganizationAbilityAsync_WhenServiceIsNotServiceBusC } [Theory, BitAutoData] - public async Task BaseDeleteOrganizationAbilityAsync_CallsServiceBusCache( + public async Task BaseDeleteOrganizationAbilityAsync_WhenFlagOff_CallsServiceBusCache( Guid organizationId) { // Arrange var currentCacheService = CreateCurrentCacheMockService(); - var sut = new FeatureRoutedCacheService(currentCacheService); + var extendedCacheService = Substitute.For(); + var featureService = Substitute.For(); + featureService.IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache).Returns(false); + var sut = new FeatureRoutedCacheService(currentCacheService, extendedCacheService, featureService); // Act await sut.BaseDeleteOrganizationAbilityAsync(organizationId); @@ -336,13 +462,40 @@ public async Task BaseDeleteOrganizationAbilityAsync_CallsServiceBusCache( await currentCacheService .Received(1) .BaseDeleteOrganizationAbilityAsync(organizationId); + await extendedCacheService + .DidNotReceiveWithAnyArgs() + .DeleteOrganizationAbilityAsync(default); + } + + [Theory, BitAutoData] + public async Task BaseDeleteOrganizationAbilityAsync_WhenFlagOn_CallsExtendedCacheService( + SutProvider sutProvider, + Guid organizationId) + { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(true); + + // Act + await sutProvider.Sut.BaseDeleteOrganizationAbilityAsync(organizationId); + + // Assert + await sutProvider.GetDependency() + .Received(1) + .DeleteOrganizationAbilityAsync(organizationId); } [Theory, BitAutoData] - public async Task BaseDeleteOrganizationAbilityAsync_WhenServiceIsNotServiceBusCache_ThrowsException( + public async Task BaseDeleteOrganizationAbilityAsync_WhenFlagOff_AndServiceIsNotServiceBusCache_ThrowsException( SutProvider sutProvider, Guid organizationId) { + // Arrange + sutProvider.GetDependency() + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(false); + // Act var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.BaseDeleteOrganizationAbilityAsync(organizationId)); From 47375586eb941a8e745d09536e696c8a0e056337 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Fri, 10 Apr 2026 12:03:00 -0500 Subject: [PATCH 2/3] fixed up events --- src/Events/Startup.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/Events/Startup.cs b/src/Events/Startup.cs index cf8534d134f7..43706e4a3880 100644 --- a/src/Events/Startup.cs +++ b/src/Events/Startup.cs @@ -56,6 +56,8 @@ public void ConfigureServices(IServiceCollection services) var usingServiceBusAppCache = CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ConnectionString) && CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ApplicationCacheTopicName); services.AddScoped(); + services.AddExtendedCache(OrganizationAbilityCacheConstants.CacheName, globalSettings); + services.AddScoped(); if (usingServiceBusAppCache) { From 907221b456a9c797788cd027c9bd08f17d576190 Mon Sep 17 00:00:00 2001 From: Jared McCannon Date: Tue, 14 Apr 2026 10:03:26 -0500 Subject: [PATCH 3/3] Moved utilities to service class. Moved tests to integration tests - with specific endpoints tested. moved registration to ext method --- ...ExtendedOrganizationAbilityCacheService.cs | 18 ++- .../OrganizationAbilityCacheConstants.cs | 9 -- ...ationAbilityServiceCollectionsExtension.cs | 12 ++ src/Events/Startup.cs | 3 +- .../Utilities/ServiceCollectionExtensions.cs | 3 +- .../OrganizationAbilityCacheTests.cs | 132 ++++++++++++++++++ ...dedOrganizationAbilityCacheServiceTests.cs | 122 ---------------- 7 files changed, 160 insertions(+), 139 deletions(-) delete mode 100644 src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs create mode 100644 src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityServiceCollectionsExtension.cs create mode 100644 test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationAbilityCacheTests.cs delete mode 100644 test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs diff --git a/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs b/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs index 0e206bcad79c..c7b72158db24 100644 --- a/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs +++ b/src/Core/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheService.cs @@ -3,17 +3,24 @@ using Bit.Core.Repositories; using Microsoft.Extensions.DependencyInjection; using ZiggyCreatures.Caching.Fusion; +using static Bit.Core.AdminConsole.AbilitiesCache.ExtendedOrganizationAbilityCacheConstants; namespace Bit.Core.AdminConsole.AbilitiesCache; +public static class ExtendedOrganizationAbilityCacheConstants +{ + public const string CacheName = "OrganizationAbilities"; +} + public class ExtendedOrganizationAbilityCacheService( - [FromKeyedServices(OrganizationAbilityCacheConstants.CacheName)] IFusionCache cache, + [FromKeyedServices(CacheName)] IFusionCache cache, IOrganizationRepository organizationRepository) : IOrganizationAbilityCacheService { + public async Task GetOrganizationAbilityAsync(Guid orgId) { - var cacheKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(orgId); + var cacheKey = BuildCacheKeyForOrganizationAbility(orgId); return await cache.GetOrSetAsync( cacheKey, async _ => await organizationRepository.GetAbilityAsync(orgId)); @@ -21,13 +28,16 @@ public class ExtendedOrganizationAbilityCacheService( public async Task UpsertOrganizationAbilityAsync(Organization organization) { - var cacheKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organization.Id); + var cacheKey = BuildCacheKeyForOrganizationAbility(organization.Id); await cache.SetAsync(cacheKey, new OrganizationAbility(organization)); } public async Task DeleteOrganizationAbilityAsync(Guid organizationId) { - var cacheKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organizationId); + var cacheKey = BuildCacheKeyForOrganizationAbility(organizationId); await cache.RemoveAsync(cacheKey); } + + private static string BuildCacheKeyForOrganizationAbility(Guid organizationId) + => $"org-ability:{organizationId:N}"; } diff --git a/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs b/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs deleted file mode 100644 index 34643560bc5e..000000000000 --- a/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityCacheConstants.cs +++ /dev/null @@ -1,9 +0,0 @@ -namespace Bit.Core.AdminConsole.AbilitiesCache; - -public static class OrganizationAbilityCacheConstants -{ - public const string CacheName = "OrganizationAbilities"; - - public static string BuildCacheKeyForOrganizationAbility(Guid organizationId) - => $"org-ability:{organizationId:N}"; -} diff --git a/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityServiceCollectionsExtension.cs b/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityServiceCollectionsExtension.cs new file mode 100644 index 000000000000..20259845e4c7 --- /dev/null +++ b/src/Core/AdminConsole/AbilitiesCache/OrganizationAbilityServiceCollectionsExtension.cs @@ -0,0 +1,12 @@ +using Bit.Core.Settings; +using Microsoft.Extensions.DependencyInjection; + +namespace Bit.Core.AdminConsole.AbilitiesCache; + +public static class OrganizationAbilityServiceCollectionsExtension +{ + public static IServiceCollection AddOrganizationAbilityCache(this IServiceCollection serviceCollection, + GlobalSettings globalSettings) => + serviceCollection.AddExtendedCache(ExtendedOrganizationAbilityCacheConstants.CacheName, globalSettings) + .AddScoped(); +} diff --git a/src/Events/Startup.cs b/src/Events/Startup.cs index 43706e4a3880..a098664a430d 100644 --- a/src/Events/Startup.cs +++ b/src/Events/Startup.cs @@ -56,8 +56,7 @@ public void ConfigureServices(IServiceCollection services) var usingServiceBusAppCache = CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ConnectionString) && CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ApplicationCacheTopicName); services.AddScoped(); - services.AddExtendedCache(OrganizationAbilityCacheConstants.CacheName, globalSettings); - services.AddScoped(); + services.AddOrganizationAbilityCache(globalSettings); if (usingServiceBusAppCache) { diff --git a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs index 44934e2b1bec..1429b4ec9d65 100644 --- a/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs +++ b/src/SharedWeb/Utilities/ServiceCollectionExtensions.cs @@ -295,8 +295,7 @@ public static void AddDefaultServices(this IServiceCollection services, GlobalSe services.AddTokenizers(); services.AddScoped(); - services.AddExtendedCache(OrganizationAbilityCacheConstants.CacheName, globalSettings); - services.AddScoped(); + services.AddOrganizationAbilityCache(globalSettings); if (CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ConnectionString) && CoreHelpers.SettingHasValue(globalSettings.ServiceBus.ApplicationCacheTopicName)) diff --git a/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationAbilityCacheTests.cs b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationAbilityCacheTests.cs new file mode 100644 index 000000000000..6bacc79007ad --- /dev/null +++ b/test/Api.IntegrationTest/AdminConsole/Controllers/OrganizationAbilityCacheTests.cs @@ -0,0 +1,132 @@ +using System.Net; +using Bit.Api.AdminConsole.Models.Request.Organizations; +using Bit.Api.Auth.Models.Request.Accounts; +using Bit.Api.IntegrationTest.Factories; +using Bit.Api.IntegrationTest.Helpers; +using Bit.Core; +using Bit.Core.AdminConsole.Entities; +using Bit.Core.Billing.Enums; +using Bit.Core.Enums; +using Bit.Core.Services; +using NSubstitute; +using Xunit; + +namespace Bit.Api.IntegrationTest.AdminConsole.Controllers; + +public class OrganizationAbilityCacheTests : IClassFixture, IAsyncLifetime +{ + private readonly HttpClient _client; + private readonly ApiApplicationFactory _factory; + private readonly LoginHelper _loginHelper; + + private Organization _organization = null!; + private string _ownerEmail = null!; + + public OrganizationAbilityCacheTests(ApiApplicationFactory factory) + { + _factory = factory; + _factory.SubstituteService(featureService => + { + featureService + .IsEnabled(FeatureFlagKeys.OrgAbilityExtendedCache) + .Returns(true); + }); + _client = factory.CreateClient(); + _loginHelper = new LoginHelper(_factory, _client); + } + + public async Task InitializeAsync() + { + _ownerEmail = $"cache-test-{Guid.NewGuid()}@example.com"; + await _factory.LoginWithNewAccount(_ownerEmail); + + var result = await OrganizationTestHelpers.SignUpAsync( + _factory, + plan: PlanType.EnterpriseAnnually, + ownerEmail: _ownerEmail, + passwordManagerSeats: 5, + paymentMethod: PaymentMethodType.Card); + _organization = result.Item1; + } + + public Task DisposeAsync() + { + _client.Dispose(); + return Task.CompletedTask; + } + + [Fact] + public async Task SignUp_PopulatesCache_GetOrganizationAbilityReturnsAbility() + { + // Arrange - organization already created in InitializeAsync via SignUpAsync, + // which calls UpsertOrganizationAbilityAsync + + // Act - read the cached ability directly + var cacheService = _factory.GetService(); + var ability = await cacheService.GetOrganizationAbilityAsync(_organization.Id); + + // Assert - cache was populated by the sign-up flow + Assert.NotNull(ability); + Assert.Equal(_organization.Id, ability.Id); + Assert.True(ability.Enabled); + } + + [Fact] + public async Task Put_UpdatesOrganization_CacheReflectsUpdatedValues() + { + // Arrange - setup in InitializeAsync() + await _loginHelper.LoginAsync(_ownerEmail); + var updateRequest = new OrganizationUpdateRequestModel + { + Name = "Updated Cache Test Org", + BillingEmail = "updated-cache@example.com" + }; + + // Act - update the organization via the HTTP endpoint + var response = await _client.PutAsJsonAsync($"/organizations/{_organization.Id}", updateRequest); + + // Assert - endpoint succeeded and cache was updated + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var cacheService = _factory.GetService(); + var ability = await cacheService.GetOrganizationAbilityAsync(_organization.Id); + Assert.NotNull(ability); + Assert.Equal(_organization.Id, ability.Id); + } + + [Fact] + public async Task Delete_RemovesOrganization_CacheReturnsNull() + { + // Arrange - create a separate org for deletion so we don't affect other tests + var deleteOwnerEmail = $"delete-test-{Guid.NewGuid()}@example.com"; + await _factory.LoginWithNewAccount(deleteOwnerEmail); + + var signUpResult = await OrganizationTestHelpers.SignUpAsync( + _factory, + plan: PlanType.EnterpriseAnnually, + ownerEmail: deleteOwnerEmail, + passwordManagerSeats: 5, + paymentMethod: PaymentMethodType.Card); + var orgToDelete = signUpResult.Item1; + + // Verify cache is populated before delete + var cacheService = _factory.GetService(); + var abilityBeforeDelete = await cacheService.GetOrganizationAbilityAsync(orgToDelete.Id); + Assert.NotNull(abilityBeforeDelete); + + // Act - delete the organization via the HTTP endpoint + await _loginHelper.LoginAsync(deleteOwnerEmail); + var deleteRequest = new SecretVerificationRequestModel + { + MasterPasswordHash = "master_password_hash" + }; + var response = await _client.PostAsJsonAsync( + $"/organizations/{orgToDelete.Id}/delete", deleteRequest); + + // Assert - endpoint succeeded and cache was cleared + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + + var abilityAfterDelete = await cacheService.GetOrganizationAbilityAsync(orgToDelete.Id); + Assert.Null(abilityAfterDelete); + } +} diff --git a/test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs b/test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs deleted file mode 100644 index 62d5793890ba..000000000000 --- a/test/Core.Test/AdminConsole/AbilitiesCache/ExtendedOrganizationAbilityCacheServiceTests.cs +++ /dev/null @@ -1,122 +0,0 @@ -using Bit.Core.AdminConsole.AbilitiesCache; -using Bit.Core.AdminConsole.Entities; -using Bit.Core.Models.Data.Organizations; -using Bit.Core.Repositories; -using Bit.Test.Common.AutoFixture; -using Bit.Test.Common.AutoFixture.Attributes; -using NSubstitute; -using Xunit; -using ZiggyCreatures.Caching.Fusion; - -namespace Bit.Core.Test.AdminConsole.AbilitiesCache; - -[SutProviderCustomize] -public class ExtendedOrganizationAbilityCacheServiceTests -{ - [Theory, BitAutoData] - public async Task GetOrganizationAbilityAsync_CallsGetOrSetAsyncWithCorrectKey( - SutProvider sutProvider, - Guid orgId, - OrganizationAbility expectedAbility) - { - // Arrange - var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(orgId); - sutProvider.GetDependency() - .GetAbilityAsync(orgId) - .Returns(expectedAbility); - - sutProvider.GetDependency() - .GetOrSetAsync( - key: Arg.Is(expectedKey), - factory: Arg.Any>>(), - options: Arg.Any(), - tags: Arg.Any>()) - .Returns(callInfo => - { - var factory = callInfo - .ArgAt, CancellationToken, - Task>>(1); - return new ValueTask(factory.Invoke(null!, CancellationToken.None)); - }); - - // Act - var result = await sutProvider.Sut.GetOrganizationAbilityAsync(orgId); - - // Assert - Assert.Equal(expectedAbility, result); - await sutProvider.GetDependency() - .Received(1) - .GetAbilityAsync(orgId); - } - - [Theory, BitAutoData] - public async Task GetOrganizationAbilityAsync_WhenOrgDoesNotExist_ReturnsNull( - SutProvider sutProvider, - Guid orgId) - { - // Arrange - var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(orgId); - sutProvider.GetDependency() - .GetAbilityAsync(orgId) - .Returns((OrganizationAbility?)null); - - sutProvider.GetDependency() - .GetOrSetAsync( - key: Arg.Is(expectedKey), - factory: Arg.Any>>(), - options: Arg.Any(), - tags: Arg.Any>()) - .Returns(callInfo => - { - var factory = callInfo - .ArgAt, CancellationToken, - Task>>(1); - return new ValueTask(factory.Invoke(null!, CancellationToken.None)); - }); - - // Act - var result = await sutProvider.Sut.GetOrganizationAbilityAsync(orgId); - - // Assert - Assert.Null(result); - } - - [Theory, BitAutoData] - public async Task UpsertOrganizationAbilityAsync_CallsSetAsyncWithCorrectKey( - SutProvider sutProvider, - Organization organization) - { - // Arrange - var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organization.Id); - - // Act - await sutProvider.Sut.UpsertOrganizationAbilityAsync(organization); - - // Assert - await sutProvider.GetDependency() - .Received(1) - .SetAsync( - Arg.Is(expectedKey), - Arg.Is(a => a != null && a.Id == organization.Id), - Arg.Any(), - Arg.Any>(), - Arg.Any()); - } - - [Theory, BitAutoData] - public async Task DeleteOrganizationAbilityAsync_CallsRemoveAsyncWithCorrectKey( - SutProvider sutProvider, - Guid organizationId) - { - // Arrange - var expectedKey = OrganizationAbilityCacheConstants.BuildCacheKeyForOrganizationAbility(organizationId); - - // Act - await sutProvider.Sut.DeleteOrganizationAbilityAsync(organizationId); - - // Assert - await sutProvider.GetDependency() - .Received(1) - .RemoveAsync(expectedKey); - } -}