From 8de02c32c53e95a51432198895214f034b1bacbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Jasi=C5=84ski?= Date: Mon, 15 Jun 2026 19:14:13 +0200 Subject: [PATCH] fix(pr-review): resolve inline review comments regarding debounce race and result pattern --- .../PublishBookVersionCommandHandler.cs | 2 +- .../Queries/Creator/GetBookRevisionsQuery.cs | 2 +- .../Components/MarkdownEditor.razor | 17 +++++----- src/NexusReader.Web/Program.cs | 30 +++++++---------- .../Commands/PublishBookVersionTests.cs | 33 ++++++++++++------- .../Queries/CreatorDashboardTests.cs | 12 ++++--- 6 files changed, 51 insertions(+), 45 deletions(-) diff --git a/src/NexusReader.Application/Features/Books/Commands/PublishBookVersionCommandHandler.cs b/src/NexusReader.Application/Features/Books/Commands/PublishBookVersionCommandHandler.cs index 7525437..bc4ceb9 100644 --- a/src/NexusReader.Application/Features/Books/Commands/PublishBookVersionCommandHandler.cs +++ b/src/NexusReader.Application/Features/Books/Commands/PublishBookVersionCommandHandler.cs @@ -38,7 +38,7 @@ public class PublishBookVersionCommandHandler : ICommandHandler>(new FluentResults.Error($"Book with ID '{request.BookId}' was not found.")); } // Fetch all revisions sorted chronologically diff --git a/src/NexusReader.UI.Shared/Components/MarkdownEditor.razor b/src/NexusReader.UI.Shared/Components/MarkdownEditor.razor index 4c9fe7e..9cdc5d4 100644 --- a/src/NexusReader.UI.Shared/Components/MarkdownEditor.razor +++ b/src/NexusReader.UI.Shared/Components/MarkdownEditor.razor @@ -43,6 +43,7 @@ private IJSObjectReference? _module; private DotNetObjectReference? _dotNetHelper; private string? _lastInitializedEditorId; + private bool _disposed; private enum SaveStatus { @@ -350,6 +351,7 @@ // Cancel pending timers thread-safely CancellationTokenSource? ctsToCancel = null; + CancellationToken token; lock (_timerLock) { if (_debounceCts != null) @@ -358,6 +360,7 @@ _debounceCts = null; } _debounceCts = new CancellationTokenSource(); + token = _debounceCts.Token; // Capture token synchronously under lock on UI thread } if (ctsToCancel != null) @@ -376,13 +379,6 @@ // Start 5-second idle debounce timer _ = Task.Run(async () => { - CancellationToken token; - lock (_timerLock) - { - if (_debounceCts == null) return; - token = _debounceCts.Token; - } - try { await Task.Delay(5000, token); @@ -401,7 +397,7 @@ private async Task TriggerAutosaveAsync(string markdown, CancellationToken token) { - if (token.IsCancellationRequested) return; + if (token.IsCancellationRequested || _disposed) return; _status = SaveStatus.Saving; await InvokeAsync(StateHasChanged); @@ -416,6 +412,8 @@ token ); + if (_disposed) return; + if (response.IsSuccessStatusCode) { // Purge LocalStorage backup key on HTTP success @@ -431,10 +429,12 @@ } catch (Exception ex) { + if (_disposed) return; _status = SaveStatus.OfflineLocalBackup; Console.WriteLine($"[MarkdownEditor] Autosave HTTP exception: {ex.Message}"); } + if (_disposed) return; await InvokeAsync(StateHasChanged); } @@ -477,6 +477,7 @@ public async ValueTask DisposeAsync() { + _disposed = true; try { _cts.Cancel(); diff --git a/src/NexusReader.Web/Program.cs b/src/NexusReader.Web/Program.cs index 28bf8a7..d3484d3 100644 --- a/src/NexusReader.Web/Program.cs +++ b/src/NexusReader.Web/Program.cs @@ -519,18 +519,15 @@ app.MapGet("/api/creator/books/{bookId:guid}/revisions", async (Guid bookId, Cla var tenantId = user.FindFirstValue("TenantId") ?? "global"; - try - { - var result = await mediator.Send(new NexusReader.Application.Queries.Creator.GetBookRevisionsQuery(bookId, userId, tenantId)); - if (result.IsSuccess) return Results.Ok(result.Value); + var result = await mediator.Send(new NexusReader.Application.Queries.Creator.GetBookRevisionsQuery(bookId, userId, tenantId)); + if (result.IsSuccess) return Results.Ok(result.Value); - var errorMsg = result.Errors.Count > 0 ? result.Errors[0].Message : "Unknown server error"; - return Results.BadRequest(errorMsg); - } - catch (NexusReader.Domain.Exceptions.BookNotFoundException) + var errorMsg = result.Errors.Count > 0 ? result.Errors[0].Message : "Unknown server error"; + if (errorMsg.Contains("was not found", StringComparison.OrdinalIgnoreCase)) { - return Results.NotFound($"Book with ID '{bookId}' was not found."); + return Results.NotFound(errorMsg); } + return Results.BadRequest(errorMsg); }).RequireAuthorization(); app.MapPost("/api/creator/books/{bookId:guid}/publish", async (Guid bookId, [FromQuery] string version, ClaimsPrincipal user, IMediator mediator) => @@ -545,18 +542,15 @@ app.MapPost("/api/creator/books/{bookId:guid}/publish", async (Guid bookId, [Fro return Results.BadRequest("Version string is required."); } - try - { - var result = await mediator.Send(new NexusReader.Application.Features.Books.Commands.PublishBookVersionCommand(bookId, version, userId, tenantId)); - if (result.IsSuccess) return Results.Ok(); + var result = await mediator.Send(new NexusReader.Application.Features.Books.Commands.PublishBookVersionCommand(bookId, version, userId, tenantId)); + if (result.IsSuccess) return Results.Ok(); - var errorMsg = result.Errors.Count > 0 ? result.Errors[0].Message : "Unknown server error"; - return Results.BadRequest(errorMsg); - } - catch (NexusReader.Domain.Exceptions.BookNotFoundException) + var errorMsg = result.Errors.Count > 0 ? result.Errors[0].Message : "Unknown server error"; + if (errorMsg.Contains("was not found", StringComparison.OrdinalIgnoreCase)) { - return Results.NotFound($"Book with ID '{bookId}' was not found."); + return Results.NotFound(errorMsg); } + return Results.BadRequest(errorMsg); }).RequireAuthorization(); app.MapPost("/api/creator/books", async ( diff --git a/tests/NexusReader.Application.Tests/Commands/PublishBookVersionTests.cs b/tests/NexusReader.Application.Tests/Commands/PublishBookVersionTests.cs index a5bfe18..7e40963 100644 --- a/tests/NexusReader.Application.Tests/Commands/PublishBookVersionTests.cs +++ b/tests/NexusReader.Application.Tests/Commands/PublishBookVersionTests.cs @@ -169,7 +169,7 @@ public class PublishBookVersionTests : IDisposable } [Fact] - public async Task Handle_WithMismatchedTenantId_ThrowsBookNotFoundException() + public async Task Handle_WithMismatchedTenantId_ReturnsFailure() { // Arrange var bookId = Guid.NewGuid(); @@ -210,13 +210,16 @@ public class PublishBookVersionTests : IDisposable var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object); - // Act & Assert - var action = () => handler.Handle(command, CancellationToken.None); - await action.Should().ThrowAsync(); + // Act + var result = await handler.Handle(command, CancellationToken.None); + + // Assert + result.IsSuccess.Should().BeFalse(); + result.Errors.Should().Contain(e => e.Message.Contains("was not found")); } [Fact] - public async Task Handle_WithMismatchedUserId_ThrowsBookNotFoundException() + public async Task Handle_WithMismatchedUserId_ReturnsFailure() { // Arrange var bookId = Guid.NewGuid(); @@ -257,13 +260,16 @@ public class PublishBookVersionTests : IDisposable var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object); - // Act & Assert - var action = () => handler.Handle(command, CancellationToken.None); - await action.Should().ThrowAsync(); + // Act + var result = await handler.Handle(command, CancellationToken.None); + + // Assert + result.IsSuccess.Should().BeFalse(); + result.Errors.Should().Contain(e => e.Message.Contains("was not found")); } [Fact] - public async Task Handle_WithNonExistentBook_ThrowsBookNotFoundException() + public async Task Handle_WithNonExistentBook_ReturnsFailure() { // Arrange var command = new PublishBookVersionCommand( @@ -275,9 +281,12 @@ public class PublishBookVersionTests : IDisposable var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object); - // Act & Assert - var action = () => handler.Handle(command, CancellationToken.None); - await action.Should().ThrowAsync(); + // Act + var result = await handler.Handle(command, CancellationToken.None); + + // Assert + result.IsSuccess.Should().BeFalse(); + result.Errors.Should().Contain(e => e.Message.Contains("was not found")); } public void Dispose() diff --git a/tests/NexusReader.Application.Tests/Queries/CreatorDashboardTests.cs b/tests/NexusReader.Application.Tests/Queries/CreatorDashboardTests.cs index 15f50ff..2450d32 100644 --- a/tests/NexusReader.Application.Tests/Queries/CreatorDashboardTests.cs +++ b/tests/NexusReader.Application.Tests/Queries/CreatorDashboardTests.cs @@ -234,7 +234,7 @@ public class CreatorDashboardTests : IDisposable } [Fact] - public async Task GetBookRevisions_WithMismatchedUserOrTenant_ThrowsBookNotFoundException() + public async Task GetBookRevisions_WithMismatchedUserOrTenant_ReturnsFailure() { // Arrange var userId = "creator-123"; @@ -262,12 +262,14 @@ public class CreatorDashboardTests : IDisposable // Act & Assert var queryMismatchedTenant = new GetBookRevisionsQuery(bookId, userId, "different-tenant"); - var actionTenant = () => handler.Handle(queryMismatchedTenant, CancellationToken.None); - await actionTenant.Should().ThrowAsync(); + var resultTenant = await handler.Handle(queryMismatchedTenant, CancellationToken.None); + resultTenant.IsSuccess.Should().BeFalse(); + resultTenant.Errors.Should().Contain(e => e.Message.Contains("was not found")); var queryMismatchedUser = new GetBookRevisionsQuery(bookId, "different-user", tenantId); - var actionUser = () => handler.Handle(queryMismatchedUser, CancellationToken.None); - await actionUser.Should().ThrowAsync(); + var resultUser = await handler.Handle(queryMismatchedUser, CancellationToken.None); + resultUser.IsSuccess.Should().BeFalse(); + resultUser.Errors.Should().Contain(e => e.Message.Contains("was not found")); } public void Dispose()