fix(pr-review): resolve inline review comments regarding debounce race and result pattern

This commit is contained in:
2026-06-15 19:14:13 +02:00
parent b74ba4ba54
commit 8de02c32c5
6 changed files with 51 additions and 45 deletions
@@ -38,7 +38,7 @@ public class PublishBookVersionCommandHandler : ICommandHandler<PublishBookVersi
if (book == null) if (book == null)
{ {
throw new BookNotFoundException(request.BookId); return Result.Fail(new Error($"Book with ID '{request.BookId}' was not found."));
} }
var oldDraftRevision = book.CurrentDraftRevision; var oldDraftRevision = book.CurrentDraftRevision;
@@ -41,7 +41,7 @@ public class GetBookRevisionsQueryHandler : IQueryHandler<GetBookRevisionsQuery,
if (!bookExists) if (!bookExists)
{ {
throw new BookNotFoundException(request.BookId); return FluentResults.Result.Fail<List<CreatorBookRevisionDto>>(new FluentResults.Error($"Book with ID '{request.BookId}' was not found."));
} }
// Fetch all revisions sorted chronologically // Fetch all revisions sorted chronologically
@@ -43,6 +43,7 @@
private IJSObjectReference? _module; private IJSObjectReference? _module;
private DotNetObjectReference<MarkdownEditor>? _dotNetHelper; private DotNetObjectReference<MarkdownEditor>? _dotNetHelper;
private string? _lastInitializedEditorId; private string? _lastInitializedEditorId;
private bool _disposed;
private enum SaveStatus private enum SaveStatus
{ {
@@ -350,6 +351,7 @@
// Cancel pending timers thread-safely // Cancel pending timers thread-safely
CancellationTokenSource? ctsToCancel = null; CancellationTokenSource? ctsToCancel = null;
CancellationToken token;
lock (_timerLock) lock (_timerLock)
{ {
if (_debounceCts != null) if (_debounceCts != null)
@@ -358,6 +360,7 @@
_debounceCts = null; _debounceCts = null;
} }
_debounceCts = new CancellationTokenSource(); _debounceCts = new CancellationTokenSource();
token = _debounceCts.Token; // Capture token synchronously under lock on UI thread
} }
if (ctsToCancel != null) if (ctsToCancel != null)
@@ -376,13 +379,6 @@
// Start 5-second idle debounce timer // Start 5-second idle debounce timer
_ = Task.Run(async () => _ = Task.Run(async () =>
{ {
CancellationToken token;
lock (_timerLock)
{
if (_debounceCts == null) return;
token = _debounceCts.Token;
}
try try
{ {
await Task.Delay(5000, token); await Task.Delay(5000, token);
@@ -401,7 +397,7 @@
private async Task TriggerAutosaveAsync(string markdown, CancellationToken token) private async Task TriggerAutosaveAsync(string markdown, CancellationToken token)
{ {
if (token.IsCancellationRequested) return; if (token.IsCancellationRequested || _disposed) return;
_status = SaveStatus.Saving; _status = SaveStatus.Saving;
await InvokeAsync(StateHasChanged); await InvokeAsync(StateHasChanged);
@@ -416,6 +412,8 @@
token token
); );
if (_disposed) return;
if (response.IsSuccessStatusCode) if (response.IsSuccessStatusCode)
{ {
// Purge LocalStorage backup key on HTTP success // Purge LocalStorage backup key on HTTP success
@@ -431,10 +429,12 @@
} }
catch (Exception ex) catch (Exception ex)
{ {
if (_disposed) return;
_status = SaveStatus.OfflineLocalBackup; _status = SaveStatus.OfflineLocalBackup;
Console.WriteLine($"[MarkdownEditor] Autosave HTTP exception: {ex.Message}"); Console.WriteLine($"[MarkdownEditor] Autosave HTTP exception: {ex.Message}");
} }
if (_disposed) return;
await InvokeAsync(StateHasChanged); await InvokeAsync(StateHasChanged);
} }
@@ -477,6 +477,7 @@
public async ValueTask DisposeAsync() public async ValueTask DisposeAsync()
{ {
_disposed = true;
try try
{ {
_cts.Cancel(); _cts.Cancel();
+12 -18
View File
@@ -519,18 +519,15 @@ app.MapGet("/api/creator/books/{bookId:guid}/revisions", async (Guid bookId, Cla
var tenantId = user.FindFirstValue("TenantId") ?? "global"; 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"; var errorMsg = result.Errors.Count > 0 ? result.Errors[0].Message : "Unknown server error";
return Results.BadRequest(errorMsg); if (errorMsg.Contains("was not found", StringComparison.OrdinalIgnoreCase))
}
catch (NexusReader.Domain.Exceptions.BookNotFoundException)
{ {
return Results.NotFound($"Book with ID '{bookId}' was not found."); return Results.NotFound(errorMsg);
} }
return Results.BadRequest(errorMsg);
}).RequireAuthorization(); }).RequireAuthorization();
app.MapPost("/api/creator/books/{bookId:guid}/publish", async (Guid bookId, [FromQuery] string version, ClaimsPrincipal user, IMediator mediator) => 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."); 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"; var errorMsg = result.Errors.Count > 0 ? result.Errors[0].Message : "Unknown server error";
return Results.BadRequest(errorMsg); if (errorMsg.Contains("was not found", StringComparison.OrdinalIgnoreCase))
}
catch (NexusReader.Domain.Exceptions.BookNotFoundException)
{ {
return Results.NotFound($"Book with ID '{bookId}' was not found."); return Results.NotFound(errorMsg);
} }
return Results.BadRequest(errorMsg);
}).RequireAuthorization(); }).RequireAuthorization();
app.MapPost("/api/creator/books", async ( app.MapPost("/api/creator/books", async (
@@ -169,7 +169,7 @@ public class PublishBookVersionTests : IDisposable
} }
[Fact] [Fact]
public async Task Handle_WithMismatchedTenantId_ThrowsBookNotFoundException() public async Task Handle_WithMismatchedTenantId_ReturnsFailure()
{ {
// Arrange // Arrange
var bookId = Guid.NewGuid(); var bookId = Guid.NewGuid();
@@ -210,13 +210,16 @@ public class PublishBookVersionTests : IDisposable
var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object); var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object);
// Act & Assert // Act
var action = () => handler.Handle(command, CancellationToken.None); var result = await handler.Handle(command, CancellationToken.None);
await action.Should().ThrowAsync<BookNotFoundException>();
// Assert
result.IsSuccess.Should().BeFalse();
result.Errors.Should().Contain(e => e.Message.Contains("was not found"));
} }
[Fact] [Fact]
public async Task Handle_WithMismatchedUserId_ThrowsBookNotFoundException() public async Task Handle_WithMismatchedUserId_ReturnsFailure()
{ {
// Arrange // Arrange
var bookId = Guid.NewGuid(); var bookId = Guid.NewGuid();
@@ -257,13 +260,16 @@ public class PublishBookVersionTests : IDisposable
var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object); var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object);
// Act & Assert // Act
var action = () => handler.Handle(command, CancellationToken.None); var result = await handler.Handle(command, CancellationToken.None);
await action.Should().ThrowAsync<BookNotFoundException>();
// Assert
result.IsSuccess.Should().BeFalse();
result.Errors.Should().Contain(e => e.Message.Contains("was not found"));
} }
[Fact] [Fact]
public async Task Handle_WithNonExistentBook_ThrowsBookNotFoundException() public async Task Handle_WithNonExistentBook_ReturnsFailure()
{ {
// Arrange // Arrange
var command = new PublishBookVersionCommand( var command = new PublishBookVersionCommand(
@@ -275,9 +281,12 @@ public class PublishBookVersionTests : IDisposable
var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object); var handler = new PublishBookVersionCommandHandler(_dbContextFactoryMock.Object);
// Act & Assert // Act
var action = () => handler.Handle(command, CancellationToken.None); var result = await handler.Handle(command, CancellationToken.None);
await action.Should().ThrowAsync<BookNotFoundException>();
// Assert
result.IsSuccess.Should().BeFalse();
result.Errors.Should().Contain(e => e.Message.Contains("was not found"));
} }
public void Dispose() public void Dispose()
@@ -234,7 +234,7 @@ public class CreatorDashboardTests : IDisposable
} }
[Fact] [Fact]
public async Task GetBookRevisions_WithMismatchedUserOrTenant_ThrowsBookNotFoundException() public async Task GetBookRevisions_WithMismatchedUserOrTenant_ReturnsFailure()
{ {
// Arrange // Arrange
var userId = "creator-123"; var userId = "creator-123";
@@ -262,12 +262,14 @@ public class CreatorDashboardTests : IDisposable
// Act & Assert // Act & Assert
var queryMismatchedTenant = new GetBookRevisionsQuery(bookId, userId, "different-tenant"); var queryMismatchedTenant = new GetBookRevisionsQuery(bookId, userId, "different-tenant");
var actionTenant = () => handler.Handle(queryMismatchedTenant, CancellationToken.None); var resultTenant = await handler.Handle(queryMismatchedTenant, CancellationToken.None);
await actionTenant.Should().ThrowAsync<BookNotFoundException>(); resultTenant.IsSuccess.Should().BeFalse();
resultTenant.Errors.Should().Contain(e => e.Message.Contains("was not found"));
var queryMismatchedUser = new GetBookRevisionsQuery(bookId, "different-user", tenantId); var queryMismatchedUser = new GetBookRevisionsQuery(bookId, "different-user", tenantId);
var actionUser = () => handler.Handle(queryMismatchedUser, CancellationToken.None); var resultUser = await handler.Handle(queryMismatchedUser, CancellationToken.None);
await actionUser.Should().ThrowAsync<BookNotFoundException>(); resultUser.IsSuccess.Should().BeFalse();
resultUser.Errors.Should().Contain(e => e.Message.Contains("was not found"));
} }
public void Dispose() public void Dispose()