Stop throwing for not-found; add Guard and request validation

Not-found lookups return null/false instead of throwing NotFoundException across all services — a missing row is expected control flow, not an exceptional condition. NotFoundException stays for embedded precondition checks inside mutations (missing parent, invalid foreign reference). Guard (copied from mic-check) enforces required arguments at the top of every service method. A ported IModelValidator<T> framework validates every request DTO at the API layer via a new ValidationEndpointFilter, returning a 400 with field-level messages; services re-run the same validator and throw for direct callers that bypass the API. Endpoints translate null/false into 404 via a new ToApiResult() helper. The agent toolset boundary translates the same nullable/bool results into the tool-error text the model already expected.
2026-08-06 15:13:36 -07:00
parent 04917fa09e
commit 40f93e40a8
45 changed files with 1523 additions and 377 deletions
@@ -1,5 +1,6 @@
 using Novelly.Api.Beats;
 using Novelly.Api.Common;
+using Novelly.Api.Common.Validation;
 using Novelly.Api.Scenes;
 using Novelly.Api.Tags;

@@ -53,6 +54,23 @@ public record CreateChapterRequest(
    int? TargetWordCount = null,
    IReadOnlyList<string>? Tags = null);

+public class CreateChapterRequestValidator : IModelValidator<CreateChapterRequest>
+{
+    public ValidationResult Validate(CreateChapterRequest model)
+    {
+        var result = new ValidationResult();
+
+        if (string.IsNullOrWhiteSpace(model.Title))
+            result.AddError("Title", "'Title' must not be empty.");
+        else if (model.Title.Length > 200)
+            result.AddError("Title", "'Title' must be 200 characters or fewer.");
+
+        ChapterValidation.OptionalFields(model.Number, model.Summary, model.Setting, model.Notes, model.TargetWordCount, model.Tags, result);
+
+        return result;
+    }
+}
+
 /// <summary>
 /// Patch-style update. A null field is left alone; an empty string clears it. Passing a
 /// <see cref="Tags"/> list replaces the chapter's tags outright.
@@ -68,6 +86,51 @@ public record UpdateChapterRequest(
    int? TargetWordCount = null,
    IReadOnlyList<string>? Tags = null);

