mirror of
https://github.com/Sea-Haven-Industries/shoc-backend.git
synced 2026-10-07 02:22:11 +00:00
fix: make team member creation atomic
This commit is contained in:
parent
f7db6b9f84
commit
3b626cca58
5 changed files with 273 additions and 31 deletions
|
|
@ -319,7 +319,7 @@ public sealed class TeamMemberServiceTests
|
||||||
[Fact]
|
[Fact]
|
||||||
public async Task Create_ConcurrentDuplicateEmail_ReturnsAlreadyInUseInsteadOf500()
|
public async Task Create_ConcurrentDuplicateEmail_ReturnsAlreadyInUseInsteadOf500()
|
||||||
{
|
{
|
||||||
var service = NewService(out var userManager, out var roleManager, out _, out _, out _, out _);
|
var service = NewService(out var userManager, out var roleManager, out _, out _, out var userData, out _);
|
||||||
userManager
|
userManager
|
||||||
.Setup(manager => manager.FindByEmailAsync(It.IsAny<string>()))
|
.Setup(manager => manager.FindByEmailAsync(It.IsAny<string>()))
|
||||||
.ReturnsAsync((ApplicationUser?)null);
|
.ReturnsAsync((ApplicationUser?)null);
|
||||||
|
|
@ -332,10 +332,57 @@ public sealed class TeamMemberServiceTests
|
||||||
|
|
||||||
result.Success.Should().BeFalse();
|
result.Success.Should().BeFalse();
|
||||||
result.Error.Should().Be("Email is already in use.");
|
result.Error.Should().Be("Email is already in use.");
|
||||||
|
userData.Verify(
|
||||||
|
data => data.ExecuteTransactionalAsync(It.IsAny<Func<CancellationToken, Task>>(), It.IsAny<CancellationToken>()),
|
||||||
|
Times.Once);
|
||||||
userManager.Verify(manager => manager.AddToRoleAsync(It.IsAny<ApplicationUser>(), It.IsAny<string>()), Times.Never);
|
userManager.Verify(manager => manager.AddToRoleAsync(It.IsAny<ApplicationUser>(), It.IsAny<string>()), Times.Never);
|
||||||
userManager.Verify(manager => manager.DeleteAsync(It.IsAny<ApplicationUser>()), Times.Never);
|
userManager.Verify(manager => manager.DeleteAsync(It.IsAny<ApplicationUser>()), Times.Never);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
[Fact]
|
||||||
|
public async Task Create_MidSequencePersistenceFailure_PropagatesWithoutCompensation()
|
||||||
|
{
|
||||||
|
var service = NewService(out var userManager, out var roleManager, out var areas, out var overrides, out _, out _);
|
||||||
|
ApplicationUser? created = null;
|
||||||
|
userManager
|
||||||
|
.Setup(manager => manager.FindByEmailAsync(It.IsAny<string>()))
|
||||||
|
.ReturnsAsync((ApplicationUser?)null);
|
||||||
|
userManager
|
||||||
|
.Setup(manager => manager.CreateAsync(It.IsAny<ApplicationUser>()))
|
||||||
|
.Callback<ApplicationUser>(user =>
|
||||||
|
{
|
||||||
|
user.Id = "mid-fail-user";
|
||||||
|
created = user;
|
||||||
|
})
|
||||||
|
.ReturnsAsync(IdentityResult.Success);
|
||||||
|
userManager
|
||||||
|
.Setup(manager => manager.AddToRoleAsync(It.IsAny<ApplicationUser>(), "Dispatcher"))
|
||||||
|
.ReturnsAsync(IdentityResult.Success);
|
||||||
|
roleManager.Setup(manager => manager.RoleExistsAsync("Dispatcher")).ReturnsAsync(true);
|
||||||
|
areas
|
||||||
|
.Setup(data => data.ReplaceAsync(
|
||||||
|
"mid-fail-user",
|
||||||
|
It.Is<IReadOnlyCollection<string>>(value => value.SequenceEqual(new[] { "East", "West" })),
|
||||||
|
It.IsAny<CancellationToken>()))
|
||||||
|
.Returns(Task.CompletedTask);
|
||||||
|
overrides
|
||||||
|
.Setup(data => data.SetOverridesAsync(
|
||||||
|
"mid-fail-user",
|
||||||
|
It.IsAny<IReadOnlyDictionary<string, UserPermissionState>>(),
|
||||||
|
It.IsAny<CancellationToken>()))
|
||||||
|
.ThrowsAsync(new InvalidOperationException("injected mid-sequence failure"));
|
||||||
|
|
||||||
|
var failure = await Record.ExceptionAsync(() => service.CreateAsync(ValidRequest(), Admin(), CancellationToken.None));
|
||||||
|
|
||||||
|
failure.Should().BeOfType<InvalidOperationException>()
|
||||||
|
.Which.Message.Should().Be("injected mid-sequence failure");
|
||||||
|
created.Should().NotBeNull();
|
||||||
|
areas.Verify(
|
||||||
|
data => data.ReplaceAsync("mid-fail-user", It.IsAny<IReadOnlyCollection<string>>(), It.IsAny<CancellationToken>()),
|
||||||
|
Times.Once);
|
||||||
|
userManager.Verify(manager => manager.DeleteAsync(It.IsAny<ApplicationUser>()), Times.Never);
|
||||||
|
}
|
||||||
|
|
||||||
private static TeamMemberService NewService(
|
private static TeamMemberService NewService(
|
||||||
out Mock<UserManager<ApplicationUser>> userManager,
|
out Mock<UserManager<ApplicationUser>> userManager,
|
||||||
out Mock<RoleManager<IdentityRole>> roleManager,
|
out Mock<RoleManager<IdentityRole>> roleManager,
|
||||||
|
|
@ -350,6 +397,11 @@ public sealed class TeamMemberServiceTests
|
||||||
overrides = new Mock<ITeamPermissionOverrideDataService>();
|
overrides = new Mock<ITeamPermissionOverrideDataService>();
|
||||||
userData = new Mock<IUserDataService>();
|
userData = new Mock<IUserDataService>();
|
||||||
permissions = new Mock<ITeamPermissionService>();
|
permissions = new Mock<ITeamPermissionService>();
|
||||||
|
userData
|
||||||
|
.Setup(data => data.ExecuteTransactionalAsync(
|
||||||
|
It.IsAny<Func<CancellationToken, Task>>(),
|
||||||
|
It.IsAny<CancellationToken>()))
|
||||||
|
.Returns<Func<CancellationToken, Task>, CancellationToken>((callback, token) => callback(token));
|
||||||
return new TeamMemberService(
|
return new TeamMemberService(
|
||||||
userManager.Object,
|
userManager.Object,
|
||||||
roleManager.Object,
|
roleManager.Object,
|
||||||
|
|
|
||||||
|
|
@ -166,6 +166,25 @@ namespace SeaHaven.DataServices.Implementation
|
||||||
await transaction.CommitAsync(cancellationToken);
|
await transaction.CommitAsync(cancellationToken);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
public async Task ExecuteTransactionalAsync(
|
||||||
|
Func<CancellationToken, Task> callback,
|
||||||
|
CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
ArgumentNullException.ThrowIfNull(callback);
|
||||||
|
|
||||||
|
await using var transaction = await _context.Database.BeginTransactionAsync(cancellationToken);
|
||||||
|
try
|
||||||
|
{
|
||||||
|
await callback(cancellationToken);
|
||||||
|
await transaction.CommitAsync(cancellationToken);
|
||||||
|
}
|
||||||
|
catch
|
||||||
|
{
|
||||||
|
await transaction.RollbackAsync(CancellationToken.None);
|
||||||
|
throw;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
public async Task<string?> GetEmailByIdAsync(
|
public async Task<string?> GetEmailByIdAsync(
|
||||||
string userId, CancellationToken cancellationToken)
|
string userId, CancellationToken cancellationToken)
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -18,6 +18,7 @@ namespace SeaHaven.DataServices.Interfaces
|
||||||
Task<ApplicationUser?> GetByEmailNormalizedAsync(string email, CancellationToken cancellationToken);
|
Task<ApplicationUser?> GetByEmailNormalizedAsync(string email, CancellationToken cancellationToken);
|
||||||
Task UpdateUserAsync(ApplicationUser user, CancellationToken cancellationToken);
|
Task UpdateUserAsync(ApplicationUser user, CancellationToken cancellationToken);
|
||||||
Task DeleteUserWithCascadeAsync(ApplicationUser user, CancellationToken cancellationToken);
|
Task DeleteUserWithCascadeAsync(ApplicationUser user, CancellationToken cancellationToken);
|
||||||
|
Task ExecuteTransactionalAsync(Func<CancellationToken, Task> callback, CancellationToken cancellationToken);
|
||||||
Task<string?> GetEmailByIdAsync(string userId, CancellationToken cancellationToken);
|
Task<string?> GetEmailByIdAsync(string userId, CancellationToken cancellationToken);
|
||||||
Task<IReadOnlyDictionary<string, string>> GetDisplayNamesByIdsAsync(IEnumerable<string> ids);
|
Task<IReadOnlyDictionary<string, string>> GetDisplayNamesByIdsAsync(IEnumerable<string> ids);
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -52,10 +52,6 @@ public sealed class TeamMemberService : ITeamMemberService
|
||||||
if (await _userManager.FindByEmailAsync(email!) is not null)
|
if (await _userManager.FindByEmailAsync(email!) is not null)
|
||||||
return Failure("Email is already in use.");
|
return Failure("Email is already in use.");
|
||||||
|
|
||||||
var roleError = await EnsureRoleAsync(role!);
|
|
||||||
if (roleError is not null)
|
|
||||||
return Failure(roleError);
|
|
||||||
|
|
||||||
var now = DateTime.UtcNow;
|
var now = DateTime.UtcNow;
|
||||||
var user = new ApplicationUser
|
var user = new ApplicationUser
|
||||||
{
|
{
|
||||||
|
|
@ -72,39 +68,44 @@ public sealed class TeamMemberService : ITeamMemberService
|
||||||
PendingRegistrationCreatedDate = now
|
PendingRegistrationCreatedDate = now
|
||||||
};
|
};
|
||||||
|
|
||||||
IdentityResult createResult;
|
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
createResult = await _userManager.CreateAsync(user);
|
await _userDataService.ExecuteTransactionalAsync(
|
||||||
}
|
async transactionCancellationToken =>
|
||||||
catch (DbUpdateException)
|
{
|
||||||
{
|
var roleError = await EnsureRoleAsync(role!);
|
||||||
// A concurrent create won the race on the unique user-name index
|
if (roleError is not null)
|
||||||
// between the FindByEmailAsync check above and this insert. Surface
|
throw new TeamMemberCreateException(roleError);
|
||||||
// the same conflict message instead of letting the database
|
|
||||||
// exception bubble up as a 500.
|
|
||||||
return Failure("Email is already in use.");
|
|
||||||
}
|
|
||||||
|
|
||||||
if (!createResult.Succeeded)
|
IdentityResult createResult;
|
||||||
return Failure(createResult.Errors.FirstOrDefault()?.Description ?? "Unable to create team member.");
|
try
|
||||||
|
{
|
||||||
|
createResult = await _userManager.CreateAsync(user);
|
||||||
|
}
|
||||||
|
catch (DbUpdateException)
|
||||||
|
{
|
||||||
|
// A concurrent create won the race on the unique user-name index
|
||||||
|
// between the FindByEmailAsync check above and this insert. Surface
|
||||||
|
// the same conflict message instead of letting the database
|
||||||
|
// exception bubble up as a 500.
|
||||||
|
throw new TeamMemberCreateException("Email is already in use.");
|
||||||
|
}
|
||||||
|
|
||||||
var addRoleResult = await _userManager.AddToRoleAsync(user, role!);
|
if (!createResult.Succeeded)
|
||||||
if (!addRoleResult.Succeeded)
|
throw new TeamMemberCreateException(createResult.Errors.FirstOrDefault()?.Description ?? "Unable to create team member.");
|
||||||
{
|
|
||||||
await _userManager.DeleteAsync(user);
|
|
||||||
return Failure(addRoleResult.Errors.FirstOrDefault()?.Description ?? "Unable to assign role.");
|
|
||||||
}
|
|
||||||
|
|
||||||
try
|
var addRoleResult = await _userManager.AddToRoleAsync(user, role!);
|
||||||
{
|
if (!addRoleResult.Succeeded)
|
||||||
await _areaDataService.ReplaceAsync(user.Id, areas!, cancellationToken);
|
throw new TeamMemberCreateException(addRoleResult.Errors.FirstOrDefault()?.Description ?? "Unable to assign role.");
|
||||||
await _permissionDataService.SetOverridesAsync(user.Id, overrides!, cancellationToken);
|
|
||||||
|
await _areaDataService.ReplaceAsync(user.Id, areas!, transactionCancellationToken);
|
||||||
|
await _permissionDataService.SetOverridesAsync(user.Id, overrides!, transactionCancellationToken);
|
||||||
|
},
|
||||||
|
cancellationToken);
|
||||||
}
|
}
|
||||||
catch
|
catch (TeamMemberCreateException exception)
|
||||||
{
|
{
|
||||||
await _userManager.DeleteAsync(user);
|
return Failure(exception.Message);
|
||||||
throw;
|
|
||||||
}
|
}
|
||||||
|
|
||||||
return new CreateTeamMemberOutcomeDTO
|
return new CreateTeamMemberOutcomeDTO
|
||||||
|
|
@ -391,6 +392,13 @@ public sealed class TeamMemberService : ITeamMemberService
|
||||||
private static CreateTeamMemberOutcomeDTO Failure(string error) =>
|
private static CreateTeamMemberOutcomeDTO Failure(string error) =>
|
||||||
new() { Success = false, Error = error };
|
new() { Success = false, Error = error };
|
||||||
|
|
||||||
|
private sealed class TeamMemberCreateException : Exception
|
||||||
|
{
|
||||||
|
public TeamMemberCreateException(string error) : base(error)
|
||||||
|
{
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
private static TeamMemberOperationOutcomeDTO OperationFailure(string error) =>
|
private static TeamMemberOperationOutcomeDTO OperationFailure(string error) =>
|
||||||
new() { Success = false, Error = error };
|
new() { Success = false, Error = error };
|
||||||
}
|
}
|
||||||
|
|
|
||||||
162
SeaHavenIndustries.Tests/TeamMemberCreateTransactionTests.cs
Normal file
162
SeaHavenIndustries.Tests/TeamMemberCreateTransactionTests.cs
Normal file
|
|
@ -0,0 +1,162 @@
|
||||||
|
using System.Security.Claims;
|
||||||
|
using Data.SeaHavenIndustries;
|
||||||
|
using Data.SeaHavenIndustries.Enums;
|
||||||
|
using Microsoft.AspNetCore.Identity;
|
||||||
|
using Microsoft.AspNetCore.Identity.EntityFrameworkCore;
|
||||||
|
using Microsoft.Data.Sqlite;
|
||||||
|
using Microsoft.EntityFrameworkCore;
|
||||||
|
using Microsoft.EntityFrameworkCore.Metadata;
|
||||||
|
using Microsoft.Extensions.Logging.Abstractions;
|
||||||
|
using SeaHaven.DataServices.Dto;
|
||||||
|
using SeaHaven.DataServices.Implementation;
|
||||||
|
using SeaHaven.DataServices.Interfaces;
|
||||||
|
using SeaHaven.Services.DTOs;
|
||||||
|
using SeaHaven.Services.Implementation;
|
||||||
|
|
||||||
|
namespace SeaHavenIndustries.Tests;
|
||||||
|
|
||||||
|
public sealed class TeamMemberCreateTransactionTests
|
||||||
|
{
|
||||||
|
[Fact]
|
||||||
|
public async Task Create_MidSequencePersistenceFailure_RollsBackAllMemberRows()
|
||||||
|
{
|
||||||
|
await using var connection = new SqliteConnection("Data Source=:memory:;Foreign Keys=True");
|
||||||
|
await connection.OpenAsync();
|
||||||
|
|
||||||
|
var options = new DbContextOptionsBuilder<ApplicationDbContext>()
|
||||||
|
.UseSqlite(connection)
|
||||||
|
.Options;
|
||||||
|
|
||||||
|
await using (var setup = new SqliteTeamMemberTestDbContext(options))
|
||||||
|
await setup.Database.EnsureCreatedAsync();
|
||||||
|
|
||||||
|
await using (var createScope = new SqliteTeamMemberTestDbContext(options))
|
||||||
|
{
|
||||||
|
var permissionData = new ThrowingAfterPersistPermissionDataService(
|
||||||
|
new TeamPermissionOverrideDataService(createScope));
|
||||||
|
var service = new TeamMemberService(
|
||||||
|
CreateUserManager(createScope),
|
||||||
|
CreateRoleManager(createScope),
|
||||||
|
new UserServiceAreaDataService(createScope),
|
||||||
|
permissionData,
|
||||||
|
new UserDataService(createScope),
|
||||||
|
permissionService: null!);
|
||||||
|
|
||||||
|
var failure = await Record.ExceptionAsync(() => service.CreateAsync(
|
||||||
|
ValidRequest(),
|
||||||
|
Admin(),
|
||||||
|
CancellationToken.None));
|
||||||
|
|
||||||
|
Assert.IsType<InvalidOperationException>(failure);
|
||||||
|
Assert.Equal("injected mid-sequence failure", failure!.Message);
|
||||||
|
}
|
||||||
|
|
||||||
|
await using var verify = new SqliteTeamMemberTestDbContext(options);
|
||||||
|
Assert.Empty(await verify.Users.AsNoTracking().ToListAsync());
|
||||||
|
Assert.Empty(await verify.Roles.AsNoTracking().ToListAsync());
|
||||||
|
Assert.Empty(await verify.UserRoles.AsNoTracking().ToListAsync());
|
||||||
|
Assert.Empty(await verify.UserServiceAreas.AsNoTracking().ToListAsync());
|
||||||
|
Assert.Empty(await verify.UserPermissionOverrides.AsNoTracking().ToListAsync());
|
||||||
|
}
|
||||||
|
|
||||||
|
private static UserManager<ApplicationUser> CreateUserManager(ApplicationDbContext context) =>
|
||||||
|
new(
|
||||||
|
new UserStore<ApplicationUser>(context),
|
||||||
|
Microsoft.Extensions.Options.Options.Create(new IdentityOptions()),
|
||||||
|
new PasswordHasher<ApplicationUser>(),
|
||||||
|
Array.Empty<IUserValidator<ApplicationUser>>(),
|
||||||
|
Array.Empty<IPasswordValidator<ApplicationUser>>(),
|
||||||
|
new UpperInvariantLookupNormalizer(),
|
||||||
|
new IdentityErrorDescriber(),
|
||||||
|
new EmptyServiceProvider(),
|
||||||
|
NullLogger<UserManager<ApplicationUser>>.Instance);
|
||||||
|
|
||||||
|
private static RoleManager<IdentityRole> CreateRoleManager(ApplicationDbContext context) =>
|
||||||
|
new(
|
||||||
|
new RoleStore<IdentityRole>(context),
|
||||||
|
Array.Empty<IRoleValidator<IdentityRole>>(),
|
||||||
|
new UpperInvariantLookupNormalizer(),
|
||||||
|
new IdentityErrorDescriber(),
|
||||||
|
NullLogger<RoleManager<IdentityRole>>.Instance);
|
||||||
|
|
||||||
|
private static CreateTeamMemberRequestDTO ValidRequest() => new()
|
||||||
|
{
|
||||||
|
Name = "Taylor Dispatcher",
|
||||||
|
Role = "dispatcher",
|
||||||
|
Color = "#F59E0B",
|
||||||
|
Email = "taylor@example.com",
|
||||||
|
Phone = "555-0100",
|
||||||
|
ServiceAreas = new[] { "east", "West" },
|
||||||
|
PermissionOverrides = new Dictionary<string, UserPermissionState>
|
||||||
|
{
|
||||||
|
["deleteSites"] = UserPermissionState.Allow
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
|
private static ClaimsPrincipal Admin() =>
|
||||||
|
new(new ClaimsIdentity(new[] { new Claim(ClaimTypes.Role, "Admin") }, "test"));
|
||||||
|
|
||||||
|
private sealed class SqliteTeamMemberTestDbContext : ApplicationDbContext
|
||||||
|
{
|
||||||
|
public SqliteTeamMemberTestDbContext(DbContextOptions<ApplicationDbContext> options)
|
||||||
|
: base(options)
|
||||||
|
{
|
||||||
|
}
|
||||||
|
|
||||||
|
protected override void OnModelCreating(ModelBuilder builder)
|
||||||
|
{
|
||||||
|
base.OnModelCreating(builder);
|
||||||
|
|
||||||
|
foreach (var index in builder.Model.GetEntityTypes().SelectMany(entity => entity.GetIndexes()))
|
||||||
|
{
|
||||||
|
if (index.GetFilter() is not null)
|
||||||
|
index.SetFilter(null);
|
||||||
|
}
|
||||||
|
|
||||||
|
foreach (var property in builder.Model.GetEntityTypes()
|
||||||
|
.SelectMany(entity => entity.GetProperties())
|
||||||
|
.Where(property => property.Name == "RowVersion" && property.ClrType == typeof(byte[])))
|
||||||
|
{
|
||||||
|
property.ValueGenerated = ValueGenerated.Never;
|
||||||
|
property.IsConcurrencyToken = false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private sealed class ThrowingAfterPersistPermissionDataService : ITeamPermissionOverrideDataService
|
||||||
|
{
|
||||||
|
private readonly ITeamPermissionOverrideDataService _inner;
|
||||||
|
|
||||||
|
public ThrowingAfterPersistPermissionDataService(ITeamPermissionOverrideDataService inner)
|
||||||
|
{
|
||||||
|
_inner = inner;
|
||||||
|
}
|
||||||
|
|
||||||
|
public Task<TeamPermissionUserData?> GetUserAsync(string userId, CancellationToken cancellationToken) =>
|
||||||
|
_inner.GetUserAsync(userId, cancellationToken);
|
||||||
|
|
||||||
|
public Task SetOverrideAsync(
|
||||||
|
string userId,
|
||||||
|
string permissionKey,
|
||||||
|
UserPermissionState state,
|
||||||
|
CancellationToken cancellationToken) =>
|
||||||
|
_inner.SetOverrideAsync(userId, permissionKey, state, cancellationToken);
|
||||||
|
|
||||||
|
public async Task SetOverridesAsync(
|
||||||
|
string userId,
|
||||||
|
IReadOnlyDictionary<string, UserPermissionState> overrides,
|
||||||
|
CancellationToken cancellationToken)
|
||||||
|
{
|
||||||
|
await _inner.SetOverridesAsync(userId, overrides, cancellationToken);
|
||||||
|
throw new InvalidOperationException("injected mid-sequence failure");
|
||||||
|
}
|
||||||
|
|
||||||
|
public Task ClearOverridesAsync(string userId, CancellationToken cancellationToken) =>
|
||||||
|
_inner.ClearOverridesAsync(userId, cancellationToken);
|
||||||
|
}
|
||||||
|
|
||||||
|
private sealed class EmptyServiceProvider : IServiceProvider
|
||||||
|
{
|
||||||
|
public object? GetService(Type serviceType) => null;
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
Add table
Reference in a new issue