From ff475afc2ff9f0bb5c47a84d9a0437c06f380095 Mon Sep 17 00:00:00 2001 From: Rui Tome Date: Thu, 12 Jun 2025 14:49:38 +0100 Subject: [PATCH] Refactor ICollectionService and CollectionService to remove SaveAsync method * Removed the SaveAsync method from ICollectionService and its implementation in CollectionService. * Updated related tests in CollectionServiceTests to reflect the removal of SaveAsync, ensuring existing functionality remains intact. --- src/Core/Services/ICollectionService.cs | 2 - .../Implementations/CollectionService.cs | 52 -------- .../Services/CollectionServiceTests.cs | 124 ------------------ 3 files changed, 178 deletions(-) diff --git a/src/Core/Services/ICollectionService.cs b/src/Core/Services/ICollectionService.cs index c116e5f076..101f3ea23b 100644 --- a/src/Core/Services/ICollectionService.cs +++ b/src/Core/Services/ICollectionService.cs @@ -1,10 +1,8 @@ using Bit.Core.Entities; -using Bit.Core.Models.Data; namespace Bit.Core.Services; public interface ICollectionService { - Task SaveAsync(Collection collection, IEnumerable groups = null, IEnumerable users = null); Task DeleteUserAsync(Collection collection, Guid organizationUserId); } diff --git a/src/Core/Services/Implementations/CollectionService.cs b/src/Core/Services/Implementations/CollectionService.cs index 852f2da073..2a3f8c42dc 100644 --- a/src/Core/Services/Implementations/CollectionService.cs +++ b/src/Core/Services/Implementations/CollectionService.cs @@ -2,7 +2,6 @@ using Bit.Core.Entities; using Bit.Core.Exceptions; -using Bit.Core.Models.Data; using Bit.Core.Repositories; namespace Bit.Core.Services; @@ -10,71 +9,20 @@ namespace Bit.Core.Services; public class CollectionService : ICollectionService { private readonly IEventService _eventService; - private readonly IOrganizationRepository _organizationRepository; private readonly IOrganizationUserRepository _organizationUserRepository; private readonly ICollectionRepository _collectionRepository; public CollectionService( IEventService eventService, - IOrganizationRepository organizationRepository, IOrganizationUserRepository organizationUserRepository, ICollectionRepository collectionRepository) { _eventService = eventService; - _organizationRepository = organizationRepository; _organizationUserRepository = organizationUserRepository; _collectionRepository = collectionRepository; } - public async Task SaveAsync(Collection collection, IEnumerable? groups = null, - IEnumerable? users = null) - { - var org = await _organizationRepository.GetByIdAsync(collection.OrganizationId); - if (org == null) - { - throw new BadRequestException("Organization not found"); - } - var groupsList = groups?.ToList(); - var usersList = users?.ToList(); - - // Cannot use Manage with ReadOnly/HidePasswords permissions - var invalidAssociations = groupsList?.Where(cas => cas.Manage && (cas.ReadOnly || cas.HidePasswords)); - if (invalidAssociations?.Any() ?? false) - { - throw new BadRequestException("The Manage property is mutually exclusive and cannot be true while the ReadOnly or HidePasswords properties are also true."); - } - - // A collection should always have someone with Can Manage permissions - var groupHasManageAccess = groupsList?.Any(g => g.Manage) ?? false; - var userHasManageAccess = usersList?.Any(u => u.Manage) ?? false; - if (!groupHasManageAccess && !userHasManageAccess && !org.AllowAdminAccessToAllCollectionItems) - { - throw new BadRequestException( - "At least one member or group must have can manage permission."); - } - - if (collection.Id == default(Guid)) - { - if (org.MaxCollections.HasValue) - { - var collectionCount = await _collectionRepository.GetCountByOrganizationIdAsync(org.Id); - if (org.MaxCollections.Value <= collectionCount) - { - throw new BadRequestException("You have reached the maximum number of collections " + - $"({org.MaxCollections.Value}) for this organization."); - } - } - - await _collectionRepository.CreateAsync(collection, org.UseGroups ? groupsList : null, usersList); - await _eventService.LogCollectionEventAsync(collection, Enums.EventType.Collection_Created); - } - else - { - await _collectionRepository.ReplaceAsync(collection, org.UseGroups ? groupsList : null, usersList); - await _eventService.LogCollectionEventAsync(collection, Enums.EventType.Collection_Updated); - } - } public async Task DeleteUserAsync(Collection collection, Guid organizationUserId) { diff --git a/test/Core.Test/Services/CollectionServiceTests.cs b/test/Core.Test/Services/CollectionServiceTests.cs index 6d788deb05..6963337ced 100644 --- a/test/Core.Test/Services/CollectionServiceTests.cs +++ b/test/Core.Test/Services/CollectionServiceTests.cs @@ -2,10 +2,8 @@ using Bit.Core.Entities; using Bit.Core.Enums; using Bit.Core.Exceptions; -using Bit.Core.Models.Data; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Test.AutoFixture; using Bit.Core.Test.AutoFixture.OrganizationFixtures; using Bit.Test.Common.AutoFixture; using Bit.Test.Common.AutoFixture.Attributes; @@ -18,128 +16,6 @@ namespace Bit.Core.Test.Services; [OrganizationCustomize] public class CollectionServiceTest { - [Theory, BitAutoData] - public async Task SaveAsync_DefaultIdWithUsers_CreatesCollectionInTheRepository(Collection collection, Organization organization, [CollectionAccessSelectionCustomize(true)] IEnumerable users, SutProvider sutProvider) - { - collection.Id = default; - sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - var utcNow = DateTime.UtcNow; - - await sutProvider.Sut.SaveAsync(collection, null, users); - - await sutProvider.GetDependency().Received() - .CreateAsync(collection, Arg.Is>(l => l == null), - Arg.Is>(l => l.Any(i => i.Manage == true))); - await sutProvider.GetDependency().Received() - .LogCollectionEventAsync(collection, EventType.Collection_Created); - Assert.True(collection.CreationDate - utcNow < TimeSpan.FromSeconds(1)); - Assert.True(collection.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory, BitAutoData] - public async Task SaveAsync_DefaultIdWithGroupsAndUsers_CreateCollectionWithGroupsAndUsersInRepository(Collection collection, - [CollectionAccessSelectionCustomize(true)] IEnumerable groups, IEnumerable users, Organization organization, SutProvider sutProvider) - { - collection.Id = default; - organization.UseGroups = true; - sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - var utcNow = DateTime.UtcNow; - - await sutProvider.Sut.SaveAsync(collection, groups, users); - - await sutProvider.GetDependency().Received() - .CreateAsync(collection, Arg.Is>(l => l.Any(i => i.Manage == true)), - Arg.Any>()); - await sutProvider.GetDependency().Received() - .LogCollectionEventAsync(collection, EventType.Collection_Created); - Assert.True(collection.CreationDate - utcNow < TimeSpan.FromSeconds(1)); - Assert.True(collection.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory, BitAutoData] - public async Task SaveAsync_NonDefaultId_ReplacesCollectionInRepository(Collection collection, Organization organization, [CollectionAccessSelectionCustomize(true)] IEnumerable users, SutProvider sutProvider) - { - var creationDate = collection.CreationDate; - sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - var utcNow = DateTime.UtcNow; - - await sutProvider.Sut.SaveAsync(collection, null, users); - - await sutProvider.GetDependency().Received().ReplaceAsync(collection, - Arg.Is>(l => l == null), - Arg.Is>(l => l.Any(i => i.Manage == true))); - await sutProvider.GetDependency().Received() - .LogCollectionEventAsync(collection, EventType.Collection_Updated); - Assert.Equal(collection.CreationDate, creationDate); - Assert.True(collection.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory, BitAutoData] - public async Task SaveAsync_OrganizationNotUseGroup_CreateCollectionWithoutGroupsInRepository(Collection collection, - [CollectionAccessSelectionCustomize] IEnumerable groups, [CollectionAccessSelectionCustomize(true)] IEnumerable users, - Organization organization, SutProvider sutProvider) - { - collection.Id = default; - organization.UseGroups = false; - sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - var utcNow = DateTime.UtcNow; - - await sutProvider.Sut.SaveAsync(collection, groups, users); - - await sutProvider.GetDependency().Received().CreateAsync(collection, - Arg.Is>(l => l == null), - Arg.Is>(l => l.Any(i => i.Manage == true))); - await sutProvider.GetDependency().Received() - .LogCollectionEventAsync(collection, EventType.Collection_Created); - Assert.True(collection.CreationDate - utcNow < TimeSpan.FromSeconds(1)); - Assert.True(collection.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); - } - - [Theory, BitAutoData] - public async Task SaveAsync_NonExistingOrganizationId_ThrowsBadRequest(Collection collection, SutProvider sutProvider) - { - var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.SaveAsync(collection)); - Assert.Contains("Organization not found", ex.Message); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default, default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCollectionEventAsync(default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_NoManageAccess_ThrowsBadRequest(Collection collection, Organization organization, - [CollectionAccessSelectionCustomize] IEnumerable users, SutProvider sutProvider) - { - collection.Id = default; - sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - organization.AllowAdminAccessToAllCollectionItems = false; - - var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.SaveAsync(collection, null, users)); - Assert.Contains("At least one member or group must have can manage permission.", ex.Message); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default, default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCollectionEventAsync(default, default); - } - - [Theory, BitAutoData] - public async Task SaveAsync_ExceedsOrganizationMaxCollections_ThrowsBadRequest(Collection collection, - Organization organization, [CollectionAccessSelectionCustomize(true)] IEnumerable users, - SutProvider sutProvider) - { - collection.Id = default; - sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); - sutProvider.GetDependency().GetCountByOrganizationIdAsync(organization.Id) - .Returns(organization.MaxCollections.Value); - - var ex = await Assert.ThrowsAsync(() => sutProvider.Sut.SaveAsync(collection, null, users)); - Assert.Equal($@"You have reached the maximum number of collections ({organization.MaxCollections.Value}) for this organization.", ex.Message); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default, default, default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); - await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogCollectionEventAsync(default, default); - } - [Theory, BitAutoData] public async Task DeleteUserAsync_DeletesValidUserWhoBelongsToCollection(Collection collection, Organization organization, OrganizationUser organizationUser, SutProvider sutProvider)