+public class UpdateChapterRequestValidator : IModelValidator<UpdateChapterRequest>
+{
+    public ValidationResult Validate(UpdateChapterRequest model)
+    {
+        var result = new ValidationResult();
+
+        if (model.Title is not null)
+        {
+            if (model.Title.Length == 0)
+                result.AddError("Title", "'Title' can not be cleared — a chapter always needs one.");
+            else if (model.Title.Length > 200)
+                result.AddError("Title", "'Title' must be 200 characters or fewer.");
+        }
+
+        ChapterValidation.OptionalFields(model.Number, model.Summary, model.Setting, model.Notes, model.TargetWordCount, model.Tags, result);
+
+        return result;
+    }
+}
+
+file static class ChapterValidation
+{
+    public static void OptionalFields(
+        int? number, string? summary, string? setting, string? notes, int? targetWordCount, IReadOnlyList<string>? tags, ValidationResult result)
+    {
+        if (number is <= 0)
+            result.AddError("Number", "'Number' must be greater than zero.");
+
+        if (summary is { Length: > 20000 })
+            result.AddError("Summary", "'Summary' must be 20,000 characters or fewer.");
+
+        if (setting is { Length: > 500 })
+            result.AddError("Setting", "'Setting' must be 500 characters or fewer.");
+
+        if (notes is { Length: > 20000 })
+            result.AddError("Notes", "'Notes' must be 20,000 characters or fewer.");
+
+        if (targetWordCount is < 0)
+            result.AddError("TargetWordCount", "'Target Word Count' must be zero or greater.");
+
+        if (tags is not null && tags.Any(string.IsNullOrWhiteSpace))
+            result.AddError("Tags", "'Tags' must not contain blank entries.");
+    }
+}
+
 public static class ChapterMapping
 {
    public static ChapterDto ToDto(this Chapter c) => new(
@@ -1,4 +1,5 @@
 using Novelly.Api.Common;
+using Novelly.Api.Common.Validation;

 namespace Novelly.Api.Chapters;

@@ -6,7 +7,9 @@ public static class ChapterEndpoints
 {
    public static IEndpointRouteBuilder MapChapterEndpoints(this IEndpointRouteBuilder app)
    {
-        var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/chapters").WithTags("Chapters").AddEndpointFilter<RequestLoggingEndpointFilter>();
+        var projectScoped = app.MapGroup("/api/projects/{projectId:guid}/chapters").WithTags("Chapters")
+            .AddEndpointFilter<RequestLoggingEndpointFilter>()
+            .AddEndpointFilter<ValidationEndpointFilter>();

        projectScoped.MapGet("/", async (Guid projectId, ChapterService service, CancellationToken ct) =>
                Results.Ok(await service.ListAsync(projectId, ct)))
@@ -20,22 +23,21 @@ public static class ChapterEndpoints
            })
            .WithSummary("Add a chapter.");

-        var chapters = app.MapGroup("/api/chapters").WithTags("Chapters").AddEndpointFilter<RequestLoggingEndpointFilter>();
+        var chapters = app.MapGroup("/api/chapters").WithTags("Chapters")
+            .AddEndpointFilter<RequestLoggingEndpointFilter>()
+            .AddEndpointFilter<ValidationEndpointFilter>();

        chapters.MapGet("/{id:guid}", async (Guid id, ChapterService service, CancellationToken ct) =>
-                Results.Ok(await service.GetAsync(id, ct)))
+                (await service.GetAsync(id, ct)).ToApiResult())
            .WithSummary("Read a chapter with all of its scenes.");

        chapters.MapPatch("/{id:guid}", async (
                Guid id, UpdateChapterRequest request, ChapterService service, CancellationToken ct) =>
-                Results.Ok(await service.UpdateAsync(id, request, ct)))
+                (await service.UpdateAsync(id, request, ct)).ToApiResult())
            .WithSummary("Update a chapter.");

        chapters.MapDelete("/{id:guid}", async (Guid id, ChapterService service, CancellationToken ct) =>
-            {
-                await service.DeleteAsync(id, ct);
-                return Results.NoContent();
-            })
+                await service.DeleteAsync(id, ct) ? Results.NoContent() : Results.NotFound())
            .WithSummary("Delete a chapter and its scenes.");

        return app;
@@ -1,15 +1,23 @@
 using Microsoft.EntityFrameworkCore;
 using Novelly.Api.Common;
+using Novelly.Api.Common.Validation;
 using Novelly.Api.Data;
 using Novelly.Api.Projects;
 using Novelly.Api.Tags;

 namespace Novelly.Api.Chapters;

