Merge pull request #12 from sheepla/devin/1777622660-fix-discovered-issues

Fix discovered design issues: AsParameters defaults, CsvBindingExceptionFilter, dead code

Author: sheepla

src/AxisEndpoints.Extensions.CsvHelper/CsvBindingExceptionFilter.cs

@@ -3,11 +3,15 @@
 namespace AxisEndpoints.Extensions.CsvHelper;
 
 /// <summary>
-/// An endpoint filter that catches <see cref="CsvBindingException"/> thrown during CSV
-/// request binding and converts it to an RFC 9457 <c>ValidationProblem</c> response,
-/// consistent with the error shape produced by AxisEndpoints' built-in
+/// An endpoint filter that inspects <see cref="CsvRequest{TRow}.BindingErrors"/> collected
+/// during CSV request binding and converts them to an RFC 9457 <c>ValidationProblem</c>
+/// response, consistent with the error shape produced by AxisEndpoints' built-in
 /// DataAnnotations validation filter.
 ///
+/// Because <c>BindAsync</c> executes before the endpoint filter pipeline, the previous
+/// approach of catching <see cref="CsvBindingException"/> in a try/catch could never work.
+/// This filter instead checks the bound request argument for deferred errors.
+///
 /// Register this filter on endpoints that accept a <see cref="CsvRequest{TRow}"/> parameter:
 /// <code>
 /// config.Post("/import").AddFilter&lt;CsvBindingExceptionFilter&gt;();
@@ -20,13 +24,14 @@ public sealed class CsvBindingExceptionFilter : IEndpointFilter
         EndpointFilterDelegate next
     )
     {
-        try
-        {
-            return await next(context);
-        }
-        catch (CsvBindingException ex)
+        foreach (var argument in context.Arguments)
         {
-            return TypedResults.ValidationProblem(ex.Errors);
+            if (argument is ICsvBindingErrors { BindingErrors: { Count: > 0 } errors })
+            {
+                return TypedResults.ValidationProblem(errors);
+            }
         }
+
+        return await next(context);
     }
 }

src/AxisEndpoints.Extensions.CsvHelper/CsvRequest.cs

@@ -26,15 +26,23 @@ namespace AxisEndpoints.Extensions.CsvHelper;
 /// derived class to customise CsvHelper behaviour without touching the binding logic.
 ///
 /// DataAnnotations placed on <typeparamref name="TRow"/> are validated per-row during binding.
-/// Errors are collected across all rows and surfaced as a <see cref="CsvBindingException"/>
-/// before the endpoint handler is invoked.
+/// Errors are collected across all rows and stored in <see cref="BindingErrors"/> so that
+/// <see cref="CsvBindingExceptionFilter"/> can inspect them inside the endpoint filter pipeline.
 /// </summary>
 /// <typeparam name="TRow">The strongly-typed row model.</typeparam>
