[EC-654] Create commands for Group Create and Group Update (#2442)

* [EC-654] Add CreateGroupCommand and UpdateGroupCommand

Added new CQRS commands CreateGroupCommand and UpdateGroupCommand
Updated GroupService to use new commands
Edited existing GroupServiceTests and added new tests for the new commands

* [EC-654] dotnet format

* [EC-654] Replace GroupService.SaveAsync with CreateGroup and UpdateGroup commands

* [EC-654] Add assertions to check calls on IReferenceEventService

* [EC-654] Use AssertHelper.AssertRecent for DateTime properties

* [EC-654] Extracted database reads from CreateGroupCommand and UpdateGroupCommand. Added unit tests.

* [EC-654] Changed CreateGroupCommand and UpdateGroupCommand Validate method to private

src/Api/Controllers/GroupsController.cs

@@ -2,6 +2,7 @@
 using Bit.Api.Models.Response;
 using Bit.Core.Context;
 using Bit.Core.Exceptions;
+using Bit.Core.OrganizationFeatures.Groups.Interfaces;
 using Bit.Core.Repositories;
 using Bit.Core.Services;
 using Microsoft.AspNetCore.Authorization;
@@ -15,16 +16,25 @@ public class GroupsController : Controller
 {
     private readonly IGroupRepository _groupRepository;
     private readonly IGroupService _groupService;
+    private readonly IOrganizationRepository _organizationRepository;
     private readonly ICurrentContext _currentContext;
+    private readonly ICreateGroupCommand _createGroupCommand;
+    private readonly IUpdateGroupCommand _updateGroupCommand;
 
     public GroupsController(
         IGroupRepository groupRepository,
         IGroupService groupService,
-        ICurrentContext currentContext)
+        IOrganizationRepository organizationRepository,
+        ICurrentContext currentContext,
+        ICreateGroupCommand createGroupCommand,
+        IUpdateGroupCommand updateGroupCommand)
     {
         _groupRepository = groupRepository;
         _groupService = groupService;
+        _organizationRepository = organizationRepository;
         _currentContext = currentContext;
+        _createGroupCommand = createGroupCommand;
+        _updateGroupCommand = updateGroupCommand;
     }
 
     [HttpGet("{id}")]
@@ -93,8 +103,10 @@ public class GroupsController : Controller
             throw new NotFoundException();
         }
 
+        var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
         var group = model.ToGroup(orgIdGuid);
-        await _groupService.SaveAsync(group, model.Collections?.Select(c => c.ToSelectionReadOnly()));
+        await _createGroupCommand.CreateGroupAsync(group, organization, model.Collections?.Select(c => c.ToSelectionReadOnly()));
+
         return new GroupResponseModel(group);
     }
 
@@ -108,7 +120,10 @@ public class GroupsController : Controller
             throw new NotFoundException();
         }
 
-        await _groupService.SaveAsync(model.ToGroup(group), model.Collections?.Select(c => c.ToSelectionReadOnly()));
+        var orgIdGuid = new Guid(orgId);
+        var organization = await _organizationRepository.GetByIdAsync(orgIdGuid);
+
+        await _updateGroupCommand.UpdateGroupAsync(model.ToGroup(group), organization, model.Collections?.Select(c => c.ToSelectionReadOnly()));
         return new GroupResponseModel(group);
     }
 

src/Api/Public/Controllers/GroupsController.cs

@@ -2,8 +2,8 @@
 using Bit.Api.Models.Public.Request;
 using Bit.Api.Models.Public.Response;
 using Bit.Core.Context;
+using Bit.Core.OrganizationFeatures.Groups.Interfaces;
 using Bit.Core.Repositories;
-using Bit.Core.Services;
 using Microsoft.AspNetCore.Authorization;
 using Microsoft.AspNetCore.Mvc;
 