-public class ChapterService(INovelDbContext db, TagService tags, ILogger<ChapterService> logger)
+public class ChapterService(
+    INovelDbContext db,
+    TagService tags,
+    ILogger<ChapterService> logger,
+    IModelValidator<CreateChapterRequest> createValidator,
+    IModelValidator<UpdateChapterRequest> updateValidator)
 {
    public async Task<IReadOnlyList<ChapterSummaryDto>> ListAsync(Guid projectId, CancellationToken ct = default)
    {
+        Guard.Default(projectId, nameof(projectId));
+
        logger.LogInformation("Listing chapters for project {ProjectId}", projectId);

        var chapters = await db.Chapters
@@ -24,19 +32,26 @@ public class ChapterService(INovelDbContext db, TagService tags, ILogger<Chapter
        return [.. chapters.Select(c => c.ToSummaryDto())];
    }

-    public async Task<ChapterDto> GetAsync(Guid id, CancellationToken ct = default)
+    /// <summary>Null when no chapter has this id — a lookup miss is expected, not exceptional.</summary>
+    public async Task<ChapterDto?> GetAsync(Guid id, CancellationToken ct = default)
    {
+        Guard.Default(id, nameof(id));
+
        logger.LogInformation("Getting chapter {ChapterId}", id);
-        return (await FindAsync(id, ct)).ToDto();
+        return (await FindAsync(id, ct))?.ToDto();
    }

    public async Task<ChapterDto> CreateAsync(Guid projectId, CreateChapterRequest request, CancellationToken ct = default)
    {
+        Guard.Default(projectId, nameof(projectId));
+        Guard.Null(request, nameof(request));
+        createValidator.Validate(request).ThrowIfInvalid();
+
        logger.LogInformation("Creating chapter {Title} for project {ProjectId}", request.Title, projectId);

        if (!await db.Projects.AnyAsync(p => p.Id == projectId, ct))
        {
-            logger.LogWarning("Project {ProjectId} not found", projectId);
+            logger.LogWarning("Rejected chapter creation: project {ProjectId} not found", projectId);
            throw new NotFoundException(nameof(Project), projectId);
        }

@@ -60,14 +75,24 @@ public class ChapterService(INovelDbContext db, TagService tags, ILogger<Chapter

        db.Chapters.Add(chapter);
        await db.SaveChangesAsync(ct);
-        return (await FindAsync(chapter.Id, ct)).ToDto();
+
+        // Just created it — the reload is only to pick up includes, not to check existence.
+        return (await FindAsync(chapter.Id, ct))!.ToDto();
    }

-    public async Task<ChapterDto> UpdateAsync(Guid id, UpdateChapterRequest request, CancellationToken ct = default)
+    public async Task<ChapterDto?> UpdateAsync(Guid id, UpdateChapterRequest request, CancellationToken ct = default)
    {
+        Guard.Default(id, nameof(id));
+        Guard.Null(request, nameof(request));
+        updateValidator.Validate(request).ThrowIfInvalid();
+
        logger.LogInformation("Updating chapter {ChapterId}", id);

        var chapter = await FindAsync(id, ct);
+        if (chapter is null)
+        {
+            return null;
+        }

        chapter.Title = Patch.Apply(chapter.Title, request.Title) ?? chapter.Title;
        chapter.Number = request.Number ?? chapter.Number;
@@ -85,16 +110,25 @@ public class ChapterService(INovelDbContext db, TagService tags, ILogger<Chapter
        }

        await db.SaveChangesAsync(ct);
-        return (await FindAsync(id, ct)).ToDto();
+        return (await FindAsync(id, ct))!.ToDto();
    }

-    public async Task DeleteAsync(Guid id, CancellationToken ct = default)
+    /// <summary>True if a chapter was deleted; false if no chapter had this id.</summary>
+    public async Task<bool> DeleteAsync(Guid id, CancellationToken ct = default)
    {
+        Guard.Default(id, nameof(id));
+
        logger.LogInformation("Deleting chapter {ChapterId}", id);

        var chapter = await FindAsync(id, ct);
+        if (chapter is null)
+        {
+            return false;
+        }
+
        db.Chapters.Remove(chapter);
        await db.SaveChangesAsync(ct);
+        return true;
    }

    private async Task<int> NextChapterNumberAsync(Guid projectId, CancellationToken ct)
@@ -110,7 +144,7 @@ public class ChapterService(INovelDbContext db, TagService tags, ILogger<Chapter
        return next;
    }

-    private async Task<Chapter> FindAsync(Guid id, CancellationToken ct)
+    private async Task<Chapter?> FindAsync(Guid id, CancellationToken ct)
    {
        logger.LogDebug("Finding chapter {ChapterId}", id);

@@ -125,11 +159,13 @@ public class ChapterService(INovelDbContext db, TagService tags, ILogger<Chapter

        if (chapter is null)
        {
-            logger.LogWarning("Chapter {ChapterId} not found", id);
-            throw new NotFoundException(nameof(Chapter), id);
+            logger.LogInformation("Chapter {ChapterId} not found", id);
+        }
+        else
+        {
+            logger.LogDebug("Found chapter {ChapterId}", id);
        }

-        logger.LogDebug("Found chapter {ChapterId}", id);
        return chapter;
    }
 }