Complete backend coverage and simplify test infrastructure - #96
Merged
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (2)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Summary by CodeRabbit
WalkthroughThe package configuration updates ChangesPackage security pin
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested labels: Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (7 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Pull request overview
This PR significantly refactors the backend test/integration infrastructure and strengthens real-boundary coverage across storage (MinIO), search (Elasticsearch), OCR (CreatePdf.NET), and REST endpoints, while aligning configuration/contracts (notably MinIO endpoint parsing and search DTO removal) and updating security pins.
Changes:
- Replace several mock-heavy/branch tests with real boundary coverage (MinIO/Elasticsearch behavior, endpoint validation/error paths, OCR cancellation/blank PDFs) and simplify shared container fixtures.
- Tighten and standardize configuration contracts (MinIO
EndpointasUriwith origin validation; removeUseSsland obsolete search DTO/types). - Update operational/error handling behavior (ProblemDetails enrichment, exception sanitization, delete/search/storage behaviors) and bump transitive security pins (e.g.,
System.Security.Cryptography.Xml10.0.10).
Reviewed changes
Copilot reviewed 47 out of 48 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Pipeline/Build.csproj | Update/clarify transitive security pins for Pipeline (non-CPM project). |
| PaperlessUI.Angular/src/app/core/api/generated/api-types.ts | Remove obsolete SearchQuery schema/type from generated Angular API types. |
| PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs | Validate MinIO endpoint as an absolute HTTP(S) origin; build MinIO client from Uri. |
| PaperlessServices/Features/OcrProcessing/Infrastructure/Storage/StorageService.cs | Ensure MemoryStream is disposed if MinIO download fails. |
| PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/IPdfExtractor.cs | Update cancellation contract documentation to reflect CreatePdf.NET token propagation. |
| PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/CreatePdfExtractor.cs | Propagate cancellation (don’t convert it into domain failure); refine exception handling. |
| PaperlessServices/Features/OcrProcessing/Application/OcrProcessor.cs | Ensure caller cancellation during download propagates (not converted into error result). |
| PaperlessServices/Configuration/MinioOptions.cs | Change Endpoint to Uri; remove UseSsl option. |
| PaperlessServices.Tests/Unit/StorageServiceTests.cs | Align tests with Uri endpoint; add cancellation propagation test for storage download. |
| PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs | Update MinIO config expectations; add invalid endpoint validation tests. |
| PaperlessServices.Tests/Unit/OcrProcessorTests.cs | Assert stream ownership/disposal; add cancellation propagation test for download stage. |
| PaperlessServices.Tests/Unit/FakeLoggerExtensionsTests.cs | Add unit coverage for log polling (already-met condition, timeout, caller cancellation). |
| PaperlessServices.Tests/Unit/CreatePdfExtractorTests.cs | Replace weak OCR “maybe success” tests with cancellation + disposed-stream failure tests. |
| PaperlessServices.Tests/Integration/WorkerTestBase.cs | Simplify shared fixture composition; improve host teardown and upload helpers. |
| PaperlessServices.Tests/Integration/StorageIntegrationTests.cs | Strengthen storage round-trip assertions (exact bytes + stream position). |
| PaperlessServices.Tests/Integration/OcrIntegrationTests.cs | Add integration coverage for blank-PDF → Ocr.EmptyDocument. |
| PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs | Use AddProblemDetails() + enricher; enforce MinIO origin validation; build client from Uri. |
| PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs | Harden search handler validation/defaulting; minor pagination cursor behavior change. |
| PaperlessREST/Features/DocumentManagement/Infrastructure/Storage/DocumentStorageService.cs | Make delete contract fail-fast (no boolean suppression); simplify remove operation. |
| PaperlessREST/Features/DocumentManagement/Infrastructure/Search/DocumentSearchService.cs | Change search API to return materialized results; add cancellation mapping helper. |
| PaperlessREST/Features/DocumentManagement/Application/DocumentService.cs | Update search/delete contracts; refine storage exception mapping and deletion ordering. |
| PaperlessREST/Features/BatchProcessing/Application/BatchOrchestrator.cs | Simplify logging branches; make ProcessFileAsync private; assume .processing invariant. |
| PaperlessREST/Contracts/DocumentManagement/DocumentDtos.cs | Remove obsolete SearchQuery transport DTO. |
| PaperlessREST/Configuration/MinioOptions.cs | Change Endpoint to Uri; remove UseSsl + helper accessor extension. |
| PaperlessREST/Configuration/Constraints.cs | Remove service-layer search truncation constants. |
| PaperlessREST/API/GlobalExceptionHandler.cs | Centralize/expand validation ProblemDetails shaping; sanitize details in production; enrich PD metadata. |
| PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs | Update wiring/config expectations; validate MinIO endpoint origin rules; adjust ProblemDetails customization assertions. |
| PaperlessREST.Tests/Unit/PathNormalizationTests.cs | Add unit coverage for path normalization and distinct path detection behavior. |
| PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs | Update exception mapping expectations; improve validation-member handling assertions and production sanitization tests. |
| PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs | Remove obsolete/duplicative exception handler test suite (consolidated elsewhere). |
| PaperlessREST.Tests/Unit/DocumentStorageServiceTests.cs | Update MinIO endpoint to Uri; validate delete now propagates failures/cancellation. |
| PaperlessREST.Tests/Unit/DocumentServiceTestBase.cs | Allow injecting storage service for MinIO-backed contract tests. |
| PaperlessREST.Tests/Unit/DocumentServiceContractTests.cs | Replace mocked storage failure mapping tests with MinIO SDK boundary simulations; adjust delete/search contracts. |
| PaperlessREST.Tests/Unit/DocumentSearchServiceTests.cs | Remove obsolete tests for the old search abstraction. |
| PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs | Focus on full-flow file-system tests; add archive-move failure and embedded “processing” filename coverage. |
| PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs | Add collection fixture + deterministic env override/restore; add temp batch root management; robust teardown. |
| PaperlessREST.Tests/Integration/RabbitMqExtensionsTests.cs | Avoid env dependency; ensure DI containers are disposed asynchronously. |
| PaperlessREST.Tests/Integration/GlobalExceptionHandlerMiddlewareTests.cs | Strengthen production sanitization + validation-member behavior coverage. |
| PaperlessREST.Tests/Integration/DocumentEndpointTests.cs | Expand endpoint coverage (pagination defaults, search validation/defaulting, upload constraints, summary, search service contracts). |
| PaperlessREST.Tests/DocumentBuilder.cs | Remove obsolete SearchQueryBuilder. |
| Paperless.TestSupport/TestEnv.cs | Simplify docs and .env.test loading helper. |
| Paperless.TestSupport/FakeLoggerExtensions.cs | Improve timeout vs caller-cancellation semantics and error messaging in log polling helper. |
| Paperless.TestSupport/ContainerFixtureBase.cs | Simplify fixture contract; robust teardown aggregation; improve ES polling cancellation semantics; standardize MinIO endpoint URI formatting. |
| Paperless.TestSupport/AssemblyInfo.cs | Simplify rationale for excluding test-support assembly from coverage. |
| Directory.Packages.props | Bump System.Security.Cryptography.Xml transitive pin to 10.0.10 and clarify pin rationale. |
| AGENTS.md | Update documented coverage metrics/notes to match current state. |
| .env.test | Update MinIO endpoint to include scheme; remove UseSsl. |
| .env.example | Provide concrete MinIO endpoint origin example; remove UseSsl placeholder. |
Suppressed comments (1)
PaperlessREST/Features/DocumentManagement/Infrastructure/Search/DocumentSearchService.cs:58
DeleteAsynclogs success unconditionally after calling the Elasticsearch client. If the client returns an invalid response without throwing, this will log a false success and hide the failure.
await ExecuteElasticsearchAsync(
token => elastic.DeleteAsync(deleteRequest, token),
cancellationToken);
logger.LogInformation("Document {DocumentId} removed from search index", id);
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+32
to
+36
| var response = await ExecuteElasticsearchAsync( | ||
| token => elastic.SearchAsync<DocumentSearchResult>( | ||
| s => s.Indices(elastic.ElasticsearchClientSettings.DefaultIndex) | ||
| .Query(q => q.MultiMatch(mm => mm | ||
| .Query(query) |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 47 out of 48 changed files in this pull request and generated no new comments.
Suppressed comments (1)
PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs:55
- Search query validation currently only checks for null/empty and max length, but the error message references
SearchConstraints.QueryMinLengthand the handler passesqueryto the service. IfQueryMinLengthever changes (or if whitespace-only input should be rejected), this validation won’t match the declared constraints.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
System.Security.Cryptography.Xml10.0.10Why
The previous suite mixed duplicated tests, defensive catch padding, environment races, and coverage-only seams. Several boundary paths were either asserted through mocks or not covered at all, and OCR cancellation was converted into a domain failure instead of propagating naturally from CreatePdf.NET.
This change exercises owned behavior at the real external boundaries, makes resource ownership explicit, and leaves only two unreachable Roslyn sequence-point artifacts outside coverage.
Validation
./build.sh UnitTests— 294/294 passed./build.sh IntegrationTests— 91/91 passed./build.sh Coverage— 385/385 instrumented tests passed./build.sh ReportCoverage --coverage-min-line 0 --coverage-min-branch 0 --coverage-format markdown --coverage-exclude-generated-param true— 99.9% lines (1359/1361), 96.1% branches (317/330)