From 69c2673f1fb97fdd828f62caeb871208ae6326b7 Mon Sep 17 00:00:00 2001 From: Sang <11912225+hantatsang@users.noreply.github.com> Date: Wed, 12 May 2021 12:36:23 +1000 Subject: [PATCH] Write GroupService unit tests (#1267) * Write GroupService tests * Rewrite with AutoFixture, improve tests * Resolve PR comments * Rename OrganizationCustomization Co-authored-by: Matt Gibson --- test/Core.Test/AutoFixture/GroupFixtures.cs | 19 +++ .../AutoFixture/OrganizationFixtures.cs | 30 +++- test/Core.Test/Services/GroupServiceTests.cs | 144 +++++++++++++++--- .../Services/OrganizationServiceTests.cs | 48 +++--- 4 files changed, 187 insertions(+), 54 deletions(-) create mode 100644 test/Core.Test/AutoFixture/GroupFixtures.cs diff --git a/test/Core.Test/AutoFixture/GroupFixtures.cs b/test/Core.Test/AutoFixture/GroupFixtures.cs new file mode 100644 index 0000000000..cb780761f0 --- /dev/null +++ b/test/Core.Test/AutoFixture/GroupFixtures.cs @@ -0,0 +1,19 @@ +using Bit.Core.Test.AutoFixture.Attributes; +using Bit.Core.Test.AutoFixture.OrganizationFixtures; + +namespace Bit.Core.Test.AutoFixture +{ + internal class GroupOrganizationAutoDataAttribute : CustomAutoDataAttribute + { + public GroupOrganizationAutoDataAttribute() : base( + new SutProviderCustomization(), new Organization { UseGroups = true }) + { } + } + + internal class GroupOrganizationNotUseGroupsAutoDataAttribute : CustomAutoDataAttribute + { + public GroupOrganizationNotUseGroupsAutoDataAttribute() : base( + new SutProviderCustomization(), new Organization { UseGroups = false }) + { } + } +} diff --git a/test/Core.Test/AutoFixture/OrganizationFixtures.cs b/test/Core.Test/AutoFixture/OrganizationFixtures.cs index f3723f895a..246f7c3436 100644 --- a/test/Core.Test/AutoFixture/OrganizationFixtures.cs +++ b/test/Core.Test/AutoFixture/OrganizationFixtures.cs @@ -12,6 +12,22 @@ using Bit.Core.Utilities; namespace Bit.Core.Test.AutoFixture.OrganizationFixtures { + public class Organization : ICustomization + { + public bool UseGroups { get; set; } + + public void Customize(IFixture fixture) + { + var organizationId = Guid.NewGuid(); + + fixture.Customize(composer => composer + .With(o => o.Id, organizationId) + .With(o => o.UseGroups, UseGroups)); + + fixture.Customize(composer => composer.With(g => g.OrganizationId, organizationId)); + } + } + internal class PaidOrganization : ICustomization { public PlanType CheckedPlanType { get; set; } @@ -21,7 +37,7 @@ namespace Bit.Core.Test.AutoFixture.OrganizationFixtures var lowestActivePaidPlan = validUpgradePlans.First(); CheckedPlanType = CheckedPlanType.Equals(Enums.PlanType.Free) ? lowestActivePaidPlan : CheckedPlanType; validUpgradePlans.Remove(lowestActivePaidPlan); - fixture.Customize(composer => composer + fixture.Customize(composer => composer .With(o => o.PlanType, CheckedPlanType)); fixture.Customize(composer => composer .With(ou => ou.Plan, validUpgradePlans.First())); @@ -32,7 +48,7 @@ namespace Bit.Core.Test.AutoFixture.OrganizationFixtures { public void Customize(IFixture fixture) { - fixture.Customize(composer => composer + fixture.Customize(composer => composer .With(o => o.PlanType, PlanType.Free)); var plansToIgnore = new List { PlanType.Free, PlanType.Custom }; @@ -57,7 +73,7 @@ namespace Bit.Core.Test.AutoFixture.OrganizationFixtures { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, }); - fixture.Customize(composer => composer + fixture.Customize(composer => composer .With(o => o.Id, organizationId) .With(o => o.Seats, (short)100)); fixture.Customize(composer => composer @@ -99,10 +115,10 @@ namespace Bit.Core.Test.AutoFixture.OrganizationFixtures internal class OrganizationInviteAutoDataAttribute : CustomAutoDataAttribute { public OrganizationInviteAutoDataAttribute(int inviteeUserType = 0, int invitorUserType = 0, string permissionsBlob = null) : base(new SutProviderCustomization(), - new OrganizationInvite - { - InviteeUserType = (OrganizationUserType)inviteeUserType, - InvitorUserType = (OrganizationUserType)invitorUserType, + new OrganizationInvite + { + InviteeUserType = (OrganizationUserType)inviteeUserType, + InvitorUserType = (OrganizationUserType)invitorUserType, PermissionsBlob = permissionsBlob, }) { } diff --git a/test/Core.Test/Services/GroupServiceTests.cs b/test/Core.Test/Services/GroupServiceTests.cs index 2301caf01b..3cb14f85a1 100644 --- a/test/Core.Test/Services/GroupServiceTests.cs +++ b/test/Core.Test/Services/GroupServiceTests.cs @@ -1,6 +1,14 @@ using System; +using System.Collections.Generic; +using System.Threading.Tasks; +using Bit.Core.Enums; +using Bit.Core.Exceptions; +using Bit.Core.Models.Data; +using Bit.Core.Models.Table; using Bit.Core.Repositories; using Bit.Core.Services; +using Bit.Core.Test.AutoFixture; +using Bit.Core.Test.AutoFixture.Attributes; using NSubstitute; using Xunit; @@ -8,34 +16,124 @@ namespace Bit.Core.Test.Services { public class GroupServiceTests { - private readonly GroupService _sut; - - private readonly IEventService _eventService; - private readonly IOrganizationRepository _organizationRepository; - private readonly IOrganizationUserRepository _organizationUserRepository; - private readonly IGroupRepository _groupRepository; - - public GroupServiceTests() + [Theory, GroupOrganizationAutoData] + public async Task SaveAsync_DefaultGroupId_CreatesGroupInRepository(Group group, Organization organization, SutProvider sutProvider) { - _eventService = Substitute.For(); - _organizationRepository = Substitute.For(); - _organizationUserRepository = Substitute.For(); - _groupRepository = Substitute.For(); + group.Id = default(Guid); + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + organization.UseGroups = true; + var utcNow = DateTime.UtcNow; - _sut = new GroupService( - _eventService, - _organizationRepository, - _organizationUserRepository, - _groupRepository - ); + await sutProvider.Sut.SaveAsync(group); + + await sutProvider.GetDependency().Received().CreateAsync(group); + await sutProvider.GetDependency().Received() + .LogGroupEventAsync(group, EventType.Group_Created); + Assert.True(group.CreationDate - utcNow < TimeSpan.FromSeconds(1)); + Assert.True(group.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); } - // Remove this test when we add actual tests. It only proves that - // we've properly constructed the system under test. - [Fact] - public void ServiceExists() + [Theory, GroupOrganizationAutoData] + public async Task SaveAsync_DefaultGroupIdAndCollections_CreatesGroupInRepository(Group group, Organization organization, List collections, SutProvider sutProvider) { - Assert.NotNull(_sut); + group.Id = default(Guid); + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + organization.UseGroups = true; + var utcNow = DateTime.UtcNow; + + await sutProvider.Sut.SaveAsync(group, collections); + + await sutProvider.GetDependency().Received().CreateAsync(group, collections); + await sutProvider.GetDependency().Received() + .LogGroupEventAsync(group, EventType.Group_Created); + Assert.True(group.CreationDate - utcNow < TimeSpan.FromSeconds(1)); + Assert.True(group.RevisionDate - utcNow < TimeSpan.FromSeconds(1)); + } + + [Theory, GroupOrganizationAutoData] + public async Task SaveAsync_NonDefaultGroupId_ReplaceGroupInRepository(Group group, Organization organization, List collections, SutProvider sutProvider) + { + organization.UseGroups = true; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + await sutProvider.Sut.SaveAsync(group, collections); + + await sutProvider.GetDependency().Received().ReplaceAsync(group, collections); + await sutProvider.GetDependency().Received() + .LogGroupEventAsync(group, EventType.Group_Updated); + Assert.True(group.RevisionDate - DateTime.UtcNow < TimeSpan.FromSeconds(1)); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task SaveAsync_NonExistingOrganizationId_ThrowsBadRequest(Group group, Organization organization, SutProvider sutProvider) + { + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(group)); + Assert.Contains("Organization not found", exception.Message); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogGroupEventAsync(default, default, default); + } + + [Theory, GroupOrganizationNotUseGroupsAutoData] + public async Task SaveAsync_OrganizationDoesNotUseGroups_ThrowsBadRequest(Group group, Organization organization, SutProvider sutProvider) + { + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + + var exception = await Assert.ThrowsAsync( + () => sutProvider.Sut.SaveAsync(group)); + + Assert.Contains("This organization cannot use groups", exception.Message); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().CreateAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().ReplaceAsync(default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs().LogGroupEventAsync(default, default, default); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteAsync_ValidData_DeletesGroup(Group group, SutProvider sutProvider) + { + await sutProvider.Sut.DeleteAsync(group); + + await sutProvider.GetDependency().Received().DeleteAsync(group); + await sutProvider.GetDependency().Received() + .LogGroupEventAsync(group, EventType.Group_Deleted); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUserAsync_ValidData_DeletesUserInGroupRepository(Group group, Organization organization, OrganizationUser organizationUser, SutProvider sutProvider) + { + group.OrganizationId = organization.Id; + organization.UseGroups = true; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + organizationUser.OrganizationId = organization.Id; + sutProvider.GetDependency().GetByIdAsync(organizationUser.Id) + .Returns(organizationUser); + + await sutProvider.Sut.DeleteUserAsync(group, organizationUser.Id); + + await sutProvider.GetDependency().Received().DeleteUserAsync(group.Id, organizationUser.Id); + await sutProvider.GetDependency().Received() + .LogOrganizationUserEventAsync(organizationUser, EventType.OrganizationUser_UpdatedGroups); + } + + [Theory, CustomAutoData(typeof(SutProviderCustomization))] + public async Task DeleteUserAsync_InvalidUser_ThrowsNotFound(Group group, Organization organization, OrganizationUser organizationUser, SutProvider sutProvider) + { + group.OrganizationId = organization.Id; + organization.UseGroups = true; + sutProvider.GetDependency().GetByIdAsync(organization.Id).Returns(organization); + // organizationUser.OrganizationId = organization.Id; + sutProvider.GetDependency().GetByIdAsync(organizationUser.Id) + .Returns(organizationUser); + + // user not in organization + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteUserAsync(group, organizationUser.Id)); + // invalid user + await Assert.ThrowsAsync(() => sutProvider.Sut.DeleteUserAsync(group, Guid.NewGuid())); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .DeleteUserAsync(default, default); + await sutProvider.GetDependency().DidNotReceiveWithAnyArgs() + .LogOrganizationUserEventAsync(default, default); } } } diff --git a/test/Core.Test/Services/OrganizationServiceTests.cs b/test/Core.Test/Services/OrganizationServiceTests.cs index 016af4e6c0..bc906a5021 100644 --- a/test/Core.Test/Services/OrganizationServiceTests.cs +++ b/test/Core.Test/Services/OrganizationServiceTests.cs @@ -6,7 +6,6 @@ using Bit.Core.Models.Table; using Bit.Core.Models.Business; using Bit.Core.Repositories; using Bit.Core.Services; -using Bit.Core.Settings; using Microsoft.AspNetCore.DataProtection; using NSubstitute; using Xunit; @@ -16,6 +15,7 @@ using Bit.Core.Enums; using Bit.Core.Test.AutoFixture.Attributes; using Bit.Core.Test.AutoFixture.OrganizationFixtures; using System.Text.Json; +using Organization = Bit.Core.Models.Table.Organization; namespace Bit.Core.Test.Services { @@ -187,7 +187,7 @@ namespace Bit.Core.Test.Services () => sutProvider.Sut.UpgradePlanAsync(organization.Id, upgrade)); Assert.Contains("can only upgrade", exception.Message); } - + [Theory] [FreeOrganizationUpgradeAutoData] public async Task UpgradePlan_Passes(Organization organization, OrganizationUpgrade upgrade, @@ -209,12 +209,12 @@ namespace Bit.Core.Test.Services () => sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite)); } - [Theory] + [Theory] [OrganizationInviteAutoData( - inviteeUserType: (int)OrganizationUserType.Owner, + inviteeUserType: (int)OrganizationUserType.Owner, invitorUserType: (int)OrganizationUserType.Admin )] - public async Task InviteUser_NonOwnerConfiguringOwner_Throws(Organization organization, OrganizationUserInvite invite, + public async Task InviteUser_NonOwnerConfiguringOwner_Throws(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); @@ -228,12 +228,12 @@ namespace Bit.Core.Test.Services Assert.Contains("only an owner", exception.Message.ToLowerInvariant()); } - [Theory] + [Theory] [OrganizationInviteAutoData( - inviteeUserType: (int)OrganizationUserType.Custom, + inviteeUserType: (int)OrganizationUserType.Custom, invitorUserType: (int)OrganizationUserType.Admin )] - public async Task InviteUser_NonAdminConfiguringAdmin_Throws(Organization organization, OrganizationUserInvite invite, + public async Task InviteUser_NonAdminConfiguringAdmin_Throws(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { var organizationRepository = sutProvider.GetDependency(); @@ -247,15 +247,15 @@ namespace Bit.Core.Test.Services Assert.Contains("only owners and admins", exception.Message.ToLowerInvariant()); } - [Theory] + [Theory] [OrganizationInviteAutoData( - inviteeUserType: (int)OrganizationUserType.Manager, + inviteeUserType: (int)OrganizationUserType.Manager, invitorUserType: (int)OrganizationUserType.Custom )] - public async Task InviteUser_CustomUserWithoutManageUsersConfiguringUser_Throws(Organization organization, OrganizationUserInvite invite, + public async Task InviteUser_CustomUserWithoutManageUsersConfiguringUser_Throws(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { - invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = false }, + invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = false }, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, @@ -272,15 +272,15 @@ namespace Bit.Core.Test.Services Assert.Contains("account does not have permission", exception.Message.ToLowerInvariant()); } - [Theory] + [Theory] [OrganizationInviteAutoData( - inviteeUserType: (int)OrganizationUserType.Admin, + inviteeUserType: (int)OrganizationUserType.Admin, invitorUserType: (int)OrganizationUserType.Custom )] - public async Task InviteUser_CustomUserConfiguringAdmin_Throws(Organization organization, OrganizationUserInvite invite, + public async Task InviteUser_CustomUserConfiguringAdmin_Throws(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { - invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = true }, + invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = true }, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, @@ -297,12 +297,12 @@ namespace Bit.Core.Test.Services Assert.Contains("can not manage admins", exception.Message.ToLowerInvariant()); } - [Theory] + [Theory] [OrganizationInviteAutoData( - inviteeUserType: (int)OrganizationUserType.User, + inviteeUserType: (int)OrganizationUserType.User, invitorUserType: (int)OrganizationUserType.Owner )] - public async Task InviteUser_NoPermissionsObject_Passes(Organization organization, OrganizationUserInvite invite, + public async Task InviteUser_NoPermissionsObject_Passes(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { invite.Permissions = null; @@ -316,15 +316,15 @@ namespace Bit.Core.Test.Services await sutProvider.Sut.InviteUserAsync(organization.Id, invitor.UserId, null, invite); } - [Theory] + [Theory] [OrganizationInviteAutoData( - inviteeUserType: (int)OrganizationUserType.User, + inviteeUserType: (int)OrganizationUserType.User, invitorUserType: (int)OrganizationUserType.Custom )] - public async Task InviteUser_Passes(Organization organization, OrganizationUserInvite invite, + public async Task InviteUser_Passes(Organization organization, OrganizationUserInvite invite, OrganizationUser invitor, SutProvider sutProvider) { - invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = true }, + invitor.Permissions = JsonSerializer.Serialize(new Permissions() { ManageUsers = true }, new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase, @@ -366,7 +366,7 @@ namespace Bit.Core.Test.Services IEnumerable collections, OrganizationUser savingUser, SutProvider sutProvider) { var organizationUserRepository = sutProvider.GetDependency(); - + newUserData.Id = oldUserData.Id; newUserData.UserId = oldUserData.UserId; newUserData.OrganizationId = savingUser.OrganizationId = oldUserData.OrganizationId;