-public abstract class CsvRequest<TRow>
+public abstract class CsvRequest<TRow> : ICsvBindingErrors
 {
     /// <summary>The rows parsed from the CSV body. Empty when the body contained no data rows.</summary>
     public IReadOnlyList<TRow> Rows { get; private set; } = [];
 
+    /// <summary>
+    /// Validation errors collected during binding, keyed by "row {n}: {memberName}".
+    /// Empty when all rows pass validation. Inspected by <see cref="CsvBindingExceptionFilter"/>
+    /// to return a <c>ValidationProblem</c> response before the handler is invoked.
+    /// </summary>
+    public IReadOnlyDictionary<string, string[]> BindingErrors { get; private set; } =
+        new Dictionary<string, string[]>();
+
     // -------------------------------------------------------------------------
     // Binding helper — call this from the derived class's BindAsync
     // -------------------------------------------------------------------------
@@ -73,7 +81,7 @@ protected static async ValueTask<TDerived> BindCsvAsync<TDerived>(HttpContext co
 
         if (errors.Count > 0)
         {
-            throw new CsvBindingException(errors);
+            instance.BindingErrors = errors;
         }
 
         instance.Rows = rows;
@@ -113,57 +121,61 @@ private static async Task<Stream> ResolveBodyStreamAsync(HttpContext context)
     {
         var contentType = context.Request.ContentType ?? string.Empty;
 
-        if (contentType.StartsWith("multipart/form-data", StringComparison.OrdinalIgnoreCase))
+        if (contentType.Contains("multipart/form-data", StringComparison.OrdinalIgnoreCase))
         {
             var form = await context.Request.ReadFormAsync();
             var file =
                 form.Files.FirstOrDefault()
-                ?? throw new InvalidOperationException(
-                    "No file was found in the multipart/form-data request."
+                ?? throw new BadHttpRequestException(
+                    "Expected a CSV file in the multipart/form-data request."
                 );
             return file.OpenReadStream();
         }
 
-        // Fall through for text/csv and application/octet-stream.
-        return context.Request.Body;
+        // Assume the body itself is the CSV content (text/csv or similar).
+        // Return the body stream wrapped in a MemoryStream so callers can dispose it
+        // without closing the original request body.
+        var ms = new MemoryStream();
+        await context.Request.Body.CopyToAsync(ms);
+        ms.Position = 0;
+        return ms;
     }
 
     /// <summary>
-    /// Runs DataAnnotations validation against a single parsed row.
-    /// Errors are keyed as "row {rowNumber}: {memberName}" to surface the source line.
+    /// Validates a single row using DataAnnotations and appends any errors to the
+    /// <paramref name="errors"/> dictionary keyed by <c>"row {rowNumber}: {memberName}"</c>.
     /// </summary>
-    private static void ValidateRow(TRow row, int rowNumber, Dictionary<string, string[]> errors)
+    private static void ValidateRow(
+        TRow row,
+        int rowNumber,
+        Dictionary<string, string[]> errors
+    )
     {
         if (row is null)
         {
             return;
         }
 
-        var validationContext = new ValidationContext(row);
-        var validationResults = new List<ValidationResult>();
-
-        if (
-            Validator.TryValidateObject(
-                row,
-                validationContext,
-                validationResults,
-                validateAllProperties: true
-            )
-        )
+        var context = new ValidationContext(row);
+        var results = new List<ValidationResult>();
+
+        if (Validator.TryValidateObject(row, context, results, validateAllProperties: true))
         {
             return;
         }
 
-        foreach (var result in validationResults)
+        foreach (var result in results)
         {
-            foreach (var member in result.MemberNames.DefaultIfEmpty(string.Empty))
+            var memberName = result.MemberNames.FirstOrDefault() ?? "(unknown)";
+            var key = $"row {rowNumber}: {memberName}";
+            var message = result.ErrorMessage ?? "Validation failed.";
+            if (errors.TryGetValue(key, out var existing))
             {
-                var key = $"row {rowNumber}: {member}";
-                var message = result.ErrorMessage ?? "Validation failed.";
-
-                errors[key] = errors.TryGetValue(key, out var existing)
-                    ? [.. existing, message]
-                    : [message];
+                errors[key] = [..existing, message];
+            }
+            else
+            {
+                errors[key] = [message];
             }
         }
     }

src/AxisEndpoints.Extensions.CsvHelper/ICsvBindingErrors.cs

@@ -0,0 +1,15 @@
+namespace AxisEndpoints.Extensions.CsvHelper;
+
+/// <summary>
+/// Non-generic interface exposing CSV binding validation errors. Implemented by
+/// <see cref="CsvRequest{TRow}"/> so that <see cref="CsvBindingExceptionFilter"/>
+/// can inspect binding errors without knowing the concrete generic type.
+/// </summary>
+public interface ICsvBindingErrors
+{
+    /// <summary>
+    /// Validation errors collected during binding, keyed by "row {n}: {memberName}".
+    /// Empty when all rows pass validation.
+    /// </summary>
+    IReadOnlyDictionary<string, string[]> BindingErrors { get; }
+}

tests/AxisEndpoints.Example.Tests/CsvEndpointTests.cs

@@ -1,6 +1,5 @@
 using System.Net;
 using System.Text;
-using AxisEndpoints.Extensions.CsvHelper;
 using FluentAssertions;
 
 namespace AxisEndpoints.Example.Tests;
@@ -49,22 +48,14 @@ public async Task ImportCsv_ValidCsv_Returns204()
         response.StatusCode.Should().Be(HttpStatusCode.NoContent);
     }
 
-    /// <summary>
-    /// CsvBindingException is thrown during BindAsync (parameter binding phase),
-    /// which executes before the endpoint filter pipeline. As a result,
-    /// CsvBindingExceptionFilter cannot catch it and the exception propagates
-    /// as an unhandled server error.
-    /// </summary>
     [Fact]
-    public async Task ImportCsv_InvalidRow_ThrowsCsvBindingException()
+    public async Task ImportCsv_InvalidRow_Returns400ValidationProblem()
     {
         var csvContent = "name,email,role\n,invalid-email,";
         var content = new StringContent(csvContent, Encoding.UTF8, "text/csv");
 
-        var act = () => _client.PostAsync("/api/users/users/import", content);
+        var response = await _client.PostAsync("/api/users/users/import", content);
 
-        await act.Should()
-            .ThrowAsync<Exception>()
-            .Where(ex => ex.ToString().Contains(nameof(CsvBindingException)));
+        response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
     }
 }

tests/AxisEndpoints.Example.Tests/ListUsersEndpointTests.cs

@@ -3,6 +3,7 @@
 using AxisEndpoints.Example.Features.Users;
 using AxisEndpoints.Example.Features.Users.List;
 using FluentAssertions;
+using Microsoft.AspNetCore.Mvc;
 
 namespace AxisEndpoints.Example.Tests;
 
@@ -18,7 +19,7 @@ public ListUsersEndpointTests(ExampleWebApplicationFactory factory)
     [Fact]
     public async Task ListUsers_DefaultPagination_ReturnsAllUsers()
     {
-        var response = await _client.GetAsync("/api/users?page=1&pageSize=20");
+        var response = await _client.GetAsync("/api/users");
 
         response.StatusCode.Should().Be(HttpStatusCode.OK);
         var body = await response.Content.ReadFromJsonAsync<ListUsersResponse>();
@@ -30,7 +31,7 @@ public async Task ListUsers_DefaultPagination_ReturnsAllUsers()
     }
 
     [Fact]
-    public async Task ListUsers_WithPagination_ReturnsPagedResults()
+    public async Task ListUsers_WithExplicitPagination_ReturnsPagedResults()
     {
         var response = await _client.GetAsync("/api/users?page=1&pageSize=2");
 
@@ -46,7 +47,7 @@ public async Task ListUsers_WithPagination_ReturnsPagedResults()
     [Fact]
     public async Task ListUsers_WithRoleFilter_ReturnsFilteredResults()
     {
-        var response = await _client.GetAsync("/api/users?role=Admin&page=1&pageSize=20");
+        var response = await _client.GetAsync("/api/users?role=Admin");
 
         response.StatusCode.Should().Be(HttpStatusCode.OK);
         var body = await response.Content.ReadFromJsonAsync<ListUsersResponse>();
@@ -70,17 +71,25 @@ public async Task ListUsers_SecondPage_ReturnsCorrectItems()
         body.Items[1].Name.Should().Be("Diana");
     }
 
-    /// <summary>
-    /// When page/pageSize query parameters are omitted, [AsParameters] binding
-    /// uses the CLR default (0) instead of the property initializer values (1, 20).
-    /// DataAnnotations [Range(1, ...)] validation then rejects 0, returning 400.
-    /// This is a known limitation of [AsParameters] with value-type defaults.
-    /// </summary>
     [Fact]
-    public async Task ListUsers_OmittedPaginationParams_Returns400DueToDefaultValueLimitation()
+    public async Task ListUsers_InvalidPage_Returns400()
     {
-        var response = await _client.GetAsync("/api/users");
+        var response = await _client.GetAsync("/api/users?page=0");
+
+        response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
+        var body = await response.Content.ReadFromJsonAsync<ValidationProblemDetails>();
+        body.Should().NotBeNull();
+        body!.Errors.Should().ContainKey("Page");
+    }
+
+    [Fact]
+    public async Task ListUsers_InvalidPageSize_Returns400()
+    {
+        var response = await _client.GetAsync("/api/users?pageSize=0");
 
         response.StatusCode.Should().Be(HttpStatusCode.BadRequest);
+        var body = await response.Content.ReadFromJsonAsync<ValidationProblemDetails>();
+        body.Should().NotBeNull();
+        body!.Errors.Should().ContainKey("PageSize");
     }
 }

tests/AxisEndpoints.Example/Features/Users/FindById/FindUserByIdRequest.cs

@@ -32,6 +32,9 @@ public Task<Response<ListUsersResponse>> HandleAsync(
         CancellationToken cancel
     )
     {
+        var page = request.Page ?? 1;
+        var pageSize = request.PageSize ?? 20;
+
         // Dummy data — a real implementation would query a repository.
         var allUsers = new List<UserResponse>
         {
@@ -70,8 +73,8 @@ CancellationToken cancel
             : allUsers;
 
         var items = filtered
-            .Skip((request.Page - 1) * request.PageSize)
-            .Take(request.PageSize)
+            .Skip((page - 1) * pageSize)
+            .Take(pageSize)
             .ToList();
 
         return Task.FromResult(
@@ -80,8 +83,8 @@ CancellationToken cancel
                 Body = new ListUsersResponse
                 {
                     Items = items,
-                    Page = request.Page,
-                    PageSize = request.PageSize,
+                    Page = page,
+                    PageSize = pageSize,
                     TotalCount = filtered.Count,
                 },
             }

tests/AxisEndpoints.Example/Features/Users/List/ListUsersEndpoint.cs

@@ -5,16 +5,19 @@ namespace AxisEndpoints.Example.Features.Users.List;
 
 /// <summary>
 /// Demonstrates [FromQuery] binding with multiple parameters and DataAnnotations on query values.
+/// Page and PageSize are nullable so that [AsParameters] binding correctly distinguishes
+/// "not provided" (null) from an explicit value — property initializers on value types
+/// are ignored by [AsParameters], which would otherwise default to 0.
 /// </summary>
 public class ListUsersRequest
 {
     [FromQuery]
     [Range(1, int.MaxValue, ErrorMessage = "Page must be 1 or greater.")]
-    public int Page { get; init; } = 1;
+    public int? Page { get; init; }
 
     [FromQuery]
     [Range(1, 100, ErrorMessage = "PageSize must be between 1 and 100.")]
-    public int PageSize { get; init; } = 20;
+    public int? PageSize { get; init; }
 
     /// <summary>Optional role filter. Returns all roles when omitted.</summary>
     [FromQuery]