From 15530a5ae95fd2c7dc7afacc18afa8dc58301a0e Mon Sep 17 00:00:00 2001 From: SpicyCoder Date: Sat, 20 Jun 2026 12:16:37 +0530 Subject: [PATCH 1/4] ci: add CodeQL security scanning --- .github/workflows/codeql.yml | 58 ++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) create mode 100644 .github/workflows/codeql.yml diff --git a/.github/workflows/codeql.yml b/.github/workflows/codeql.yml new file mode 100644 index 0000000..8fdca2d --- /dev/null +++ b/.github/workflows/codeql.yml @@ -0,0 +1,58 @@ +name: CodeQL + +on: + push: + branches: [main] + pull_request: + branches: [main] + schedule: + - cron: '0 6 * * 1' + +jobs: + analyze: + name: Analyze (${{ matrix.language }}) + runs-on: ubuntu-latest + permissions: + security-events: write + + strategy: + fail-fast: false + matrix: + include: + - language: csharp + build-mode: build + - language: javascript-typescript + build-mode: none + + steps: + - uses: actions/checkout@v4 + + - name: Initialize CodeQL + uses: github/codeql-action/init@v3 + with: + languages: ${{ matrix.language }} + build-mode: ${{ matrix.build-mode }} + + - if: matrix.build-mode == 'build' + name: Setup .NET + uses: actions/setup-dotnet@v4 + with: + dotnet-version: 10.0.x + + - if: matrix.build-mode == 'build' + run: dotnet workload install aspire + + - if: matrix.build-mode == 'build' + run: dotnet tool restore + + - if: matrix.build-mode == 'build' + run: dotnet restore ./ScrumPoker.slnx + + - if: matrix.build-mode == 'build' + name: Build .NET + run: dotnet build --no-restore -c Release ./ScrumPoker.slnx + + - name: Perform CodeQL Analysis + uses: github/codeql-action/analyze@v3 + with: + category: "/language:${{ matrix.language }}" From 97dcc9873059be369ae75fca73a7a6f6895fc491 Mon Sep 17 00:00:00 2001 From: SpicyCoder Date: Sat, 20 Jun 2026 12:41:46 +0530 Subject: [PATCH 2/4] chore: upgrade Swashbuckle to 10.2.2 and Aspire.Hosting.Testing to 13.4.6 --- Directory.Packages.props | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Directory.Packages.props b/Directory.Packages.props index 86d58d2..cbf1554 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -18,8 +18,8 @@ - - + + @@ -28,7 +28,7 @@ - + From 12769aecf6aad31ff4e7a9895ca6d147820e3137 Mon Sep 17 00:00:00 2001 From: SpicyCoder Date: Sat, 20 Jun 2026 12:52:20 +0530 Subject: [PATCH 3/4] ci: skip badge update on non-main branches --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5a5e744..6ec589e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -60,7 +60,7 @@ jobs: "dotnet test --no-build -c Release ./ScrumPoker.slnx" - name: Update test badges - if: always() + if: always() && github.ref == 'refs/heads/main' env: GH_TOKEN: ${{ secrets.GIST_TOKEN }} run: | From 32af7da25ffb2b5e0c9a600b4a584cb243194c7b Mon Sep 17 00:00:00 2001 From: SpicyCoder Date: Sat, 20 Jun 2026 23:52:14 +0530 Subject: [PATCH 4/4] Simplify backend: remove Result types, streamline controllers, fix races - Replace 6 Result discriminated unions with Room? return types - RoomController: try/catch for domain exceptions instead of switch-on-result - PlayerConnectionTracker: single-instance, lock Join/Leave, fix scope disposal - Remove PostgreSQL URI parser from Program.cs - Clean up Bootstrap.cs (params Assembly[] -> Assembly?) - Remove dead code from ServiceDefaults/Extensions - Remove _ = ct discards from RoomRepository - Fix PokerHub: group join inside validation block - Fix integration test: 400 -> 404 for id=0 - Frontend: handle 409 Conflict in JoinRoomForm - All handlers return 'saved' consistently - StatsCard bars capped at 100% to prevent overflow --- .../Controllers/RoomController.cs | 108 +++++------------- .../Controllers/WarmupController.cs | 2 +- src/ScrumPoker.API/Program.cs | 37 +++--- src/ScrumPoker.Application/Bootstrap.cs | 6 +- .../Commands/JoinRoom/JoinRoomHandler.cs | 30 ++--- .../Commands/JoinRoom/JoinRoomResult.cs | 10 -- .../Commands/LeaveRoom/LeaveRoomHandler.cs | 23 ++-- .../Commands/LeaveRoom/LeaveRoomResult.cs | 10 -- .../Commands/ResetVotes/ResetVotesHandler.cs | 12 +- .../Commands/ResetVotes/ResetVotesResult.cs | 9 -- .../RevealVotes/RevealVotesHandler.cs | 12 +- .../Commands/RevealVotes/RevealVotesResult.cs | 9 -- .../Features/Commands/Vote/VoteHandler.cs | 22 +--- .../Features/Commands/Vote/VoteResult.cs | 10 -- .../GetGameState/GetGameStateHandler.cs | 8 +- .../GetGameState/GetGameStateResult.cs | 9 -- .../Realtime/PlayerConnectionTracker.cs | 92 +++------------ .../Realtime/PokerHub.cs | 2 +- src/ScrumPoker.Persistence/RoomRepository.cs | 2 - src/ScrumPoker.ServiceDefaults/Extensions.cs | 15 --- .../Controllers/GetRoomStateEndpointTests.cs | 31 +---- .../Controllers/JoinRoomEndpointTests.cs | 38 +----- .../Controllers/LeaveRoomEndpointTests.cs | 38 +----- .../Controllers/ResetVotesEndpointTests.cs | 31 +---- .../Controllers/RevealVotesEndpointTests.cs | 31 +---- .../Controllers/VoteEndpointTests.cs | 38 +----- .../GetRoomStateTests.cs | 4 +- .../Commands/JoinRoom/JoinRoomHandlerTests.cs | 33 +++--- .../LeaveRoom/LeaveRoomHandlerTests.cs | 22 ++-- .../ResetVotes/ResetVotesHandlerTests.cs | 14 ++- .../RevealVotes/RevealVotesHandlerTests.cs | 14 ++- .../Commands/Vote/VoteHandlerTests.cs | 32 +++--- .../GetGameState/GetGameStateHandlerTests.cs | 14 +-- .../components/JoinRoomForm/JoinRoomForm.tsx | 4 +- 34 files changed, 204 insertions(+), 568 deletions(-) delete mode 100644 src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomResult.cs delete mode 100644 src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomResult.cs delete mode 100644 src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesResult.cs delete mode 100644 src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesResult.cs delete mode 100644 src/ScrumPoker.Application/Features/Commands/Vote/VoteResult.cs delete mode 100644 src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateResult.cs diff --git a/src/ScrumPoker.API/Controllers/RoomController.cs b/src/ScrumPoker.API/Controllers/RoomController.cs index a4dbbc3..9902caa 100644 --- a/src/ScrumPoker.API/Controllers/RoomController.cs +++ b/src/ScrumPoker.API/Controllers/RoomController.cs @@ -11,6 +11,7 @@ using ScrumPoker.Application.Features.Commands.ResetVotes; using ScrumPoker.Application.Features.Commands.Vote; using ScrumPoker.Application.Features.Queries.GetGameState; +using ScrumPoker.Domain.Rooms; using Wolverine; namespace ScrumPoker.API.Controllers; @@ -32,141 +33,84 @@ public RoomController(IMessageBus bus) public async Task Create([FromBody] CreateRoomRequest request, CancellationToken ct) { var command = new CreateRoomCommand(request.PlayerName, request.CardSet); - var room = await _bus.InvokeAsync(command, ct); + var room = await _bus.InvokeAsync(command, ct); Response.Headers.Location = $"/api/rooms/{room.Id}"; return StatusCode(201); } [HttpGet("{id:int}")] [ProducesResponseType(typeof(GameStateResponse), 200)] - [ProducesResponseType(400)] [ProducesResponseType(404)] public async Task Get([FromRoute] int id, CancellationToken ct) { - if (id <= 0) - { - return BadRequest(); - } - - var result = await _bus.InvokeAsync(new GetGameStateQuery(id), ct); - - return result switch - { - GetGameStateResult.Success(var room) => Ok(GameStateMapper.ToResponse(room)), - GetGameStateResult.RoomNotFound => NotFound(), - _ => throw new InvalidOperationException($"Unknown result type: {result.GetType().Name}") - }; + var room = await _bus.InvokeAsync(new GetGameStateQuery(id), ct); + return room is null ? NotFound() : Ok(GameStateMapper.ToResponse(room)); } [HttpPost("{id:int}/join")] [ProducesResponseType(201)] - [ProducesResponseType(400)] [ProducesResponseType(404)] [ProducesResponseType(409)] public async Task Join([FromRoute] int id, [FromBody] JoinRoomRequest request, CancellationToken ct) { - if (id <= 0) + try { - return BadRequest(); + var room = await _bus.InvokeAsync(new JoinRoomCommand(id, request.PlayerName), ct); + return room is null ? NotFound() : StatusCode(201); } - - var command = new JoinRoomCommand(id, request.PlayerName); - var result = await _bus.InvokeAsync(command, ct); - - return result switch + catch (InvalidOperationException ex) when (ex.Message.Contains("already in room")) { - JoinRoomResult.Success => StatusCode(201), - JoinRoomResult.RoomNotFound => NotFound(), - JoinRoomResult.PlayerAlreadyInRoom => Conflict(), - _ => throw new InvalidOperationException($"Unknown result type: {result.GetType().Name}") - }; + return Conflict(); + } } [HttpPost("{id:int}/vote")] [ProducesResponseType(201)] - [ProducesResponseType(400)] [ProducesResponseType(404)] public async Task Vote([FromRoute] int id, [FromBody] VoteRequest request, CancellationToken ct) { - if (id <= 0) + try { - return BadRequest(); + var room = await _bus.InvokeAsync(new VoteCommand(id, request.PlayerName, request.Value), ct); + return room is null ? NotFound() : StatusCode(201); } - - var command = new VoteCommand(id, request.PlayerName, request.Value); - var result = await _bus.InvokeAsync(command, ct); - - return result switch + catch (InvalidOperationException) { - VoteResult.Success => StatusCode(201), - VoteResult.RoomNotFound => NotFound(), - VoteResult.PlayerNotInRoom => NotFound(), - _ => throw new InvalidOperationException($"Unknown result type: {result.GetType().Name}") - }; + return NotFound(); + } } [HttpPost("{id:int}/reveal")] [ProducesResponseType(201)] - [ProducesResponseType(400)] [ProducesResponseType(404)] public async Task Reveal([FromRoute] int id, CancellationToken ct) { - if (id <= 0) - { - return BadRequest(); - } - - var result = await _bus.InvokeAsync(new RevealVotesCommand(id), ct); - - return result switch - { - RevealVotesResult.Success => StatusCode(201), - RevealVotesResult.RoomNotFound => NotFound(), - _ => throw new InvalidOperationException($"Unknown result type: {result.GetType().Name}") - }; + var room = await _bus.InvokeAsync(new RevealVotesCommand(id), ct); + return room is null ? NotFound() : StatusCode(201); } [HttpPost("{id:int}/reset")] [ProducesResponseType(201)] - [ProducesResponseType(400)] [ProducesResponseType(404)] public async Task Reset([FromRoute] int id, CancellationToken ct) { - if (id <= 0) - { - return BadRequest(); - } - - var result = await _bus.InvokeAsync(new ResetVotesCommand(id), ct); - - return result switch - { - ResetVotesResult.Success => StatusCode(201), - ResetVotesResult.RoomNotFound => NotFound(), - _ => throw new InvalidOperationException($"Unknown result type: {result.GetType().Name}") - }; + var room = await _bus.InvokeAsync(new ResetVotesCommand(id), ct); + return room is null ? NotFound() : StatusCode(201); } [HttpPost("{id:int}/leave")] [ProducesResponseType(204)] - [ProducesResponseType(400)] [ProducesResponseType(404)] public async Task Leave([FromRoute] int id, [FromBody] LeaveRoomRequest request, CancellationToken ct) { - if (id <= 0) + try { - return BadRequest(); + var room = await _bus.InvokeAsync(new LeaveRoomCommand(id, request.PlayerName), ct); + return room is null ? NotFound() : NoContent(); } - - var command = new LeaveRoomCommand(id, request.PlayerName); - var result = await _bus.InvokeAsync(command, ct); - - return result switch + catch (InvalidOperationException) { - LeaveRoomResult.Success => NoContent(), - LeaveRoomResult.RoomNotFound => NotFound(), - LeaveRoomResult.PlayerNotInRoom => NotFound(), - _ => throw new InvalidOperationException($"Unknown result type: {result.GetType().Name}") - }; + return NotFound(); + } } } diff --git a/src/ScrumPoker.API/Controllers/WarmupController.cs b/src/ScrumPoker.API/Controllers/WarmupController.cs index 2cd8742..85e7f8f 100644 --- a/src/ScrumPoker.API/Controllers/WarmupController.cs +++ b/src/ScrumPoker.API/Controllers/WarmupController.cs @@ -20,7 +20,7 @@ public async Task Warmup( var ttl = TimeSpan.FromSeconds(settings.Value.ExpirationSeconds); var room = await bus.InvokeAsync( new CreateRoomCommand("warmup", ["1", "2"], ttl, IsWarmup: true)); - _ = await bus.InvokeAsync(new GetGameStateQuery(room.Id)); + _ = await bus.InvokeAsync(new GetGameStateQuery(room.Id)); return Ok(room.Id); } } diff --git a/src/ScrumPoker.API/Program.cs b/src/ScrumPoker.API/Program.cs index f6a94a5..168375d 100644 --- a/src/ScrumPoker.API/Program.cs +++ b/src/ScrumPoker.API/Program.cs @@ -1,4 +1,3 @@ -using Npgsql; using AspNetCore.Swagger.Themes; using FluentValidation; @@ -13,28 +12,12 @@ using ScrumPoker.Persistence; using Wolverine; +using Microsoft.Extensions.Logging; + var builder = WebApplication.CreateBuilder(args); builder.AddServiceDefaults(); builder.AddRedisClient("redis"); -var statsConnStr = builder.Configuration.GetConnectionString("statsdb"); -if (statsConnStr is not null && - (statsConnStr.StartsWith("postgresql://", StringComparison.OrdinalIgnoreCase) || - statsConnStr.StartsWith("postgres://", StringComparison.OrdinalIgnoreCase))) -{ - var uri = new Uri(statsConnStr); - var userInfo = uri.UserInfo?.Split(':', 2); - statsConnStr = new NpgsqlConnectionStringBuilder - { - Host = uri.Host, - Port = uri.Port > 0 ? uri.Port : 5432, - Database = uri.AbsolutePath.TrimStart('/'), - Username = userInfo?[0], - Password = userInfo?.Length > 1 ? userInfo[1] : null, - SslMode = SslMode.Require, - }.ConnectionString; - builder.Configuration["ConnectionStrings:statsdb"] = statsConnStr; -} builder.AddNpgsqlDataSource("statsdb"); var allowedOrigins = builder.Configuration.GetSection("Cors:AllowedOrigins").Get(); @@ -74,11 +57,19 @@ var tracker = app.Services.GetRequiredService(); var scopeFactory = app.Services.GetRequiredService(); -tracker.PlayerRemoved = (roomId, playerName) => +var logger = app.Services.GetRequiredService>(); +tracker.PlayerRemoved = async (roomId, playerName) => { - using var scope = scopeFactory.CreateScope(); - var bus = scope.ServiceProvider.GetRequiredService(); - return bus.InvokeAsync(new LeaveRoomCommand(roomId, playerName)); + try + { + using var scope = scopeFactory.CreateScope(); + var bus = scope.ServiceProvider.GetRequiredService(); + await bus.InvokeAsync(new LeaveRoomCommand(roomId, playerName)); + } + catch (Exception ex) + { + logger.LogWarning(ex, "PlayerRemoved cleanup failed for {PlayerName} in room {RoomId}", playerName, roomId); + } }; app.MapDefaultEndpoints(); diff --git a/src/ScrumPoker.Application/Bootstrap.cs b/src/ScrumPoker.Application/Bootstrap.cs index c55e68b..3c69988 100644 --- a/src/ScrumPoker.Application/Bootstrap.cs +++ b/src/ScrumPoker.Application/Bootstrap.cs @@ -7,14 +7,14 @@ namespace ScrumPoker.Application; public static class Bootstrap { - public static IServiceCollection AddApplication(this IServiceCollection services, params Assembly[] extraHandlerAssemblies) + public static IServiceCollection AddApplication(this IServiceCollection services, Assembly? extraHandlerAssembly = null) { services.AddWolverine(opts => { opts.Discovery.IncludeAssembly(typeof(Bootstrap).Assembly); - foreach (var assembly in extraHandlerAssemblies) + if (extraHandlerAssembly is not null) { - opts.Discovery.IncludeAssembly(assembly); + opts.Discovery.IncludeAssembly(extraHandlerAssembly); } }); diff --git a/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomHandler.cs b/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomHandler.cs index ca34e28..256179a 100644 --- a/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomHandler.cs +++ b/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomHandler.cs @@ -7,31 +7,21 @@ namespace ScrumPoker.Application.Features.Commands.JoinRoom; public sealed class JoinRoomHandler(IRoomRepository repository, IMessageBus bus, IStatsRepository stats) { - public async Task Handle(JoinRoomCommand command, CancellationToken ct) + public async Task Handle(JoinRoomCommand command, CancellationToken ct) { var room = await repository.GetByIdAsync(command.RoomId, ct); - if (room is null) - { - return new JoinRoomResult.RoomNotFound(); - } - - try - { - var (updated, @event) = room.Join(command.PlayerName); - var saved = await repository.SaveAsync(updated, null, ct); - await bus.PublishAsync(@event); + if (room is null) return null; - if (!room.IsWarmup) - { - var monthKey = DateTime.UtcNow.ToString("yyyy-MM"); - await stats.RecordPlayerJoinedAsync(monthKey, ct); - } + var (updated, @event) = room.Join(command.PlayerName); + var saved = await repository.SaveAsync(updated, null, ct); + await bus.PublishAsync(@event); - return new JoinRoomResult.Success(saved); - } - catch (InvalidOperationException) + if (!room.IsWarmup) { - return new JoinRoomResult.PlayerAlreadyInRoom(); + var monthKey = DateTime.UtcNow.ToString("yyyy-MM"); + await stats.RecordPlayerJoinedAsync(monthKey, ct); } + + return saved; } } diff --git a/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomResult.cs b/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomResult.cs deleted file mode 100644 index 75f33f6..0000000 --- a/src/ScrumPoker.Application/Features/Commands/JoinRoom/JoinRoomResult.cs +++ /dev/null @@ -1,10 +0,0 @@ -using ScrumPoker.Domain.Rooms; - -namespace ScrumPoker.Application.Features.Commands.JoinRoom; - -public abstract record JoinRoomResult -{ - public sealed record Success(Room Room) : JoinRoomResult; - public sealed record RoomNotFound : JoinRoomResult; - public sealed record PlayerAlreadyInRoom : JoinRoomResult; -} diff --git a/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomHandler.cs b/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomHandler.cs index 81619b8..3f45d60 100644 --- a/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomHandler.cs +++ b/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomHandler.cs @@ -1,28 +1,19 @@ using ScrumPoker.Domain.Abstractions; +using ScrumPoker.Domain.Rooms; using Wolverine; namespace ScrumPoker.Application.Features.Commands.LeaveRoom; public sealed class LeaveRoomHandler(IRoomRepository repository, IMessageBus bus) { - public async Task Handle(LeaveRoomCommand command, CancellationToken ct) + public async Task Handle(LeaveRoomCommand command, CancellationToken ct) { var room = await repository.GetByIdAsync(command.RoomId, ct); - if (room is null) - { - return new LeaveRoomResult.RoomNotFound(); - } + if (room is null) return null; - try - { - var (updated, @event) = room.Leave(command.PlayerName); - await repository.SaveAsync(updated, null, ct); - await bus.PublishAsync(@event); - return new LeaveRoomResult.Success(updated); - } - catch (InvalidOperationException) - { - return new LeaveRoomResult.PlayerNotInRoom(); - } + var (updated, @event) = room.Leave(command.PlayerName); + var saved = await repository.SaveAsync(updated, null, ct); + await bus.PublishAsync(@event); + return saved; } } diff --git a/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomResult.cs b/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomResult.cs deleted file mode 100644 index 2a97296..0000000 --- a/src/ScrumPoker.Application/Features/Commands/LeaveRoom/LeaveRoomResult.cs +++ /dev/null @@ -1,10 +0,0 @@ -using ScrumPoker.Domain.Rooms; - -namespace ScrumPoker.Application.Features.Commands.LeaveRoom; - -public abstract record LeaveRoomResult -{ - public sealed record Success(Room Room) : LeaveRoomResult; - public sealed record RoomNotFound : LeaveRoomResult; - public sealed record PlayerNotInRoom : LeaveRoomResult; -} diff --git a/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesHandler.cs b/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesHandler.cs index 4255027..d3964eb 100644 --- a/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesHandler.cs +++ b/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesHandler.cs @@ -1,21 +1,19 @@ using ScrumPoker.Domain.Abstractions; +using ScrumPoker.Domain.Rooms; using Wolverine; namespace ScrumPoker.Application.Features.Commands.ResetVotes; public sealed class ResetVotesHandler(IRoomRepository repository, IMessageBus bus) { - public async Task Handle(ResetVotesCommand command, CancellationToken ct) + public async Task Handle(ResetVotesCommand command, CancellationToken ct) { var room = await repository.GetByIdAsync(command.RoomId, ct); - if (room is null) - { - return new ResetVotesResult.RoomNotFound(); - } + if (room is null) return null; var (updated, @event) = room.ResetVotes(); - await repository.SaveAsync(updated, null, ct); + var saved = await repository.SaveAsync(updated, null, ct); await bus.PublishAsync(@event); - return new ResetVotesResult.Success(updated); + return saved; } } diff --git a/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesResult.cs b/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesResult.cs deleted file mode 100644 index 82c5d58..0000000 --- a/src/ScrumPoker.Application/Features/Commands/ResetVotes/ResetVotesResult.cs +++ /dev/null @@ -1,9 +0,0 @@ -using ScrumPoker.Domain.Rooms; - -namespace ScrumPoker.Application.Features.Commands.ResetVotes; - -public abstract record ResetVotesResult -{ - public sealed record Success(Room Room) : ResetVotesResult; - public sealed record RoomNotFound : ResetVotesResult; -} diff --git a/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesHandler.cs b/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesHandler.cs index 694a7ab..612ba7e 100644 --- a/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesHandler.cs +++ b/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesHandler.cs @@ -1,21 +1,19 @@ using ScrumPoker.Domain.Abstractions; +using ScrumPoker.Domain.Rooms; using Wolverine; namespace ScrumPoker.Application.Features.Commands.RevealVotes; public sealed class RevealVotesHandler(IRoomRepository repository, IMessageBus bus) { - public async Task Handle(RevealVotesCommand command, CancellationToken ct) + public async Task Handle(RevealVotesCommand command, CancellationToken ct) { var room = await repository.GetByIdAsync(command.RoomId, ct); - if (room is null) - { - return new RevealVotesResult.RoomNotFound(); - } + if (room is null) return null; var (updated, @event) = room.Reveal(); - await repository.SaveAsync(updated, null, ct); + var saved = await repository.SaveAsync(updated, null, ct); await bus.PublishAsync(@event); - return new RevealVotesResult.Success(updated); + return saved; } } diff --git a/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesResult.cs b/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesResult.cs deleted file mode 100644 index b0b0555..0000000 --- a/src/ScrumPoker.Application/Features/Commands/RevealVotes/RevealVotesResult.cs +++ /dev/null @@ -1,9 +0,0 @@ -using ScrumPoker.Domain.Rooms; - -namespace ScrumPoker.Application.Features.Commands.RevealVotes; - -public abstract record RevealVotesResult -{ - public sealed record Success(Room Room) : RevealVotesResult; - public sealed record RoomNotFound : RevealVotesResult; -} diff --git a/src/ScrumPoker.Application/Features/Commands/Vote/VoteHandler.cs b/src/ScrumPoker.Application/Features/Commands/Vote/VoteHandler.cs index 9f13e21..2a0d20f 100644 --- a/src/ScrumPoker.Application/Features/Commands/Vote/VoteHandler.cs +++ b/src/ScrumPoker.Application/Features/Commands/Vote/VoteHandler.cs @@ -6,24 +6,14 @@ namespace ScrumPoker.Application.Features.Commands.Vote; public sealed class VoteHandler(IRoomRepository repository, IMessageBus bus) { - public async Task Handle(VoteCommand command, CancellationToken ct) + public async Task Handle(VoteCommand command, CancellationToken ct) { var room = await repository.GetByIdAsync(command.RoomId, ct); - if (room is null) - { - return new VoteResult.RoomNotFound(); - } + if (room is null) return null; - try - { - var (updated, @event) = room.Vote(command.PlayerName, command.Value); - var saved = await repository.SaveAsync(updated, null, ct); - await bus.PublishAsync(@event); - return new VoteResult.Success(saved); - } - catch (InvalidOperationException) - { - return new VoteResult.PlayerNotInRoom(); - } + var (updated, @event) = room.Vote(command.PlayerName, command.Value); + var saved = await repository.SaveAsync(updated, null, ct); + await bus.PublishAsync(@event); + return saved; } } diff --git a/src/ScrumPoker.Application/Features/Commands/Vote/VoteResult.cs b/src/ScrumPoker.Application/Features/Commands/Vote/VoteResult.cs deleted file mode 100644 index 2cd8c7c..0000000 --- a/src/ScrumPoker.Application/Features/Commands/Vote/VoteResult.cs +++ /dev/null @@ -1,10 +0,0 @@ -using ScrumPoker.Domain.Rooms; - -namespace ScrumPoker.Application.Features.Commands.Vote; - -public abstract record VoteResult -{ - public sealed record Success(Room Room) : VoteResult; - public sealed record RoomNotFound : VoteResult; - public sealed record PlayerNotInRoom : VoteResult; -} diff --git a/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateHandler.cs b/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateHandler.cs index b193604..69811b4 100644 --- a/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateHandler.cs +++ b/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateHandler.cs @@ -1,14 +1,12 @@ using ScrumPoker.Domain.Abstractions; +using ScrumPoker.Domain.Rooms; namespace ScrumPoker.Application.Features.Queries.GetGameState; public sealed class GetGameStateHandler(IRoomRepository repository) { - public async Task Handle(GetGameStateQuery query, CancellationToken ct) + public async Task Handle(GetGameStateQuery query, CancellationToken ct) { - var room = await repository.GetByIdAsync(query.RoomId, ct); - return room is null - ? new GetGameStateResult.RoomNotFound() - : new GetGameStateResult.Success(room); + return await repository.GetByIdAsync(query.RoomId, ct); } } diff --git a/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateResult.cs b/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateResult.cs deleted file mode 100644 index a5e7a2b..0000000 --- a/src/ScrumPoker.Application/Features/Queries/GetGameState/GetGameStateResult.cs +++ /dev/null @@ -1,9 +0,0 @@ -using ScrumPoker.Domain.Rooms; - -namespace ScrumPoker.Application.Features.Queries.GetGameState; - -public abstract record GetGameStateResult -{ - public sealed record Success(Room Room) : GetGameStateResult; - public sealed record RoomNotFound : GetGameStateResult; -} diff --git a/src/ScrumPoker.Infrastructure/Realtime/PlayerConnectionTracker.cs b/src/ScrumPoker.Infrastructure/Realtime/PlayerConnectionTracker.cs index 68c45c3..36b11ae 100644 --- a/src/ScrumPoker.Infrastructure/Realtime/PlayerConnectionTracker.cs +++ b/src/ScrumPoker.Infrastructure/Realtime/PlayerConnectionTracker.cs @@ -1,99 +1,45 @@ using System.Collections.Concurrent; -using StackExchange.Redis; namespace ScrumPoker.Infrastructure.Realtime; -public sealed class PlayerConnectionTracker(IConnectionMultiplexer redis) +// ponytail: single-instance only. Add Redis backplane coordination if multi-instance needed. +public sealed class PlayerConnectionTracker { private readonly ConcurrentDictionary<(int RoomId, string PlayerName), ConcurrentDictionary> _connections = new(); - private readonly ConcurrentDictionary<(int RoomId, string PlayerName), CancellationTokenSource> _pendingRemovals = new(); - private static readonly TimeSpan GracePeriod = TimeSpan.FromSeconds(5); - private static readonly TimeSpan PendingRemovalTtl = GracePeriod + TimeSpan.FromSeconds(10); + private readonly object _lock = new(); public Func? PlayerRemoved { get; set; } - public async Task Join(int roomId, string playerName, string connectionId) + public Task Join(int roomId, string playerName, string connectionId) { var key = (roomId, playerName); - - // Cancel local pending removal if reconnecting on same instance - if (_pendingRemovals.TryRemove(key, out var cts)) - { - cts.Cancel(); - cts.Dispose(); - } - - // Cross-instance: delete Redis key so grace timer on another instance skips removal - try + lock (_lock) { - await redis.GetDatabase().KeyDeleteAsync(PendingRemovalKey(roomId, playerName)); + var set = _connections.GetOrAdd(key, _ => new ConcurrentDictionary()); + set.TryAdd(connectionId, 0); } - catch { /* Redis unavailable — cross-instance cancel won't work, local cancel still does */ } - - var set = _connections.GetOrAdd(key, _ => new ConcurrentDictionary()); - set.TryAdd(connectionId, 0); + return Task.CompletedTask; } public void Leave(int roomId, string playerName, string connectionId) { var key = (roomId, playerName); - if (!_connections.TryGetValue(key, out var set)) - return; + bool invokeRemoved = false; + lock (_lock) + { + if (!_connections.TryGetValue(key, out var set)) + return; - set.TryRemove(connectionId, out _); + set.TryRemove(connectionId, out _); - if (set.IsEmpty) - { - var cts = new CancellationTokenSource(); - if (_pendingRemovals.TryAdd(key, cts)) - { - // Cross-instance: store pending removal in Redis with TTL matching grace period - _ = SetPendingRemovalAsync(roomId, playerName); - _ = RemoveAfterGracePeriod(key, cts.Token); - } - else + if (set.IsEmpty) { - cts.Dispose(); + _connections.TryRemove(key, out _); + invokeRemoved = true; } } - } - private async Task SetPendingRemovalAsync(int roomId, string playerName) - { - try - { - await redis.GetDatabase().StringSetAsync(PendingRemovalKey(roomId, playerName), "1", PendingRemovalTtl); - } - catch { /* Redis unavailable — cross-instance cancel won't work, local cancel still does */ } + if (invokeRemoved) + _ = PlayerRemoved?.Invoke(roomId, playerName); } - - private async Task RemoveAfterGracePeriod((int RoomId, string PlayerName) key, CancellationToken ct) - { - try - { - await Task.Delay(GracePeriod, ct); - } - catch (OperationCanceledException) - { - return; - } - - // Cross-instance: atomically delete the key. - // Returns true → key existed, we own the removal, proceed. - // Returns false → another instance's JoinRoom already deleted it, skip removal. - try - { - var deleted = await redis.GetDatabase().KeyDeleteAsync(PendingRemovalKey(key.RoomId, key.PlayerName)); - if (!deleted) - return; - } - catch { /* Redis unavailable — proceed with removal */ } - - _pendingRemovals.TryRemove(key, out _); - _connections.TryRemove(key, out _); - _ = PlayerRemoved?.Invoke(key.RoomId, key.PlayerName); - } - - private static string PendingRemovalKey(int roomId, string playerName) => - $"pending-removal:{roomId}:{playerName}"; } diff --git a/src/ScrumPoker.Infrastructure/Realtime/PokerHub.cs b/src/ScrumPoker.Infrastructure/Realtime/PokerHub.cs index 8389372..59c6c8a 100644 --- a/src/ScrumPoker.Infrastructure/Realtime/PokerHub.cs +++ b/src/ScrumPoker.Infrastructure/Realtime/PokerHub.cs @@ -22,8 +22,8 @@ public async Task JoinRoom(string roomId, string playerName) Context.Items[RoomIdKey] = id; Context.Items[PlayerNameKey] = playerName; await tracker.Join(id, playerName, Context.ConnectionId); + await Groups.AddToGroupAsync(Context.ConnectionId, roomId); } - await Groups.AddToGroupAsync(Context.ConnectionId, roomId); } public Task LeaveRoom(string roomId) diff --git a/src/ScrumPoker.Persistence/RoomRepository.cs b/src/ScrumPoker.Persistence/RoomRepository.cs index 96573b5..56bb529 100644 --- a/src/ScrumPoker.Persistence/RoomRepository.cs +++ b/src/ScrumPoker.Persistence/RoomRepository.cs @@ -14,14 +14,12 @@ public sealed class RoomRepository(IConnectionMultiplexer redis, IOptions GetByIdAsync(int id, CancellationToken ct = default) { - _ = ct; var value = await _db.StringGetAsync(id.ToString()); return value.IsNullOrEmpty ? null : JsonSerializer.Deserialize(value.ToString()); } public async Task SaveAsync(Room room, TimeSpan? expiry = null, CancellationToken ct = default) { - _ = ct; var id = room.Id == 0 ? await GenerateIdAsync() : room.Id; var saved = room with { Id = id }; diff --git a/src/ScrumPoker.ServiceDefaults/Extensions.cs b/src/ScrumPoker.ServiceDefaults/Extensions.cs index 67c8d1d..424f592 100644 --- a/src/ScrumPoker.ServiceDefaults/Extensions.cs +++ b/src/ScrumPoker.ServiceDefaults/Extensions.cs @@ -35,12 +35,6 @@ public static TBuilder AddServiceDefaults(this TBuilder builder) where http.AddServiceDiscovery(); }); - // Uncomment the following to restrict the allowed schemes for service discovery. - // builder.Services.Configure(options => - // { - // options.AllowedSchemes = ["https"]; - // }); - return builder; } @@ -68,8 +62,6 @@ public static TBuilder ConfigureOpenTelemetry(this TBuilder builder) w !context.Request.Path.StartsWithSegments(HealthEndpointPath) && !context.Request.Path.StartsWithSegments(AlivenessEndpointPath) ) - // Uncomment the following line to enable gRPC instrumentation (requires the OpenTelemetry.Instrumentation.GrpcNetClient package) - //.AddGrpcClientInstrumentation() .AddHttpClientInstrumentation(); }); @@ -87,13 +79,6 @@ private static TBuilder AddOpenTelemetryExporters(this TBuilder builde builder.Services.AddOpenTelemetry().UseOtlpExporter(); } - // Uncomment the following lines to enable the Azure Monitor exporter (requires the Azure.Monitor.OpenTelemetry.AspNetCore package) - //if (!string.IsNullOrEmpty(builder.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"])) - //{ - // builder.Services.AddOpenTelemetry() - // .UseAzureMonitor(); - //} - return builder; } diff --git a/tests/ScrumPoker.API.UnitTests/Controllers/GetRoomStateEndpointTests.cs b/tests/ScrumPoker.API.UnitTests/Controllers/GetRoomStateEndpointTests.cs index 67bd49d..b4ea730 100644 --- a/tests/ScrumPoker.API.UnitTests/Controllers/GetRoomStateEndpointTests.cs +++ b/tests/ScrumPoker.API.UnitTests/Controllers/GetRoomStateEndpointTests.cs @@ -26,8 +26,8 @@ public GetRoomStateEndpointTests() public async Task Should_Return_200Ok_When_RoomExists() { var room = new Room { Id = 42, Players = [new Player("Alice", null)] }; - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new GetGameStateResult.Success(room)); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(room); var result = await _sut.Get(42, TestContext.Current.CancellationToken); @@ -43,35 +43,12 @@ public async Task Should_Return_200Ok_When_RoomExists() [Fact] public async Task Should_Return_404NotFound_When_RoomNotFound() { - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new GetGameStateResult.RoomNotFound()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns((Room?)null); var result = await _sut.Get(99, TestContext.Current.CancellationToken); result.ShouldBeOfType(); } - [Theory] - [InlineData(0)] - [InlineData(-5)] - public async Task Should_Return_400BadRequest_When_IdIsInvalid(int id) - { - var result = await _sut.Get(id, TestContext.Current.CancellationToken); - - result.ShouldBeOfType(); - await _bus.DidNotReceiveWithAnyArgs().InvokeAsync(default!, default(CancellationToken)); - } - - [Fact] - public async Task Should_Throw_InvalidOperationException_For_UnknownResultType() - { - var unknownResult = Substitute.For(); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(unknownResult); - - var exception = await Should.ThrowAsync(() => - _sut.Get(1, TestContext.Current.CancellationToken)); - - exception.Message.ShouldContain("Unknown result type"); - } } diff --git a/tests/ScrumPoker.API.UnitTests/Controllers/JoinRoomEndpointTests.cs b/tests/ScrumPoker.API.UnitTests/Controllers/JoinRoomEndpointTests.cs index 0f06ce9..31b0dc7 100644 --- a/tests/ScrumPoker.API.UnitTests/Controllers/JoinRoomEndpointTests.cs +++ b/tests/ScrumPoker.API.UnitTests/Controllers/JoinRoomEndpointTests.cs @@ -27,8 +27,8 @@ public async Task Should_Return_201Created_When_Success() { var request = new JoinRoomRequest("Bob"); var room = new Room { Id = 1, Players = [new Player("Alice", null), new Player("Bob", null)] }; - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new JoinRoomResult.Success(room)); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(room); var result = await _sut.Join(1, request, TestContext.Current.CancellationToken); @@ -40,8 +40,8 @@ public async Task Should_Return_201Created_When_Success() public async Task Should_Return_404NotFound_When_RoomNotFound() { var request = new JoinRoomRequest("Bob"); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new JoinRoomResult.RoomNotFound()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns((Room?)null); var result = await _sut.Join(1, request, TestContext.Current.CancellationToken); @@ -52,38 +52,12 @@ public async Task Should_Return_404NotFound_When_RoomNotFound() public async Task Should_Return_409Conflict_When_PlayerAlreadyInRoom() { var request = new JoinRoomRequest("Alice"); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new JoinRoomResult.PlayerAlreadyInRoom()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromException(new InvalidOperationException("already in room"))); var result = await _sut.Join(1, request, TestContext.Current.CancellationToken); result.ShouldBeOfType(); } - [Theory] - [InlineData(0)] - [InlineData(-5)] - public async Task Should_Return_400BadRequest_When_IdIsInvalid(int id) - { - var request = new JoinRoomRequest("Bob"); - - var result = await _sut.Join(id, request, TestContext.Current.CancellationToken); - - result.ShouldBeOfType(); - await _bus.DidNotReceiveWithAnyArgs().InvokeAsync(default!, default(CancellationToken)); - } - - [Fact] - public async Task Should_Throw_InvalidOperationException_For_UnknownResultType() - { - var request = new JoinRoomRequest("Bob"); - var unknownResult = Substitute.For(); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(unknownResult); - - var exception = await Should.ThrowAsync(() => - _sut.Join(1, request, TestContext.Current.CancellationToken)); - - exception.Message.ShouldContain("Unknown result type"); - } } diff --git a/tests/ScrumPoker.API.UnitTests/Controllers/LeaveRoomEndpointTests.cs b/tests/ScrumPoker.API.UnitTests/Controllers/LeaveRoomEndpointTests.cs index af10783..b49cf38 100644 --- a/tests/ScrumPoker.API.UnitTests/Controllers/LeaveRoomEndpointTests.cs +++ b/tests/ScrumPoker.API.UnitTests/Controllers/LeaveRoomEndpointTests.cs @@ -27,8 +27,8 @@ public async Task Should_Return_204NoContent_When_Success() { var request = new LeaveRoomRequest("Bob"); var room = new Room { Id = 1, Players = [new Player("Alice", null)] }; - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new LeaveRoomResult.Success(room)); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(room); var result = await _sut.Leave(1, request, TestContext.Current.CancellationToken); @@ -39,8 +39,8 @@ public async Task Should_Return_204NoContent_When_Success() public async Task Should_Return_404NotFound_When_RoomNotFound() { var request = new LeaveRoomRequest("Bob"); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new LeaveRoomResult.RoomNotFound()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns((Room?)null); var result = await _sut.Leave(1, request, TestContext.Current.CancellationToken); @@ -51,38 +51,12 @@ public async Task Should_Return_404NotFound_When_RoomNotFound() public async Task Should_Return_404NotFound_When_PlayerNotInRoom() { var request = new LeaveRoomRequest("Bob"); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new LeaveRoomResult.PlayerNotInRoom()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromException(new InvalidOperationException("not in room"))); var result = await _sut.Leave(1, request, TestContext.Current.CancellationToken); result.ShouldBeOfType(); } - [Theory] - [InlineData(0)] - [InlineData(-5)] - public async Task Should_Return_400BadRequest_When_IdIsInvalid(int id) - { - var request = new LeaveRoomRequest("Bob"); - - var result = await _sut.Leave(id, request, TestContext.Current.CancellationToken); - - result.ShouldBeOfType(); - await _bus.DidNotReceiveWithAnyArgs().InvokeAsync(default!, default(CancellationToken)); - } - - [Fact] - public async Task Should_Throw_InvalidOperationException_For_UnknownResultType() - { - var request = new LeaveRoomRequest("Bob"); - var unknownResult = Substitute.For(); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(unknownResult); - - var exception = await Should.ThrowAsync(() => - _sut.Leave(1, request, TestContext.Current.CancellationToken)); - - exception.Message.ShouldContain("Unknown result type"); - } } diff --git a/tests/ScrumPoker.API.UnitTests/Controllers/ResetVotesEndpointTests.cs b/tests/ScrumPoker.API.UnitTests/Controllers/ResetVotesEndpointTests.cs index be31ff1..c0b888c 100644 --- a/tests/ScrumPoker.API.UnitTests/Controllers/ResetVotesEndpointTests.cs +++ b/tests/ScrumPoker.API.UnitTests/Controllers/ResetVotesEndpointTests.cs @@ -24,8 +24,8 @@ public ResetVotesEndpointTests() [Fact] public async Task Should_Return_201Created_When_Success() { - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new ResetVotesResult.Success(new Room())); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(new Room()); var result = await _sut.Reset(1, TestContext.Current.CancellationToken); @@ -36,35 +36,12 @@ public async Task Should_Return_201Created_When_Success() [Fact] public async Task Should_Return_404NotFound_When_RoomNotFound() { - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new ResetVotesResult.RoomNotFound()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns((Room?)null); var result = await _sut.Reset(1, TestContext.Current.CancellationToken); result.ShouldBeOfType(); } - [Theory] - [InlineData(0)] - [InlineData(-5)] - public async Task Should_Return_400BadRequest_When_IdIsInvalid(int id) - { - var result = await _sut.Reset(id, TestContext.Current.CancellationToken); - - result.ShouldBeOfType(); - await _bus.DidNotReceiveWithAnyArgs().InvokeAsync(default!, default(CancellationToken)); - } - - [Fact] - public async Task Should_Throw_InvalidOperationException_For_UnknownResultType() - { - var unknownResult = Substitute.For(); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(unknownResult); - - var exception = await Should.ThrowAsync(() => - _sut.Reset(1, TestContext.Current.CancellationToken)); - - exception.Message.ShouldContain("Unknown result type"); - } } diff --git a/tests/ScrumPoker.API.UnitTests/Controllers/RevealVotesEndpointTests.cs b/tests/ScrumPoker.API.UnitTests/Controllers/RevealVotesEndpointTests.cs index 7747575..258ca11 100644 --- a/tests/ScrumPoker.API.UnitTests/Controllers/RevealVotesEndpointTests.cs +++ b/tests/ScrumPoker.API.UnitTests/Controllers/RevealVotesEndpointTests.cs @@ -24,8 +24,8 @@ public RevealVotesEndpointTests() [Fact] public async Task Should_Return_201Created_When_Success() { - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new RevealVotesResult.Success(new Room())); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(new Room()); var result = await _sut.Reveal(1, TestContext.Current.CancellationToken); @@ -36,35 +36,12 @@ public async Task Should_Return_201Created_When_Success() [Fact] public async Task Should_Return_404NotFound_When_RoomNotFound() { - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new RevealVotesResult.RoomNotFound()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns((Room?)null); var result = await _sut.Reveal(1, TestContext.Current.CancellationToken); result.ShouldBeOfType(); } - [Theory] - [InlineData(0)] - [InlineData(-5)] - public async Task Should_Return_400BadRequest_When_IdIsInvalid(int id) - { - var result = await _sut.Reveal(id, TestContext.Current.CancellationToken); - - result.ShouldBeOfType(); - await _bus.DidNotReceiveWithAnyArgs().InvokeAsync(default!, default(CancellationToken)); - } - - [Fact] - public async Task Should_Throw_InvalidOperationException_For_UnknownResultType() - { - var unknownResult = Substitute.For(); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(unknownResult); - - var exception = await Should.ThrowAsync(() => - _sut.Reveal(1, TestContext.Current.CancellationToken)); - - exception.Message.ShouldContain("Unknown result type"); - } } diff --git a/tests/ScrumPoker.API.UnitTests/Controllers/VoteEndpointTests.cs b/tests/ScrumPoker.API.UnitTests/Controllers/VoteEndpointTests.cs index 8b2c2e7..bb2fae3 100644 --- a/tests/ScrumPoker.API.UnitTests/Controllers/VoteEndpointTests.cs +++ b/tests/ScrumPoker.API.UnitTests/Controllers/VoteEndpointTests.cs @@ -27,8 +27,8 @@ public async Task Should_Return_201Created_When_Success() { var request = new VoteRequest("Alice", "5"); var room = new Room { Id = 1, Players = [new Player("Alice", "5")] }; - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new VoteResult.Success(room)); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(room); var result = await _sut.Vote(1, request, TestContext.Current.CancellationToken); @@ -40,8 +40,8 @@ public async Task Should_Return_201Created_When_Success() public async Task Should_Return_404NotFound_When_RoomNotFound() { var request = new VoteRequest("Bob", "5"); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new VoteResult.RoomNotFound()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns((Room?)null); var result = await _sut.Vote(1, request, TestContext.Current.CancellationToken); @@ -52,38 +52,12 @@ public async Task Should_Return_404NotFound_When_RoomNotFound() public async Task Should_Return_404NotFound_When_PlayerNotInRoom() { var request = new VoteRequest("Bob", "5"); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(new VoteResult.PlayerNotInRoom()); + _bus.InvokeAsync(Arg.Any(), Arg.Any()) + .Returns(Task.FromException(new InvalidOperationException("not in room"))); var result = await _sut.Vote(1, request, TestContext.Current.CancellationToken); result.ShouldBeOfType(); } - [Theory] - [InlineData(0)] - [InlineData(-5)] - public async Task Should_Return_400BadRequest_When_IdIsInvalid(int id) - { - var request = new VoteRequest("Bob", "5"); - - var result = await _sut.Vote(id, request, TestContext.Current.CancellationToken); - - result.ShouldBeOfType(); - await _bus.DidNotReceiveWithAnyArgs().InvokeAsync(default!, default(CancellationToken)); - } - - [Fact] - public async Task Should_Throw_InvalidOperationException_For_UnknownResultType() - { - var request = new VoteRequest("Bob", "5"); - var unknownResult = Substitute.For(); - _bus.InvokeAsync(Arg.Any(), Arg.Any()) - .Returns(unknownResult); - - var exception = await Should.ThrowAsync(() => - _sut.Vote(1, request, TestContext.Current.CancellationToken)); - - exception.Message.ShouldContain("Unknown result type"); - } } diff --git a/tests/ScrumPoker.AppHost.IntegrationTests/GetRoomStateTests.cs b/tests/ScrumPoker.AppHost.IntegrationTests/GetRoomStateTests.cs index 1e28046..9f96227 100644 --- a/tests/ScrumPoker.AppHost.IntegrationTests/GetRoomStateTests.cs +++ b/tests/ScrumPoker.AppHost.IntegrationTests/GetRoomStateTests.cs @@ -44,11 +44,11 @@ public async Task With_NonExistentRoom_Should_Return_404() } [Fact] - public async Task With_InvalidId_Should_Return_400() + public async Task With_InvalidId_Should_Return_404() { var response = await _httpClient.GetAsync("/api/rooms/0", TestContext.Current.CancellationToken); - response.StatusCode.ShouldBe(HttpStatusCode.BadRequest); + response.StatusCode.ShouldBe(HttpStatusCode.NotFound); } } diff --git a/tests/ScrumPoker.Application.UnitTests/Features/Commands/JoinRoom/JoinRoomHandlerTests.cs b/tests/ScrumPoker.Application.UnitTests/Features/Commands/JoinRoom/JoinRoomHandlerTests.cs index 9901bb7..6d60200 100644 --- a/tests/ScrumPoker.Application.UnitTests/Features/Commands/JoinRoom/JoinRoomHandlerTests.cs +++ b/tests/ScrumPoker.Application.UnitTests/Features/Commands/JoinRoom/JoinRoomHandlerTests.cs @@ -20,18 +20,18 @@ public JoinRoomHandlerTests() } [Fact] - public async Task Handle_Should_Return_RoomNotFound_When_Room_DoesNotExist() + public async Task Handle_Should_Return_Null_When_Room_DoesNotExist() { var command = new JoinRoomCommand(42, "Bob"); _repository.GetByIdAsync(42, Arg.Any()).Returns((Room?)null); var result = await _sut.Handle(command, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeNull(); } [Fact] - public async Task Handle_Should_Return_PlayerAlreadyInRoom_When_Player_Exists() + public async Task Handle_Should_Throw_When_Player_AlreadyInRoom() { var room = new Room { @@ -41,13 +41,12 @@ public async Task Handle_Should_Return_PlayerAlreadyInRoom_When_Player_Exists() var command = new JoinRoomCommand(1, "Bob"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - var result = await _sut.Handle(command, CancellationToken.None); - - result.ShouldBeOfType(); + await Should.ThrowAsync(() => + _sut.Handle(command, CancellationToken.None)); } [Fact] - public async Task Handle_Should_Return_Success_With_UpdatedRoom() + public async Task Handle_Should_Return_Room_With_UpdatedPlayers() { var room = new Room { @@ -56,16 +55,17 @@ public async Task Handle_Should_Return_Success_With_UpdatedRoom() }; var command = new JoinRoomCommand(1, "Bob"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - - var updatedRoom = room with { Players = [.. room.Players, new Player("Bob", null)] }; - _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()).Returns(updatedRoom); + _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => ci.ArgAt(0)); + _stats.RecordPlayerJoinedAsync(Arg.Any(), Arg.Any()) + .Returns(Task.CompletedTask); var result = await _sut.Handle(command, CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Players.Count.ShouldBe(2); - success.Room.Players.ShouldContain(p => p.Name == "Alice"); - success.Room.Players.ShouldContain(p => p.Name == "Bob"); + result.ShouldNotBeNull(); + result.Players.Count.ShouldBe(2); + result.Players.ShouldContain(p => p.Name == "Alice"); + result.Players.ShouldContain(p => p.Name == "Bob"); await _repository.Received(1).SaveAsync( Arg.Is(r => r.Players.Count == 2), @@ -81,6 +81,8 @@ public async Task Handle_Should_PublishPlayerJoinedEvent_OnSuccess() _repository.GetByIdAsync(1, Arg.Any()).Returns(room); _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()) .Returns(ci => ci.ArgAt(0)); + _stats.RecordPlayerJoinedAsync(Arg.Any(), Arg.Any()) + .Returns(Task.CompletedTask); await _sut.Handle(command, CancellationToken.None); @@ -106,7 +108,8 @@ public async Task Handle_Should_NotPublishEvent_When_PlayerAlreadyInRoom() var command = new JoinRoomCommand(1, "Bob"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - await _sut.Handle(command, CancellationToken.None); + await Should.ThrowAsync(() => + _sut.Handle(command, CancellationToken.None)); await _bus.DidNotReceiveWithAnyArgs().PublishAsync(Arg.Any()); } diff --git a/tests/ScrumPoker.Application.UnitTests/Features/Commands/LeaveRoom/LeaveRoomHandlerTests.cs b/tests/ScrumPoker.Application.UnitTests/Features/Commands/LeaveRoom/LeaveRoomHandlerTests.cs index c39d7b4..6383037 100644 --- a/tests/ScrumPoker.Application.UnitTests/Features/Commands/LeaveRoom/LeaveRoomHandlerTests.cs +++ b/tests/ScrumPoker.Application.UnitTests/Features/Commands/LeaveRoom/LeaveRoomHandlerTests.cs @@ -18,18 +18,18 @@ public LeaveRoomHandlerTests() } [Fact] - public async Task Handle_Should_Return_RoomNotFound_When_Room_DoesNotExist() + public async Task Handle_Should_Return_Null_When_Room_DoesNotExist() { var command = new LeaveRoomCommand(42, "Bob"); _repository.GetByIdAsync(42, Arg.Any()).Returns((Room?)null); var result = await _sut.Handle(command, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeNull(); } [Fact] - public async Task Handle_Should_Return_PlayerNotInRoom_When_Player_NotFound() + public async Task Handle_Should_Throw_When_Player_NotInRoom() { var room = new Room { @@ -39,13 +39,12 @@ public async Task Handle_Should_Return_PlayerNotInRoom_When_Player_NotFound() var command = new LeaveRoomCommand(1, "Bob"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - var result = await _sut.Handle(command, CancellationToken.None); - - result.ShouldBeOfType(); + await Should.ThrowAsync(() => + _sut.Handle(command, CancellationToken.None)); } [Fact] - public async Task Handle_Should_Return_Success_With_UpdatedRoom_When_Valid() + public async Task Handle_Should_Return_Room_When_Valid() { var room = new Room { @@ -59,9 +58,9 @@ public async Task Handle_Should_Return_Success_With_UpdatedRoom_When_Valid() var result = await _sut.Handle(command, CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Players.ShouldHaveSingleItem(); - success.Room.Players[0].Name.ShouldBe("Alice"); + result.ShouldNotBeNull(); + result.Players.ShouldHaveSingleItem(); + result.Players[0].Name.ShouldBe("Alice"); await _repository.Received(1).SaveAsync( Arg.Is(r => r.Players.Count == 1 && r.Players[0].Name == "Alice"), @@ -110,7 +109,8 @@ public async Task Handle_Should_NotPublishEvent_When_PlayerNotInRoom() var command = new LeaveRoomCommand(1, "Bob"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - await _sut.Handle(command, CancellationToken.None); + await Should.ThrowAsync(() => + _sut.Handle(command, CancellationToken.None)); await _bus.DidNotReceiveWithAnyArgs().PublishAsync(Arg.Any()); } diff --git a/tests/ScrumPoker.Application.UnitTests/Features/Commands/ResetVotes/ResetVotesHandlerTests.cs b/tests/ScrumPoker.Application.UnitTests/Features/Commands/ResetVotes/ResetVotesHandlerTests.cs index 94ad340..a059a89 100644 --- a/tests/ScrumPoker.Application.UnitTests/Features/Commands/ResetVotes/ResetVotesHandlerTests.cs +++ b/tests/ScrumPoker.Application.UnitTests/Features/Commands/ResetVotes/ResetVotesHandlerTests.cs @@ -18,14 +18,14 @@ public ResetVotesHandlerTests() } [Fact] - public async Task Handle_Should_ReturnRoomNotFound_When_RoomDoesNotExist() + public async Task Handle_Should_Return_Null_When_Room_DoesNotExist() { var command = new ResetVotesCommand(42); _repository.GetByIdAsync(42, Arg.Any()).Returns((Room?)null); var result = await _sut.Handle(command, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeNull(); } [Fact] @@ -39,12 +39,14 @@ public async Task Handle_Should_ClearAllVotes_And_SetRevealedFalse() }; var command = new ResetVotesCommand(1); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); + _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => ci.ArgAt(0)); var result = await _sut.Handle(command, CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Revealed.ShouldBeFalse(); - success.Room.Players.ShouldAllBe(p => p.Value == null); + result.ShouldNotBeNull(); + result.Revealed.ShouldBeFalse(); + result.Players.ShouldAllBe(p => p.Value == null); await _repository.Received(1).SaveAsync( Arg.Is(r => !r.Revealed && r.Players.All(p => p.Value == null)), @@ -63,6 +65,8 @@ public async Task Handle_Should_PublishVotesResetEvent_OnSuccess() }; var command = new ResetVotesCommand(1); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); + _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => ci.ArgAt(0)); await _sut.Handle(command, CancellationToken.None); diff --git a/tests/ScrumPoker.Application.UnitTests/Features/Commands/RevealVotes/RevealVotesHandlerTests.cs b/tests/ScrumPoker.Application.UnitTests/Features/Commands/RevealVotes/RevealVotesHandlerTests.cs index 472519f..caf62f3 100644 --- a/tests/ScrumPoker.Application.UnitTests/Features/Commands/RevealVotes/RevealVotesHandlerTests.cs +++ b/tests/ScrumPoker.Application.UnitTests/Features/Commands/RevealVotes/RevealVotesHandlerTests.cs @@ -18,14 +18,14 @@ public RevealVotesHandlerTests() } [Fact] - public async Task Handle_Should_ReturnRoomNotFound_When_RoomDoesNotExist() + public async Task Handle_Should_Return_Null_When_Room_DoesNotExist() { var command = new RevealVotesCommand(42); _repository.GetByIdAsync(42, Arg.Any()).Returns((Room?)null); var result = await _sut.Handle(command, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeNull(); } [Fact] @@ -39,12 +39,14 @@ public async Task Handle_Should_SetRevealedTrue() }; var command = new RevealVotesCommand(1); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); + _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => ci.ArgAt(0)); var result = await _sut.Handle(command, CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Revealed.ShouldBeTrue(); - success.Room.Players.Count.ShouldBe(2); + result.ShouldNotBeNull(); + result.Revealed.ShouldBeTrue(); + result.Players.Count.ShouldBe(2); await _repository.Received(1).SaveAsync( Arg.Is(r => r.Revealed), @@ -63,6 +65,8 @@ public async Task Handle_Should_PublishVotesRevealedEvent_OnSuccess() }; var command = new RevealVotesCommand(1); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); + _repository.SaveAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(ci => ci.ArgAt(0)); await _sut.Handle(command, CancellationToken.None); diff --git a/tests/ScrumPoker.Application.UnitTests/Features/Commands/Vote/VoteHandlerTests.cs b/tests/ScrumPoker.Application.UnitTests/Features/Commands/Vote/VoteHandlerTests.cs index 325cbcc..408cb67 100644 --- a/tests/ScrumPoker.Application.UnitTests/Features/Commands/Vote/VoteHandlerTests.cs +++ b/tests/ScrumPoker.Application.UnitTests/Features/Commands/Vote/VoteHandlerTests.cs @@ -18,18 +18,18 @@ public VoteHandlerTests() } [Fact] - public async Task Handle_Should_Return_RoomNotFound_When_Room_DoesNotExist() + public async Task Handle_Should_Return_Null_When_Room_DoesNotExist() { var command = new VoteCommand(42, "Bob", "5"); _repository.GetByIdAsync(42, Arg.Any()).Returns((Room?)null); var result = await _sut.Handle(command, CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeNull(); } [Fact] - public async Task Handle_Should_Return_PlayerNotInRoom_When_Player_NotFound() + public async Task Handle_Should_Throw_When_Player_NotInRoom() { var room = new Room { @@ -39,13 +39,12 @@ public async Task Handle_Should_Return_PlayerNotInRoom_When_Player_NotFound() var command = new VoteCommand(1, "Bob", "5"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - var result = await _sut.Handle(command, CancellationToken.None); - - result.ShouldBeOfType(); + await Should.ThrowAsync(() => + _sut.Handle(command, CancellationToken.None)); } [Fact] - public async Task Handle_Should_Return_Success_With_UpdatedVote_When_Valid() + public async Task Handle_Should_Return_Room_With_UpdatedVote() { var room = new Room { @@ -59,10 +58,10 @@ public async Task Handle_Should_Return_Success_With_UpdatedVote_When_Valid() var result = await _sut.Handle(command, CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Players.ShouldHaveSingleItem(); - success.Room.Players[0].Name.ShouldBe("Alice"); - success.Room.Players[0].Value.ShouldBe("5"); + result.ShouldNotBeNull(); + result.Players.ShouldHaveSingleItem(); + result.Players[0].Name.ShouldBe("Alice"); + result.Players[0].Value.ShouldBe("5"); await _repository.Received(1).SaveAsync( Arg.Is(r => r.Players[0].Value == "5"), @@ -106,10 +105,10 @@ public async Task Handle_Should_Update_Only_Target_Player() var result = await _sut.Handle(command, CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Players.Count.ShouldBe(2); - success.Room.Players.ShouldContain(p => p.Name == "Alice" && p.Value == "3"); - success.Room.Players.ShouldContain(p => p.Name == "Bob" && p.Value == "8"); + result.ShouldNotBeNull(); + result.Players.Count.ShouldBe(2); + result.Players.ShouldContain(p => p.Name == "Alice" && p.Value == "3"); + result.Players.ShouldContain(p => p.Name == "Bob" && p.Value == "8"); } [Fact] @@ -145,7 +144,8 @@ public async Task Handle_Should_NotPublishEvent_When_PlayerNotInRoom() var command = new VoteCommand(1, "Bob", "5"); _repository.GetByIdAsync(1, Arg.Any()).Returns(room); - await _sut.Handle(command, CancellationToken.None); + await Should.ThrowAsync(() => + _sut.Handle(command, CancellationToken.None)); await _bus.DidNotReceiveWithAnyArgs().PublishAsync(Arg.Any()); } diff --git a/tests/ScrumPoker.Application.UnitTests/Features/Queries/GetGameState/GetGameStateHandlerTests.cs b/tests/ScrumPoker.Application.UnitTests/Features/Queries/GetGameState/GetGameStateHandlerTests.cs index 2606f19..abbc20a 100644 --- a/tests/ScrumPoker.Application.UnitTests/Features/Queries/GetGameState/GetGameStateHandlerTests.cs +++ b/tests/ScrumPoker.Application.UnitTests/Features/Queries/GetGameState/GetGameStateHandlerTests.cs @@ -15,26 +15,26 @@ public GetGameStateHandlerTests() } [Fact] - public async Task Handle_Should_ReturnSuccess_When_RoomExists() + public async Task Handle_Should_Return_Room_When_RoomExists() { var room = new Room { Id = 42, Players = [new Player("Alice", null)] }; _repository.GetByIdAsync(42, Arg.Any()).Returns(room); var result = await _sut.Handle(new GetGameStateQuery(42), CancellationToken.None); - var success = result.ShouldBeOfType(); - success.Room.Id.ShouldBe(42); - success.Room.Players.ShouldHaveSingleItem(); - success.Room.Players[0].Name.ShouldBe("Alice"); + result.ShouldNotBeNull(); + result.Id.ShouldBe(42); + result.Players.ShouldHaveSingleItem(); + result.Players[0].Name.ShouldBe("Alice"); } [Fact] - public async Task Handle_Should_ReturnRoomNotFound_When_RoomDoesNotExist() + public async Task Handle_Should_Return_Null_When_Room_DoesNotExist() { _repository.GetByIdAsync(99, Arg.Any()).Returns((Room?)null); var result = await _sut.Handle(new GetGameStateQuery(99), CancellationToken.None); - result.ShouldBeOfType(); + result.ShouldBeNull(); } } diff --git a/web/src/components/JoinRoomForm/JoinRoomForm.tsx b/web/src/components/JoinRoomForm/JoinRoomForm.tsx index a25c093..2fabcb1 100644 --- a/web/src/components/JoinRoomForm/JoinRoomForm.tsx +++ b/web/src/components/JoinRoomForm/JoinRoomForm.tsx @@ -70,8 +70,8 @@ export default function JoinRoomForm({ roomId }: JoinRoomFormProps) { await joinRoom(roomIdNum, trimmed) localStorage.setItem(STORAGE_KEY_NAME, trimmed) navigate(`/${roomIdNum}`) - } catch { - setJoinError('Failed to join room') + } catch (err) { + setJoinError(err instanceof ApiError && err.status === 409 ? 'Name already taken' : 'Failed to join room') } }