@@ -14,17 +14,23 @@ namespace Bit.Api.Public.Controllers;
 public class GroupsController : Controller
 {
     private readonly IGroupRepository _groupRepository;
-    private readonly IGroupService _groupService;
+    private readonly IOrganizationRepository _organizationRepository;
     private readonly ICurrentContext _currentContext;
+    private readonly ICreateGroupCommand _createGroupCommand;
+    private readonly IUpdateGroupCommand _updateGroupCommand;
 
     public GroupsController(
         IGroupRepository groupRepository,
-        IGroupService groupService,
-        ICurrentContext currentContext)
+        IOrganizationRepository organizationRepository,
+        ICurrentContext currentContext,
+        ICreateGroupCommand createGroupCommand,
+        IUpdateGroupCommand updateGroupCommand)
     {
         _groupRepository = groupRepository;
-        _groupService = groupService;
+        _organizationRepository = organizationRepository;
         _currentContext = currentContext;
+        _createGroupCommand = createGroupCommand;
+        _updateGroupCommand = updateGroupCommand;
     }
 
     /// <summary>
@@ -104,7 +110,8 @@ public class GroupsController : Controller
     {
         var group = model.ToGroup(_currentContext.OrganizationId.Value);
         var associations = model.Collections?.Select(c => c.ToSelectionReadOnly());
-        await _groupService.SaveAsync(group, associations);
+        var organization = await _organizationRepository.GetByIdAsync(_currentContext.OrganizationId.Value);
+        await _createGroupCommand.CreateGroupAsync(group, organization, associations);
         var response = new GroupResponseModel(group, associations);
         return new JsonResult(response);
     }
@@ -129,9 +136,11 @@ public class GroupsController : Controller
         {
             return new NotFoundResult();
         }
+
         var updatedGroup = model.ToGroup(existingGroup);
         var associations = model.Collections?.Select(c => c.ToSelectionReadOnly());
-        await _groupService.SaveAsync(updatedGroup, associations);
+        var organization = await _organizationRepository.GetByIdAsync(_currentContext.OrganizationId.Value);
+        await _updateGroupCommand.UpdateGroupAsync(updatedGroup, organization, associations);
         var response = new GroupResponseModel(updatedGroup, associations);
         return new JsonResult(response);
     }

src/Core/OrganizationFeatures/Groups/CreateGroupCommand.cs

@@ -0,0 +1,72 @@
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+using Bit.Core.Exceptions;
+using Bit.Core.Models.Business;
+using Bit.Core.Models.Data;
+using Bit.Core.OrganizationFeatures.Groups.Interfaces;
+using Bit.Core.Repositories;
+using Bit.Core.Services;
+
+namespace Bit.Core.OrganizationFeatures.Groups;
+
+public class CreateGroupCommand : ICreateGroupCommand
+{
+    private readonly IEventService _eventService;
+    private readonly IGroupRepository _groupRepository;
+    private readonly IReferenceEventService _referenceEventService;
+
+    public CreateGroupCommand(
+        IEventService eventService,
+        IGroupRepository groupRepository,
+        IReferenceEventService referenceEventService)
+    {
+        _eventService = eventService;
+        _groupRepository = groupRepository;
+        _referenceEventService = referenceEventService;
+    }
+
+    public async Task CreateGroupAsync(Group group, Organization organization,
+        IEnumerable<SelectionReadOnly> collections = null)
+    {
+        Validate(organization);
+        await GroupRepositoryCreateGroupAsync(group, organization, collections);
+        await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Created);
+    }
+
+    public async Task CreateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
+        IEnumerable<SelectionReadOnly> collections = null)
+    {
+        Validate(organization);
+        await GroupRepositoryCreateGroupAsync(group, organization, collections);
+        await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Created, systemUser);
+    }
+
+    private async Task GroupRepositoryCreateGroupAsync(Group group, Organization organization, IEnumerable<SelectionReadOnly> collections = null)
+    {
+        group.CreationDate = group.RevisionDate = DateTime.UtcNow;
+
+        if (collections == null)
+        {
+            await _groupRepository.CreateAsync(group);
+        }
+        else
+        {
+            await _groupRepository.CreateAsync(group, collections);
+        }
+
+        await _referenceEventService.RaiseEventAsync(new ReferenceEvent(ReferenceEventType.GroupCreated, organization));
+    }
+
+    private static void Validate(Organization organization)
+    {
+        if (organization == null)
+        {
+            throw new BadRequestException("Organization not found");
+        }
+
+        if (!organization.UseGroups)
+        {
+            throw new BadRequestException("This organization cannot use groups.");
+        }
+    }
+}

src/Core/OrganizationFeatures/Groups/Interfaces/ICreateGroupCommand.cs

