From 6fe12c7759d65ae05f241fbada3fd998c35d466b Mon Sep 17 00:00:00 2001 From: JaredScar Date: Thu, 2 Jul 2026 18:03:27 -0400 Subject: [PATCH 1/2] Refactor OrganizationDataOwnershipPolicyEventHandler to utilize organization ability cache - Added IOrganizationAbilityCacheService to the constructor for improved performance. - Replaced direct organization repository calls with cache lookups in UpsertDefaultCollectionsForUsersAsync. - Introduced OrganizationUsesMyItemsAsync method to streamline cache access and fallback logic. - Updated tests to accommodate the new cache service and ensure correct behavior under various scenarios. --- ...nizationDataOwnershipPolicyEventHandler.cs | 32 ++-- ...ionDataOwnershipPolicyEventHandlerTests.cs | 147 +++++++++++++++++- 2 files changed, 165 insertions(+), 14 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs index 60026e96ea53..665d09aa360a 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs @@ -1,4 +1,5 @@ -using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.AbilitiesCache; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; @@ -12,6 +13,7 @@ public class OrganizationDataOwnershipPolicyEventHandler( IPolicyRepository policyRepository, ICollectionRepository collectionRepository, IOrganizationRepository organizationRepository, + IOrganizationAbilityCacheService organizationAbilityCacheService, IEnumerable> factories) : OrganizationPolicyEventHandler(policyRepository, factories), IOnPolicyPostUpdateEvent { @@ -44,16 +46,7 @@ public async Task ExecutePostUpsertSideEffectAsync( private async Task UpsertDefaultCollectionsForUsersAsync(PolicyUpdate policyUpdate, string defaultCollectionName) { - // FIXME: we should use the organizationAbility cache here, but it is currently flaky - // and it's not obvious how to handle a cache failure. - // https://bitwarden.atlassian.net/browse/PM-32699 - var organization = await organizationRepository.GetByIdAsync(policyUpdate.OrganizationId); - if (organization == null) - { - throw new InvalidOperationException($"Organization with ID {policyUpdate.OrganizationId} not found."); - } - - if (!organization.UseMyItems) + if (!await OrganizationUsesMyItemsAsync(policyUpdate.OrganizationId)) { return; } @@ -76,4 +69,21 @@ await collectionRepository.CreateDefaultCollectionsBulkAsync( userOrgIds, defaultCollectionName); } + + private async Task OrganizationUsesMyItemsAsync(Guid organizationId) + { + var organizationAbility = await organizationAbilityCacheService.GetOrganizationAbilityAsync(organizationId); + if (organizationAbility != null) + { + return organizationAbility.UseMyItems; + } + + var organization = await organizationRepository.GetByIdAsync(organizationId); + if (organization == null) + { + throw new InvalidOperationException($"Organization with ID {organizationId} not found."); + } + + return organization.UseMyItems; + } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs index c7cc23518e65..73e1be3eb566 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs @@ -1,10 +1,12 @@ -using Bit.Core.AdminConsole.Entities; +using Bit.Core.AdminConsole.AbilitiesCache; +using Bit.Core.AdminConsole.Entities; using Bit.Core.AdminConsole.Enums; using Bit.Core.AdminConsole.Models.Data.Organizations.Policies; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.Models; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyEventHandlers; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; using Bit.Core.Test.AdminConsole.AutoFixture; using Bit.Test.Common.AutoFixture; @@ -255,7 +257,13 @@ private static OrganizationDataOwnershipPolicyEventHandler ArrangeSut( Id = callInfo.Arg(), UseMyItems = useMyItems }); - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, [factory]); + + // Default the ability cache to a miss so the handler falls back to the organization repository + var organizationAbilityCacheService = Substitute.For(); + organizationAbilityCacheService.GetOrganizationAbilityAsync(Arg.Any()) + .Returns((OrganizationAbility?)null); + + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); return sut; } @@ -438,7 +446,12 @@ public async Task ExecuteSideEffectsAsync_OrganizationNotFound_ThrowsInvalidOper // Return null to simulate organization not found organizationRepository.GetByIdAsync(Arg.Any()).Returns((Organization?)null); - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, [factory]); + // Simulate a cache miss so the handler falls back to the organization repository + var organizationAbilityCacheService = Substitute.For(); + organizationAbilityCacheService.GetOrganizationAbilityAsync(Arg.Any()) + .Returns((OrganizationAbility?)null); + + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act & Assert @@ -479,4 +492,132 @@ await collectionRepository Arg.Any>(), Arg.Any()); } + + [Theory] + [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] + public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsEnabled_UsesCacheAndSkipsOrganizationRepository( + Policy postUpdatedPolicy, + Policy? previousPolicyState, + [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, + [OrganizationPolicyDetails(PolicyType.OrganizationDataOwnership)] IEnumerable orgPolicyDetails, + OrganizationDataOwnershipPolicyRequirementFactory factory) + { + // Arrange + var orgPolicyDetailsList = orgPolicyDetails.ToList(); + foreach (var policyDetail in orgPolicyDetailsList) + { + policyDetail.OrganizationId = policyUpdate.OrganizationId; + } + + var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); + var collectionRepository = Substitute.For(); + var organizationRepository = Substitute.For(); + + var organizationAbilityCacheService = Substitute.For(); + organizationAbilityCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId) + .Returns(new OrganizationAbility { Id = policyUpdate.OrganizationId, UseMyItems = true }); + + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); + + // Act + await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState); + + // Assert - Should trust the cached value and never hit the organization repository + await organizationRepository + .DidNotReceiveWithAnyArgs() + .GetByIdAsync(default); + + await collectionRepository + .Received(1) + .CreateDefaultCollectionsBulkAsync( + policyUpdate.OrganizationId, + Arg.Is>(ids => ids.Count() == 3), + _defaultUserCollectionName); + } + + [Theory] + [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] + public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsDisabled_ShortcutsWithoutHittingOrganizationRepository( + Policy postUpdatedPolicy, + Policy? previousPolicyState, + [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, + [OrganizationPolicyDetails(PolicyType.OrganizationDataOwnership)] IEnumerable orgPolicyDetails, + OrganizationDataOwnershipPolicyRequirementFactory factory) + { + // Arrange + var orgPolicyDetailsList = orgPolicyDetails.ToList(); + foreach (var policyDetail in orgPolicyDetailsList) + { + policyDetail.OrganizationId = policyUpdate.OrganizationId; + } + + var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); + var collectionRepository = Substitute.For(); + var organizationRepository = Substitute.For(); + + var organizationAbilityCacheService = Substitute.For(); + organizationAbilityCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId) + .Returns(new OrganizationAbility { Id = policyUpdate.OrganizationId, UseMyItems = false }); + + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); + + // Act + await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState); + + // Assert - Should shortcut based on the cached value alone, without ever calling the organization repository + await organizationRepository + .DidNotReceiveWithAnyArgs() + .GetByIdAsync(default); + + await collectionRepository + .DidNotReceiveWithAnyArgs() + .CreateDefaultCollectionsBulkAsync(default, default, default); + } + + [Theory] + [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] + public async Task ExecutePostUpsertSideEffectAsync_CacheMiss_FallsBackToOrganizationRepository( + Policy postUpdatedPolicy, + Policy? previousPolicyState, + [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, + [OrganizationPolicyDetails(PolicyType.OrganizationDataOwnership)] IEnumerable orgPolicyDetails, + OrganizationDataOwnershipPolicyRequirementFactory factory) + { + // Arrange + var orgPolicyDetailsList = orgPolicyDetails.ToList(); + foreach (var policyDetail in orgPolicyDetailsList) + { + policyDetail.OrganizationId = policyUpdate.OrganizationId; + } + + var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); + var collectionRepository = Substitute.For(); + var organizationRepository = Substitute.For(); + organizationRepository.GetByIdAsync(policyUpdate.OrganizationId) + .Returns(new Organization { Id = policyUpdate.OrganizationId, UseMyItems = true }); + + var organizationAbilityCacheService = Substitute.For(); + organizationAbilityCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId) + .Returns((OrganizationAbility?)null); + + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); + + // Act + await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState); + + // Assert - On a cache miss, the organization repository should be consulted as the source of truth + await organizationRepository + .Received(1) + .GetByIdAsync(policyUpdate.OrganizationId); + + await collectionRepository + .Received(1) + .CreateDefaultCollectionsBulkAsync( + policyUpdate.OrganizationId, + Arg.Is>(ids => ids.Count() == 3), + _defaultUserCollectionName); + } } From 5f75d752b589c292615015f94e47904b04543f6d Mon Sep 17 00:00:00 2001 From: JaredScar Date: Mon, 6 Jul 2026 11:06:57 -0400 Subject: [PATCH 2/2] Refactor OrganizationDataOwnershipPolicyEventHandler to improve error handling and remove organization repository dependency - Removed IOrganizationRepository from the constructor and refactored OrganizationUsesMyItemsAsync to throw NotFoundException when the organization is not found in the ability cache. - Updated tests to reflect changes in exception handling and removed unnecessary organization repository calls. --- ...nizationDataOwnershipPolicyEventHandler.cs | 14 +-- ...ionDataOwnershipPolicyEventHandlerTests.cs | 117 +++--------------- 2 files changed, 19 insertions(+), 112 deletions(-) diff --git a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs index 665d09aa360a..efc3f89e194b 100644 --- a/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs +++ b/src/Core/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandler.cs @@ -5,6 +5,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyUpdateEvents.Interfaces; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Exceptions; using Bit.Core.Repositories; namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyEventHandlers; @@ -12,7 +13,6 @@ namespace Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyEventHandler public class OrganizationDataOwnershipPolicyEventHandler( IPolicyRepository policyRepository, ICollectionRepository collectionRepository, - IOrganizationRepository organizationRepository, IOrganizationAbilityCacheService organizationAbilityCacheService, IEnumerable> factories) : OrganizationPolicyEventHandler(policyRepository, factories), IOnPolicyPostUpdateEvent @@ -73,17 +73,11 @@ await collectionRepository.CreateDefaultCollectionsBulkAsync( private async Task OrganizationUsesMyItemsAsync(Guid organizationId) { var organizationAbility = await organizationAbilityCacheService.GetOrganizationAbilityAsync(organizationId); - if (organizationAbility != null) + if (organizationAbility == null) { - return organizationAbility.UseMyItems; + throw new NotFoundException($"Organization with ID {organizationId} not found."); } - var organization = await organizationRepository.GetByIdAsync(organizationId); - if (organization == null) - { - throw new InvalidOperationException($"Organization with ID {organizationId} not found."); - } - - return organization.UseMyItems; + return organizationAbility.UseMyItems; } } diff --git a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs index 73e1be3eb566..65fff8c8a69d 100644 --- a/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs +++ b/test/Core.Test/AdminConsole/OrganizationFeatures/Policies/PolicyEventHandlers/OrganizationDataOwnershipPolicyEventHandlerTests.cs @@ -6,6 +6,7 @@ using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyEventHandlers; using Bit.Core.AdminConsole.OrganizationFeatures.Policies.PolicyRequirements; using Bit.Core.AdminConsole.Repositories; +using Bit.Core.Exceptions; using Bit.Core.Models.Data.Organizations; using Bit.Core.Repositories; using Bit.Core.Test.AdminConsole.AutoFixture; @@ -34,10 +35,6 @@ public async Task ExecuteSideEffectsAsync_PolicyAlreadyEnabled_DoesNothing( previousPolicyState.OrganizationId = policyUpdate.OrganizationId; organization.Id = policyUpdate.OrganizationId; - sutProvider.GetDependency() - .GetByIdAsync(policyUpdate.OrganizationId) - .Returns(organization); - var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act @@ -62,10 +59,6 @@ public async Task ExecuteSideEffectsAsync_PolicyBeingDisabled_DoesNothing( postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; organization.Id = policyUpdate.OrganizationId; - sutProvider.GetDependency() - .GetByIdAsync(policyUpdate.OrganizationId) - .Returns(organization); - var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act @@ -218,10 +211,6 @@ public async Task ExecuteSideEffectsAsync_WhenDefaultCollectionNameIsInvalid_Doe policyUpdate.Enabled = true; organization.Id = policyUpdate.OrganizationId; - sutProvider.GetDependency() - .GetByIdAsync(policyUpdate.OrganizationId) - .Returns(organization); - var policyRequest = new SavePolicyModel(policyUpdate, metadata); // Act @@ -249,21 +238,16 @@ private static OrganizationDataOwnershipPolicyEventHandler ArrangeSut( ICollectionRepository collectionRepository, bool useMyItems = true) { - var organizationRepository = Substitute.For(); // Default to UseMyItems = true for existing tests - organizationRepository.GetByIdAsync(Arg.Any()) - .Returns(callInfo => new Organization + var organizationAbilityCacheService = Substitute.For(); + organizationAbilityCacheService.GetOrganizationAbilityAsync(Arg.Any()) + .Returns(callInfo => new OrganizationAbility { Id = callInfo.Arg(), UseMyItems = useMyItems }); - // Default the ability cache to a miss so the handler falls back to the organization repository - var organizationAbilityCacheService = Substitute.For(); - organizationAbilityCacheService.GetOrganizationAbilityAsync(Arg.Any()) - .Returns((OrganizationAbility?)null); - - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationAbilityCacheService, [factory]); return sut; } @@ -280,10 +264,6 @@ public async Task ExecutePostUpsertSideEffectAsync_PolicyAlreadyEnabled_DoesNoth previousPolicyState.OrganizationId = policyUpdate.OrganizationId; organization.Id = policyUpdate.OrganizationId; - sutProvider.GetDependency() - .GetByIdAsync(policyUpdate.OrganizationId) - .Returns(organization); - var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act @@ -308,10 +288,6 @@ public async Task ExecutePostUpsertSideEffectAsync_PolicyBeingDisabled_DoesNothi postUpdatedPolicy.OrganizationId = policyUpdate.OrganizationId; organization.Id = policyUpdate.OrganizationId; - sutProvider.GetDependency() - .GetByIdAsync(policyUpdate.OrganizationId) - .Returns(organization); - var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act @@ -408,10 +384,6 @@ public async Task ExecutePostUpsertSideEffectAsync_WhenDefaultCollectionNameIsIn policyUpdate.Enabled = true; organization.Id = policyUpdate.OrganizationId; - sutProvider.GetDependency() - .GetByIdAsync(policyUpdate.OrganizationId) - .Returns(organization); - var policyRequest = new SavePolicyModel(policyUpdate, metadata); // Act @@ -425,7 +397,7 @@ await sutProvider.GetDependency() [Theory] [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] - public async Task ExecuteSideEffectsAsync_OrganizationNotFound_ThrowsInvalidOperationException( + public async Task ExecuteSideEffectsAsync_OrganizationNotFound_ThrowsNotFoundException( Policy postUpdatedPolicy, Policy? previousPolicyState, [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, @@ -441,21 +413,17 @@ public async Task ExecuteSideEffectsAsync_OrganizationNotFound_ThrowsInvalidOper var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); var collectionRepository = Substitute.For(); - var organizationRepository = Substitute.For(); - - // Return null to simulate organization not found - organizationRepository.GetByIdAsync(Arg.Any()).Returns((Organization?)null); - // Simulate a cache miss so the handler falls back to the organization repository + // Simulate the organization not being present in the ability cache var organizationAbilityCacheService = Substitute.For(); organizationAbilityCacheService.GetOrganizationAbilityAsync(Arg.Any()) .Returns((OrganizationAbility?)null); - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationAbilityCacheService, [factory]); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act & Assert - await Assert.ThrowsAsync(() => + await Assert.ThrowsAsync(() => sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState)); } @@ -495,7 +463,7 @@ await collectionRepository [Theory] [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] - public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsEnabled_UsesCacheAndSkipsOrganizationRepository( + public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsEnabled_CreatesDefaultCollections( Policy postUpdatedPolicy, Policy? previousPolicyState, [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, @@ -511,23 +479,18 @@ public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsEnabled_Use var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); var collectionRepository = Substitute.For(); - var organizationRepository = Substitute.For(); var organizationAbilityCacheService = Substitute.For(); organizationAbilityCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId) .Returns(new OrganizationAbility { Id = policyUpdate.OrganizationId, UseMyItems = true }); - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationAbilityCacheService, [factory]); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState); - // Assert - Should trust the cached value and never hit the organization repository - await organizationRepository - .DidNotReceiveWithAnyArgs() - .GetByIdAsync(default); - + // Assert - Should trust the cached value await collectionRepository .Received(1) .CreateDefaultCollectionsBulkAsync( @@ -538,7 +501,7 @@ await collectionRepository [Theory] [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] - public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsDisabled_ShortcutsWithoutHittingOrganizationRepository( + public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsDisabled_DoesNotCreateCollections( Policy postUpdatedPolicy, Policy? previousPolicyState, [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, @@ -554,70 +517,20 @@ public async Task ExecutePostUpsertSideEffectAsync_CacheHitUseMyItemsDisabled_Sh var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); var collectionRepository = Substitute.For(); - var organizationRepository = Substitute.For(); var organizationAbilityCacheService = Substitute.For(); organizationAbilityCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId) .Returns(new OrganizationAbility { Id = policyUpdate.OrganizationId, UseMyItems = false }); - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); + var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationAbilityCacheService, [factory]); var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); // Act await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState); - // Assert - Should shortcut based on the cached value alone, without ever calling the organization repository - await organizationRepository - .DidNotReceiveWithAnyArgs() - .GetByIdAsync(default); - + // Assert - Should shortcut based on the cached value alone await collectionRepository .DidNotReceiveWithAnyArgs() .CreateDefaultCollectionsBulkAsync(default, default, default); } - - [Theory] - [BitMemberAutoData(nameof(ShouldUpsertDefaultCollectionsTestCases))] - public async Task ExecutePostUpsertSideEffectAsync_CacheMiss_FallsBackToOrganizationRepository( - Policy postUpdatedPolicy, - Policy? previousPolicyState, - [PolicyUpdate(PolicyType.OrganizationDataOwnership)] PolicyUpdate policyUpdate, - [OrganizationPolicyDetails(PolicyType.OrganizationDataOwnership)] IEnumerable orgPolicyDetails, - OrganizationDataOwnershipPolicyRequirementFactory factory) - { - // Arrange - var orgPolicyDetailsList = orgPolicyDetails.ToList(); - foreach (var policyDetail in orgPolicyDetailsList) - { - policyDetail.OrganizationId = policyUpdate.OrganizationId; - } - - var policyRepository = ArrangePolicyRepository(orgPolicyDetailsList); - var collectionRepository = Substitute.For(); - var organizationRepository = Substitute.For(); - organizationRepository.GetByIdAsync(policyUpdate.OrganizationId) - .Returns(new Organization { Id = policyUpdate.OrganizationId, UseMyItems = true }); - - var organizationAbilityCacheService = Substitute.For(); - organizationAbilityCacheService.GetOrganizationAbilityAsync(policyUpdate.OrganizationId) - .Returns((OrganizationAbility?)null); - - var sut = new OrganizationDataOwnershipPolicyEventHandler(policyRepository, collectionRepository, organizationRepository, organizationAbilityCacheService, [factory]); - var policyRequest = new SavePolicyModel(policyUpdate, new OrganizationModelOwnershipPolicyModel(_defaultUserCollectionName)); - - // Act - await sut.ExecutePostUpsertSideEffectAsync(policyRequest, postUpdatedPolicy, previousPolicyState); - - // Assert - On a cache miss, the organization repository should be consulted as the source of truth - await organizationRepository - .Received(1) - .GetByIdAsync(policyUpdate.OrganizationId); - - await collectionRepository - .Received(1) - .CreateDefaultCollectionsBulkAsync( - policyUpdate.OrganizationId, - Arg.Is>(ids => ids.Count() == 3), - _defaultUserCollectionName); - } }