Migrate document API to ErrorOrX source-generated endpoints - #40
Conversation
Replace the ErrorOr 2.1.1 + ErrorOrAspNetCoreExtensions packages and the
hand-rolled HTTP-mapping glue with ErrorOrX.Generators 5.1.1.
- DocumentEndpoints: manual MapGet/MapPost + Results<> unions become
[Get]/[Post]/[Delete] static handlers returning ErrorOr<T>. [RouteGroup] +
[ApiVersion] reproduce the NewVersionedApi("/api/v{version:apiVersion}/documents")
routes EXACTLY (confirmed in the generated output), so /api/v1/documents is
preserved. Rate-limit/cache become attributes. PDF size/content-type validation
moves inline (ErrorOrX has no endpoint-filter hook). NotFound is declared via
[ReturnsError] on the IDocumentService methods; the upload's transient 503 via
[ProducesError(503)].
- Delete TypedErrorOrAsyncExtensions, ContractViolationException, PdfUploadFilter,
OpenApiMetadataExtensions — their work is now the generator's.
- DocumentService transient storage failures map to Error.Custom(503, {retryAfter})
(ErrorOrX 5.1.1 custom-status + metadata-in-ProblemDetails).
- Adapt to ErrorOrX's API surface: ErrorOr<T>.Errors is IReadOnlyList -> .ToArray().
- Remove obsolete unit tests for the deleted glue; update the storage-mapping tests
to assert the new 503 + retryAfter.
REST + Services + tests build clean (0 warnings); full unit suite green (307).
Route parity verified in generated output; HTTP-level behaviour covered by the
integration suite (Testcontainers).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Migrates the REST API and services from ErrorOr + custom ASP.NET Core glue to ErrorOrX, using its source generator to map attributed endpoint handlers and errors to HTTP responses.
Changes:
- Replaced
ErrorOr/ErrorOrAspNetCoreExtensionspackages withErrorOrX/ErrorOrX.Generatorsand updated central package versions. - Converted document endpoints from manual
MapGroup+ typedResults<>unions to attributed static handlers mapped byMapErrorOrEndpoints(). - Removed custom endpoint filters / OpenAPI metadata / typed
ErrorOr→Results<>conversion utilities, and adjusted tests accordingly.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| Version.props | Swaps ErrorOr* version properties for ErrorOrXVersion. |
| Directory.Packages.props | Replaces central package versions for ErrorOr* with ErrorOrX and generator. |
| PaperlessServices/PaperlessServices.csproj | Updates service project to reference ErrorOrX. |
| PaperlessREST/PaperlessREST.csproj | Switches REST project references from ErrorOr* to ErrorOrX.Generators. |
| PaperlessServices/Features/OcrProcessing/Application/OcrProcessor.cs | Adjusts error returns to match new ErrorOrX expectations. |
| PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs | Registers and maps ErrorOrX generated endpoints. |
| PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs | Rewrites document endpoints as attributed handlers for the ErrorOrX generator. |
| PaperlessREST/Features/DocumentManagement/Presentation/Dto/DTOs.cs | Updates docs to reflect inline upload validation. |
| PaperlessREST/Features/DocumentManagement/Application/DocumentService.cs | Adds [ReturnsError] metadata and maps transient storage failures to HTTP 503 via Error.Custom(...). |
| PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs | Adjusts error returns to match new ErrorOrX expectations. |
| PaperlessREST.Tests/Unit/DocumentServiceStorageMappingTests.cs | Updates assertions for new transient-storage error mapping behavior. |
| PaperlessREST/Host/Extensions/TypedErrorOrAsyncExtensions.cs | Removes custom ErrorOr→typed-Results<> conversion helpers. |
| PaperlessREST/Host/Extensions/OpenApiMetadataExtensions.cs | Removes custom OpenAPI metadata convenience extensions. |
| PaperlessREST/Host/Extensions/ContractViolationException.cs | Removes contract-violation exception infrastructure tied to typed Results<> unions. |
| PaperlessREST/Features/DocumentManagement/Presentation/Filters/PdfUploadFilter.cs | Removes PDF upload endpoint filter (validation moved inline). |
| PaperlessREST.Tests/Unit/* (multiple) | Removes unit tests for deleted glue/filter/endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <PackageReference Include="DotNetEnv"/> | ||
| <PackageReference Include="ErrorOr"/> | ||
| <PackageReference Include="ErrorOrAspNetCoreExtensions"/> | ||
| <PackageReference Include="ErrorOrX.Generators"/> |
| [Post("/")] | ||
| [AcceptedResponse] | ||
| [ProducesError(503, "ServiceUnavailable")] | ||
| [EnableRateLimiting(RateLimitPolicies.WriteOperations)] | ||
| public static async Task<ErrorOr<CreateDocumentResponse>> UploadDocument( | ||
| IFormFile file, | ||
| IDocumentService documentService, | ||
| CancellationToken cancellationToken) |
| [Post("/")] | ||
| [AcceptedResponse] | ||
| [ProducesError(503, "ServiceUnavailable")] | ||
| [EnableRateLimiting(RateLimitPolicies.WriteOperations)] | ||
| public static async Task<ErrorOr<CreateDocumentResponse>> UploadDocument( | ||
| IFormFile file, | ||
| IDocumentService documentService, | ||
| CancellationToken cancellationToken) |
| /// <summary>Deletes a document from Postgres, MinIO, and Elasticsearch. <c>DocumentErrors.NotFound</c> → 404.</summary> | ||
| [Delete("/{id:guid}")] | ||
| [EnableRateLimiting(RateLimitPolicies.WriteOperations)] | ||
| public static Task<ErrorOr<Deleted>> DeleteDocument( | ||
| Guid id, | ||
| IDocumentService documentService, | ||
| CancellationToken cancellationToken) => | ||
| documentService.DeleteDocumentAsync(id, cancellationToken) | ||
| .ToNoContentOr404(); | ||
| documentService.DeleteDocumentAsync(id, cancellationToken); |
| // Transient storage failures → 503 + Retry-After. Error.Custom(503, …) carries the status | ||
| // in Error.Type and the retry hint in metadata; ErrorOrX renders a 503 ProblemDetails with | ||
| // a "retryAfter" extension. (Previously Error.Unexpected → 503 via the hand-rolled glue.) | ||
| TimeoutException => Error.Custom(503, | ||
| "Document.StorageTimeout", | ||
| $"Storage timeout while processing {storagePath}"), | ||
| $"Storage timeout while processing {storagePath}", | ||
| new Dictionary<string, object> { ["retryAfter"] = 30 }), | ||
|
|
||
| HttpRequestException { StatusCode: { } code and >= HttpStatusCode.InternalServerError } => | ||
| Error.Unexpected( | ||
| Error.Custom(503, | ||
| "Document.StorageServerError", | ||
| $"Storage service returned {(int)code} for {storagePath}"), | ||
| $"Storage service returned {(int)code} for {storagePath}", | ||
| new Dictionary<string, object> { ["retryAfter"] = 30 }), | ||
|
|
||
| IOException { InnerException: SocketException } => | ||
| Error.Unexpected( | ||
| Error.Custom(503, | ||
| "Document.StorageConnectionFailed", | ||
| $"Cannot connect to storage service for {storagePath}"), | ||
| $"Cannot connect to storage service for {storagePath}", | ||
| new Dictionary<string, object> { ["retryAfter"] = 30 }), |
| if (file.Length > FileUploadConstraints.MaxFileSizeBytes) | ||
| { | ||
| return Error.Validation("File", | ||
| $"File size cannot exceed {FileUploadConstraints.MaxFileSizeBytes / FileUploadConstraints.BytesPerMegabyte:F0} MB"); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Deletes a document from all storage systems. | ||
| /// </summary> | ||
| /// <param name="id" example="550e8400-e29b-41d4-a716-446655440000">The unique document identifier.</param> | ||
| /// <param name="documentService">The document service for delete operations.</param> | ||
| /// <param name="cancellationToken">Cancellation token for the operation.</param> | ||
| /// <returns>No content on successful deletion.</returns> | ||
| /// <remarks> | ||
| /// Removes document from: PostgreSQL database, MinIO object storage, and Elasticsearch index. | ||
| /// Elasticsearch deletion is best-effort; operation succeeds even if search index removal fails. | ||
| /// </remarks> | ||
| /// <response code="204">Document successfully deleted from all storage systems.</response> | ||
| /// <response code="404"> | ||
| /// Document with the specified ID does not exist. | ||
| /// Domain error: <c>DocumentErrors.NotFound</c> | ||
| /// </response> | ||
| public static Task<Results<NoContent, NotFound>> DeleteDocument( | ||
| var contentType = file.ContentType?.Split(';')[0].Trim() ?? ""; | ||
| if (!contentType.Equals("application/pdf", StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return Error.Validation("File", "Only PDF files are allowed"); | ||
| } |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3459948f76
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| public static async Task<ErrorOr<CreateDocumentResponse>> UploadDocument( | ||
| IFormFile file, |
There was a problem hiding this comment.
When a multipart POST reaches this IFormFile endpoint, ASP.NET Core adds antiforgery validation metadata for form-file binding; this app's middleware pipeline does not call UseAntiforgery() (ServiceCollectionExtensions.ConfigureMiddleware only uses static files/exception/status/http logging/rate limiting/output cache), and the previous manual route explicitly called .DisableAntiforgery(). Without either adding antiforgery middleware or disabling it on the generated endpoint, uploads will throw at runtime instead of reaching the validation/upload logic.
Useful? React with 👍 / 👎.
| <ElasticClientsElasticsearchVersion>9.4.0</ElasticClientsElasticsearchVersion> | ||
| <ErrorOrVersion>2.1.1</ErrorOrVersion> | ||
| <ErrorOrAspNetCoreExtensionsVersion>2.0.1</ErrorOrAspNetCoreExtensionsVersion> | ||
| <ErrorOrXVersion>5.1.1</ErrorOrXVersion> |
There was a problem hiding this comment.
This repo's nuget.config clears package sources down to nuget.org only, but the public ErrorOrX/ErrorOrX.Generators package listing currently tops out at 5.0.0, so centralizing both packages on 5.1.1 makes restore of PaperlessREST and PaperlessServices fail with package-not-found before any build or tests can run. Pin this to a version available on the configured feed, or add the feed that actually contains 5.1.1.
Useful? React with 👍 / 👎.
| services.AddErrorOrEndpoints() | ||
| .WithCamelCase() | ||
| .WithIgnoreNulls(); |
There was a problem hiding this comment.
For pending documents or summaries that have not been generated yet, enabling WithIgnoreNulls() makes the generated endpoints omit fields such as content, processedAt, summary, and summaryGeneratedAt. The previous Minimal API responses used the app-level HttpJsonOptions with only the enum converter configured, so System.Text.Json included null properties by default; this silently changes the wire contract for clients that distinguish a present null from a missing field.
Useful? React with 👍 / 👎.
| [Post("/")] | ||
| [AcceptedResponse] |
There was a problem hiding this comment.
For clients that use the upload response to poll the asynchronous OCR job, this success marker no longer has the route name or { id } values that the previous TypedResults.AcceptedAtRoute(..., nameof(GetDocumentById), ...) call used to populate the Location header. The endpoint can still return 202 with a body, but the documented link to the new document disappears, so clients relying on that header lose their follow-up URL.
Useful? React with 👍 / 👎.
What
Swaps the
ErrorOr+ErrorOrAspNetCoreExtensionspackages and the hand-rolled HTTP-mapping glue forErrorOrX.Generators 5.1.1, and convertsDocumentEndpointsto attribute-driven, source-generated routes.MapGet/MapPost+Results<>unions →[Get]/[Post]/[Delete]statics returningErrorOr<T>, mapped byMapErrorOrEndpoints().[RouteGroup]+[ApiVersion]regenerate the exactNewVersionedApi("/api/v{version:apiVersion}/documents")routes —/api/v1/documentsis unchanged (confirmed in the generated output). Rate-limit/cache are[EnableRateLimiting]/[OutputCache]attributes.ErrorType.Validation;[ProducesError(503)]declares the transient path.DocumentServicestorage failures →Error.Custom(503, metadata: { retryAfter }), rendered as 503 + aretryAfterProblemDetails extension (the ErrorOrX 5.1.1 feature).TypedErrorOrAsyncExtensions,ContractViolationException,PdfUploadFilter,OpenApiMetadataExtensions+ their now-obsolete unit tests.Verification
MapGroup/MapGetoutput.DocumentEndpointTests(Testcontainers) — runs in CI.🤖 Generated with Claude Code