@@ -0,0 +1,14 @@
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+using Bit.Core.Models.Data;
+
+namespace Bit.Core.OrganizationFeatures.Groups.Interfaces;
+
+public interface ICreateGroupCommand
+{
+    Task CreateGroupAsync(Group group, Organization organization,
+        IEnumerable<SelectionReadOnly> collections = null);
+
+    Task CreateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
+        IEnumerable<SelectionReadOnly> collections = null);
+}

src/Core/OrganizationFeatures/Groups/Interfaces/IUpdateGroupCommand.cs

@@ -0,0 +1,14 @@
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+using Bit.Core.Models.Data;
+
+namespace Bit.Core.OrganizationFeatures.Groups.Interfaces;
+
+public interface IUpdateGroupCommand
+{
+    Task UpdateGroupAsync(Group group, Organization organization,
+        IEnumerable<SelectionReadOnly> collections = null);
+
+    Task UpdateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
+        IEnumerable<SelectionReadOnly> collections = null);
+}

src/Core/OrganizationFeatures/Groups/UpdateGroupCommand.cs

@@ -0,0 +1,66 @@
+using Bit.Core.Entities;
+using Bit.Core.Enums;
+using Bit.Core.Exceptions;
+using Bit.Core.Models.Data;
+using Bit.Core.OrganizationFeatures.Groups.Interfaces;
+using Bit.Core.Repositories;
+using Bit.Core.Services;
+
+namespace Bit.Core.OrganizationFeatures.Groups;
+
+public class UpdateGroupCommand : IUpdateGroupCommand
+{
+    private readonly IEventService _eventService;
+    private readonly IGroupRepository _groupRepository;
+
+    public UpdateGroupCommand(
+        IEventService eventService,
+        IGroupRepository groupRepository)
+    {
+        _eventService = eventService;
+        _groupRepository = groupRepository;
+    }
+
+    public async Task UpdateGroupAsync(Group group, Organization organization,
+        IEnumerable<SelectionReadOnly> collections = null)
+    {
+        Validate(organization);
+        await GroupRepositoryUpdateGroupAsync(group, collections);
+        await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Updated);
+    }
+
+    public async Task UpdateGroupAsync(Group group, Organization organization, EventSystemUser systemUser,
+        IEnumerable<SelectionReadOnly> collections = null)
+    {
+        Validate(organization);
+        await GroupRepositoryUpdateGroupAsync(group, collections);
+        await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Updated, systemUser);
+    }
+
+    private async Task GroupRepositoryUpdateGroupAsync(Group group, IEnumerable<SelectionReadOnly> collections = null)
+    {
+        group.RevisionDate = DateTime.UtcNow;
+
+        if (collections == null)
+        {
+            await _groupRepository.ReplaceAsync(group);
+        }
+        else
+        {
+            await _groupRepository.ReplaceAsync(group, collections);
+        }
+    }
+
+    private static void Validate(Organization organization)
+    {
+        if (organization == null)
+        {
+            throw new BadRequestException("Organization not found");
+        }
+
+        if (!organization.UseGroups)
+        {
+            throw new BadRequestException("This organization cannot use groups.");
+        }
+    }
+}

src/Core/OrganizationFeatures/OrganizationServiceCollectionExtensions.cs

@@ -1,4 +1,6 @@
 using Bit.Core.Models.Business.Tokenables;
+using Bit.Core.OrganizationFeatures.Groups;
+using Bit.Core.OrganizationFeatures.Groups.Interfaces;
 using Bit.Core.OrganizationFeatures.OrganizationApiKeys;
 using Bit.Core.OrganizationFeatures.OrganizationApiKeys.Interfaces;
 using Bit.Core.OrganizationFeatures.OrganizationConnections;
@@ -22,11 +24,19 @@ public static class OrganizationServiceCollectionExtensions
     {
         services.AddScoped<IOrganizationService, OrganizationService>();
         services.AddTokenizers();
+        services.AddOrganizationGroupCommands();
         services.AddOrganizationConnectionCommands();
         services.AddOrganizationSponsorshipCommands(globalSettings);
         services.AddOrganizationApiKeyCommandsQueries();
     }
 
+    private static void AddOrganizationGroupCommands(this IServiceCollection services)
+    {
+        services.AddScoped<ICreateGroupCommand, CreateGroupCommand>();
+        services.AddScoped<IDeleteGroupCommand, DeleteGroupCommand>();
+        services.AddScoped<IUpdateGroupCommand, UpdateGroupCommand>();
+    }
+
     private static void AddOrganizationConnectionCommands(this IServiceCollection services)
     {
         services.AddScoped<ICreateOrganizationConnectionCommand, CreateOrganizationConnectionCommand>();

src/Core/Services/IGroupService.cs

@@ -1,13 +1,10 @@
 using Bit.Core.Entities;
 using Bit.Core.Enums;
-using Bit.Core.Models.Data;
 
 namespace Bit.Core.Services;
 
 public interface IGroupService
 {
-    Task SaveAsync(Group group, IEnumerable<SelectionReadOnly> collections = null);
-    Task SaveAsync(Group group, EventSystemUser systemUser, IEnumerable<SelectionReadOnly> collections = null);
     [Obsolete("IDeleteGroupCommand should be used instead. To be removed by EC-608.")]
     Task DeleteAsync(Group group);
     [Obsolete("IDeleteGroupCommand should be used instead. To be removed by EC-608.")]

src/Core/Services/Implementations/GroupService.cs

@@ -1,8 +1,6 @@
 using Bit.Core.Entities;
 using Bit.Core.Enums;
 using Bit.Core.Exceptions;
-using Bit.Core.Models.Business;
-using Bit.Core.Models.Data;
 using Bit.Core.Repositories;
 
 namespace Bit.Core.Services;
@@ -10,96 +8,17 @@ namespace Bit.Core.Services;
 public class GroupService : IGroupService
 {
     private readonly IEventService _eventService;
-    private readonly IOrganizationRepository _organizationRepository;
     private readonly IOrganizationUserRepository _organizationUserRepository;
     private readonly IGroupRepository _groupRepository;
-    private readonly IReferenceEventService _referenceEventService;
 
     public GroupService(
         IEventService eventService,
-        IOrganizationRepository organizationRepository,
         IOrganizationUserRepository organizationUserRepository,
-        IGroupRepository groupRepository,
-        IReferenceEventService referenceEventService)
+        IGroupRepository groupRepository)
     {
         _eventService = eventService;
-        _organizationRepository = organizationRepository;
         _organizationUserRepository = organizationUserRepository;
         _groupRepository = groupRepository;
-        _referenceEventService = referenceEventService;
-    }
-
-    public async Task SaveAsync(Group group,
-        IEnumerable<SelectionReadOnly> collections = null)
-    {
-        await GroupRepositorySaveAsync(group, systemUser: null, collections);
-    }
-
-    public async Task SaveAsync(Group group, EventSystemUser systemUser,
-        IEnumerable<SelectionReadOnly> collections = null)
-    {
-        await GroupRepositorySaveAsync(group, systemUser, collections);
-    }
-
-    private async Task GroupRepositorySaveAsync(Group group, EventSystemUser? systemUser, IEnumerable<SelectionReadOnly> collections = null)
-    {
-        var org = await _organizationRepository.GetByIdAsync(group.OrganizationId);
-        if (org == null)
-        {
-            throw new BadRequestException("Organization not found");
-        }
-
-        if (!org.UseGroups)
-        {
-            throw new BadRequestException("This organization cannot use groups.");
-        }
-
-        if (group.Id == default(Guid))
-        {
-            group.CreationDate = group.RevisionDate = DateTime.UtcNow;
-
-            if (collections == null)
-            {
-                await _groupRepository.CreateAsync(group);
-            }
-            else
-            {
-                await _groupRepository.CreateAsync(group, collections);
-            }
-
-            if (systemUser.HasValue)
-            {
-                await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Created, systemUser.Value);
-            }
-            else
-            {
-                await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Created);
-            }
-
-            await _referenceEventService.RaiseEventAsync(new ReferenceEvent(ReferenceEventType.GroupCreated, org));
-        }
-        else
-        {
-            group.RevisionDate = DateTime.UtcNow;
-
-            if (collections == null)
-            {
-                await _groupRepository.ReplaceAsync(group);
-            }
-            else
-            {
-                await _groupRepository.ReplaceAsync(group, collections);
-            }
-
-            if (systemUser.HasValue)
-            {
-                await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Updated, systemUser.Value);
-            }
-            else
-            {
-                await _eventService.LogGroupEventAsync(group, Enums.EventType.Group_Updated);
-            }
-        }
     }
 
     [Obsolete("IDeleteGroupCommand should be used instead. To be removed by EC-608.")]