fix(tests): unblock CI on main — drop orphan ListenerExecuteAsyncTests + uppercase XmlContent const#20
fix(tests): unblock CI on main — drop orphan ListenerExecuteAsyncTests + uppercase XmlContent const#20ANcpLua wants to merge 54 commits into
Conversation
# Conflicts: # Paperless.Tests/DataSourceGenerators.cs # Paperless.Tests/DocumentEndpointsTests.cs # Paperless.Tests/ValidationSummaryReporter.cs # Paperless.Tests/obj/Debug/net10.0/Paperless.Tests.AssemblyInfo.cs # Paperless.Tests/obj/Debug/net10.0/Paperless.Tests.AssemblyInfoInputs.cache # Paperless.Tests/obj/Debug/net10.0/Paperless.Tests.assets.cache # Paperless.Tests/obj/Debug/net10.0/Paperless.Tests.csproj.AssemblyReference.cache # Paperless.Tests/obj/rider.project.model.nuget.info # Paperless.Tests/obj/rider.project.restore.info # Paperless.sln.DotSettings.user # PaperlessREST/PaperlessREST.csproj # PaperlessREST/Program.cs # PaperlessREST/bin/Debug/net10.0/PaperlessREST.dll # PaperlessREST/bin/Debug/net10.0/PaperlessREST.pdb # PaperlessREST/bin/Debug/net10.0/PaperlessREST.staticwebassets.endpoints.json # PaperlessREST/obj/Debug/net10.0/PaperlessREST.AssemblyInfo.cs # PaperlessREST/obj/Debug/net10.0/PaperlessREST.AssemblyInfoInputs.cache # PaperlessREST/obj/Debug/net10.0/PaperlessREST.csproj.CoreCompileInputs.cache # PaperlessREST/obj/Debug/net10.0/PaperlessREST.csproj.FileListAbsolute.txt # PaperlessREST/obj/Debug/net10.0/PaperlessREST.dll # PaperlessREST/obj/Debug/net10.0/PaperlessREST.pdb # PaperlessREST/obj/Debug/net10.0/rbcswa.dswa.cache.json # PaperlessREST/obj/Debug/net10.0/ref/PaperlessREST.dll # PaperlessREST/obj/Debug/net10.0/refint/PaperlessREST.dll # PaperlessREST/obj/Debug/net10.0/rjsmcshtml.dswa.cache.json # PaperlessREST/obj/Debug/net10.0/rjsmrazor.dswa.cache.json # PaperlessREST/obj/Debug/net10.0/rpswa.dswa.cache.json # PaperlessREST/obj/Debug/net10.0/staticwebassets.build.endpoints.json # PaperlessREST/obj/Debug/net10.0/staticwebassets.build.json # PaperlessREST/obj/Debug/net10.0/staticwebassets.build.json.cache # PaperlessServices/bin/Debug/net10.0/PaperlessServices.dll # PaperlessServices/bin/Debug/net10.0/PaperlessServices.pdb # PaperlessServices/obj/Debug/net10.0/PaperlessServices.AssemblyInfo.cs # PaperlessServices/obj/Debug/net10.0/PaperlessServices.AssemblyInfoInputs.cache # PaperlessServices/obj/Debug/net10.0/PaperlessServices.dll # PaperlessServices/obj/Debug/net10.0/PaperlessServices.pdb # PaperlessServices/obj/Debug/net10.0/ref/PaperlessServices.dll # PaperlessServices/obj/Debug/net10.0/refint/PaperlessServices.dll
- Created a README.md file with project guidelines. - Updated MinIO client registration to log detailed information during client creation. - Changed document upload method to include error handling and logging for upload failures. - Enhanced document deletion process to handle Elasticsearch deletion gracefully. - Updated various package references to previous versions for compatibility.
# Conflicts: # Paperless.sln.DotSettings.user
…le global enabled
…ture - Add SSE stream support with includeOcrResultStream parameter to RabbitMQ config - Successfully build and run all services with Docker Compose - Database migrations applied, MinIO bucket created, RabbitMQ topology configured - All unit tests pass, solution builds without errors
Migrates the production .NET projects out of ANcpLua/SWEN3.Sprint8 and scaffolds three frontend variants per the agreed layout: Paperless.slnx (modern slnx, replaces Paperless.sln) PaperlessREST/ (ASP.NET Core API) PaperlessServices/ (BackgroundService worker) PaperlessREST.Tests/ (xUnit v3 + Testcontainers) PaperlessServices.Tests/ (xUnit v3 + Testcontainers) PaperlessUI.Blazor/ (Blazor Web App, Server interactivity) PaperlessUI.Angular/ (Angular 20 + pnpm; .esproj wrapper for VS) PaperlessUI.React/ (Vite + React 19 + TS; .esproj wrapper) Pipeline/ (NUKE build) docker/, sample-data/, compose.yaml, .env.test CI is split: one backend job (NUKE + MTP v2 + xUnit v3 + Codecov upload) plus two non-gating frontend build jobs (Angular and React, pnpm). Untracks legacy build outputs and IDE state that the previous skeleton committed by accident (bin/obj/*.cache/*.dll, Paperless.sln.DotSettings.user). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NUKE 10.1's Solution.GetProject(name) doesn't traverse <Folder> wrappers in slnx, so wrapping projects under /src/, /tests/, /build/ broke the test discovery (InvalidOperationException: PaperlessREST.Tests not found). Flatten Paperless.slnx; projects still group cleanly in the IDE by their on-disk paths. Commit pnpm-lock.yaml for both PaperlessUI.Angular and PaperlessUI.React so the actions/setup-node cache (cache: pnpm) actually resolves a dependency path on CI. Verified local builds: Angular bundle ✓, React bundle ✓, NUKE UnitTests ✓. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
pnpm 10 passes everything after `--` literally to the script, so `pnpm run build -- --configuration production` reaches ng as `ng build -- --configuration production` and fails: Option '--' has been specified multiple times. Schema validation failed: Data path "" must NOT have additional properties(). `ng build` already defaults to the production configuration, so drop the redundant flag and the separator. Also dropped the `ng test` opt-in line; tests need a real browser and will be wired through Playwright in a later PR. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
chore: Configure Renovate
Tailored to the actual Paperless.slnx / three-frontends / NUKE setup. Folds in the gotchas this session surfaced: NUKE 10 doesn't traverse slnx <Folder> wrappers, pnpm 10 passes `--` literally to scripts, empty CODECOV_TOKEN breaks the Codecov CLI, BackgroundService startup race on Linux Release, Hangfire NU1107, FakeTextSummarizer for Gemini. Keeps the NUKE Cohesion rules (OCP via INukeBuild, fluent builders, TryDependsOn, typed inputs) and drops the 370 lines of NUKE-source boilerplate that lived in the SWEN3.Sprint8 CLAUDE.md. Adds the Rating-Matrix mapping so the grader (and any future Claude session) can see where each rubric category is implemented. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ckages.props) Lifts every PackageReference Version= out of the four production/test csproj files and into root-level Version.props variables consumed by Directory.Packages.props. Mirrors the TourPlanner reference layout so a single file owns the .NET 10 / EF Core 10 / Testcontainers / MTP version surface. - Pipeline opts out via Pipeline/Directory.Packages.props so the NUKE bootstrap stays self-contained (matches the existing Pipeline/Directory.Build.props parent-isolation pattern). - MartinCostello.Logging.XUnit.v3 unified to 0.7.1 (patch bump; was split 0.7.0 vs 0.7.1 across the two test projects — CPM requires a single version). - Verified with ./build.sh Compile — Restore + Compile both succeed. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
build: adopt central package management (Version.props + Directory.Packages.props)
…out (#3) Replaces the empty Angular 21 scaffold with the layered structure used in TourPlanner-Angular: src/app/ app.component.{ts,html,css,spec.ts} # root with shell app.config.ts # HttpClient + Router + API_BASE_URL app.routes.ts # lazy /documents route core/ api/api-client.service.ts # base HTTP wrapper, baseUrl from token api/generated/ # openapi-typescript output target config/api-base-url.token.ts # InjectionToken<string> layout/ shell/app-shell.component.{ts,html,css} # navbar + <router-outlet> navbar/app-navbar.component.{ts,html,css} # documents / upload / search features/ documents/pages/documents-page.component.{ts,html,css} # stub proxy.conf.json # /api -> http://localhost:5057 Every component uses ChangeDetectionStrategy.OnPush and standalone imports (no NgModules) — same conventions as TourPlanner-Angular. package.json gains generate:openapi / generate:types / generate scripts so PaperlessREST's OpenAPI doc can be flowed into src/app/core/api/generated/ once the backend side lands. pnpm stays as the package manager to keep the existing CI job intact. Build verified: ng build emits the lazy `documents-page-component` chunk (683 B) confirming the lazy-loading pattern wires through. No new deps introduced. Follow-ups: - Wire the actual /documents service + viewmodel against the generated API - Repeat for /upload and /search features - Tailwind + Leaflet adoption (TourPlanner-Angular parity) - React equivalent (PaperlessUI.React -> qyl.dashboard layout) Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces the empty Vite + React 19 scaffold with the qyl.dashboard structure:
src/
App.tsx # Router + Suspense + QueryClientProvider
main.tsx # unchanged
index.css # unchanged (placeholder)
vite-env.d.ts # vite/client types (added)
components/
layout/AppShell.tsx # navbar + <Outlet>
layout/AppNavbar.tsx # documents / upload / search nav
layout/index.ts
hooks/
use-documents.ts # TanStack Query example against /api/documents
lib/
api.ts # fetchJson / postJson wrapper
pages/
DocumentsPage.tsx
UploadPage.tsx
SearchPage.tsx
types/ # openapi-typescript output target
__tests__/
setup.ts # @testing-library/jest-dom
Configuration:
- vite.config.ts: @/* alias to src, /api proxy to PaperlessREST (5057),
manualChunks splits react-vendor + tanstack
- vitest.config.ts: jsdom env, globals, setup from __tests__/setup.ts
- tsconfig.json: single flat config with @/* path alias, "noEmit": true
(replaces tsconfig.{app,node}.json — `tsc --noEmit && vite build` in scripts)
- package.json adds @tanstack/react-query, react-router-dom, jsdom,
openapi-typescript, @testing-library/jest-dom; gains generate:types script
Deleted: src/App.css, src/assets/{hero.png,react.svg,vite.svg} (boilerplate).
Build verified: vite build emits lazy chunks for each page plus react-vendor
(230.89 kB) and tanstack (33.27 kB) split bundles.
Follow-ups:
- Wire generate:openapi to flow PaperlessREST OpenAPI → src/types/api.ts
- Tailwind 4 + Base UI for component primitives (qyl-style)
- Playwright e2e + Dockerfile/nginx.conf.template parity with qyl.dashboard
- ESLint flat config restricting Radix imports
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…boundary) (#5) First slice of the TourPlanner-pattern backend restructure (vertical-slice Features/ -> layered API/BL/Contracts/DAL split). Starts with the safest layer: the transport boundary. New project: Paperless.Contracts/Paperless.Contracts.csproj — plain class library targeting net10.0 with no external dependencies. Moved into Paperless.Contracts: - BatchProcessing/AccessReportDto.cs (was Features/BatchProcessing/Presentation/Dto/) - DocumentManagement/DocumentDtos.cs (was Features/DocumentManagement/Presentation/Dto/DTOs.cs) All response/query DTOs: PaginationQuery, SearchQuery, DocumentDto, CreateDocumentResponse, DocumentSearchResultDto, SummaryDto, PaginatedDocumentsResponse. - Validation/Constraints.cs (was Configuration/Constraints.cs subset) SearchConstraints + PaginationConstraints — the boundary-level ranges the DTOs depend on. Stayed in PaperlessREST: - UploadDocumentRequest (Features/DocumentManagement/Presentation/Dto/DTOs.cs) Carries IFormFile, an ASP.NET Core input-model concern, not a wire DTO. A doc-comment now points readers at the new Paperless.Contracts namespace for the rest of the document transport surface. - FileUploadConstraints, RateLimitPolicies, CachePolicies, SearchServiceConstraints Server-side concerns. SearchConstraints.ServiceQueryMaxLength (used only in DocumentSearchService) was renamed to SearchServiceConstraints.ServiceQueryMaxLength to keep the boundary-level SearchConstraints in Paperless.Contracts free of server-side leakage. Wiring: - Paperless.slnx adds the new project before PaperlessREST. - PaperlessREST + PaperlessREST.Tests gain ProjectReference to Paperless.Contracts. - Both projects' GlobalUsings.cs add `Paperless.Contracts.{BatchProcessing,DocumentManagement,Validation}` so callers see the records under the same short names as before. Verified with ./build.sh Compile (Restore + Compile both succeed). Follow-ups in this series: - Paperless.DAL (extract Features/*/Infrastructure/ — Persistence, Storage, Search) - Paperless.BL (extract Features/*/Application/) - PaperlessREST slims to API/endpoints, or renames to Paperless.API - PaperlessServices threads through the same Contracts/BL/DAL projects Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
) Nothing outside ExceptionHandlerSetup needs the FakeLogCollector directly - the four `_setup.Logger.Collector.GetSnapshot()` call sites reach it via FakeLogger<T>'s own property, not the setup's redundant exposure. Marking it private keeps the test fixture's surface area honest. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replace both PaperlessUI.{Angular,React}.esproj projects with the qyl
hand-rolled MSBuild pattern (qyl/services/qyl.dashboard/qyl.dashboard.esproj),
adapted for pnpm. No more <Project Sdk=...>; custom Restore/Build/Test/Clean
targets shell to pnpm only when RunFrontendOnBuild=true, defaulting to no-op
so CI's dedicated frontend jobs remain the single source of truth.
Removes the entire shim stack that existed only to neutralize the JS SDK:
- <Project Sdk="Microsoft.VisualStudio.JavaScript.Sdk/1.0.5483906">
- <ShouldRunBuildScript>false</ShouldRunBuildScript>
- <ShouldRunNpmInstall>false</ShouldRunNpmInstall>
- <JavaScriptTestRoot>src\</JavaScriptTestRoot> (VS-Windows only)
- <JavaScriptTestFramework>Jasmine|Vitest</...> (VS-Windows only)
- <BuildOutputFolder>$(MSBuildProjectDirectory)\dist\...> (backslashes leaked
literal 'obj\Debug' folders on macOS/Linux every time dotnet build ran)
- <Build Solution="Debug|*"/> opt-in in Paperless.slnx (only needed because
the JS SDK ignored projects that weren't explicitly selected per config)
The bootstrap commit (02ce2b3, today 01:34 UTC+2) hand-typed SDK version
1.0.1641815, which is a Visual Studio Windows internal build number from the
IDE's private NuGet feed and never published to nuget.org — that's why
dotnet build outside VS-Windows failed to resolve it. Dropping the SDK
entirely removes the version-pinning fragility for good.
Both projects still surface their source trees to Rider's solution explorer
via <None Include="src/**/*"/> etc., and 'dotnet build Paperless.slnx'
remains green (0 errors). Real frontend builds keep happening in the pnpm CI
jobs.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Wrapper script hardcodes the 95% line / 75% branch coverage thresholds
(plus markdown summary + generated-code exclusion) and forwards extra args
to ./build.sh Verify. The coverage-push inner loop now reads as a single
muscle-memory command instead of a four-flag invocation.
docs/COVERAGE-HANDOFF.md is updated in all three sections (framing,
fenced prompt block, syntax notes) so a fresh chat picks up the wrapper
without re-deriving the thresholds.
Trade-off acknowledged in syntax notes: the script stays outside NUKE's
discoverability ('./build.sh --plan' won't list it). Once
.github/workflows/ci.yml ratchets to 95/75, CI can call the wrapper and
the threshold lives in exactly one place.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sister to #12 ($ExceptionHandlerSetup.Collector). The only consumer is DisposeAsync inside the same private sealed class, so public access was unnecessary surface. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ve-out (#15) global.json msbuild-sdks pinned to 3.4.29 (analyzer relaxations land: 14 NetAnalyzers + 11 AL style rules go warning → suggestion globally). With that floor lowered, the 25-rule Directory.Build.props carve-out is no longer needed — file deleted instead of trimmed. Surfaced 76 real bug-class errors that the carve-out was hiding. Each fixed at the code level, not re-suppressed: TimeProvider migration (AL0026 + RS0030): - Production: RichProblemDetailsFactory uses TimeProvider.System.GetUtcNow() - Tests: replaced DateTime/DateTimeOffset.UtcNow / DateTime.Now across PaperlessServices.Tests and PaperlessREST.Tests integration + unit suites - PaperlessUI.Blazor Weather demo uses TimeProvider for current date Disposable hygiene (CA2000): - MinioOptions / ElasticsearchOptions: client construction inlined into DI factory so disposal lifetime is owned by the container, not the options-record helper - ReportProcessor: XmlReader wrapped in using - GlobalExceptionHandlerTests: Activity wrapped in using var - Integration test fixtures: nullable backing fields with property accessors (replaces null!/default! initializers that triggered IDE0370) - DocumentEndpointTests: nested try/catch covers pre-Add() window for ByteArrayContent; SuppressMessage justifies the standard HttpClient content-ownership-transfer pattern ValueTask hygiene (CA2012): - TypedErrorOrAsyncExtensions.ToOkOr404 now extends ErrorOr<T> directly (was ValueTask<ErrorOr<T>>), so callers await first and the analyzer sees a direct extension call instead of a ValueTask pipeline - DocumentEndpoints GetDocumentById/GetSummary updated to await-first Cryptographic randomness (CA5394): - Blazor Weather sample: RandomNumberGenerator.GetInt32 instead of Random Naming (IDE1006): - Test method renames from snake_case to PascalCase_When_Then style (matches existing test naming convention in PaperlessREST.Tests/Unit) Suppression hygiene (IDE0370): - Removed redundant null-forgiving operators identified by the analyzer Drive-by fix to Pipeline/Components/ITest.cs: the existing namespace filter `*.Unit.*` / `*.Integration.*` matched zero tests because the project's test namespaces are exactly `*.Tests.Unit` / `*.Tests.Integration` (no sub-namespace). Filter now correctly discovers all 273 unit tests + integration tests. Verification (local, against packed 3.4.29): - `./build.sh Compile --configuration Release`: 0 errors, 0 warnings - `dotnet build Paperless.slnx -c Release`: 0 errors, 0 warnings - `./build.sh UnitTests --configuration Release`: PaperlessServices.Tests.Unit: 59/59 pass PaperlessREST.Tests.Unit: 214/214 pass Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: bump ANcpLua.NET.Sdk to 3.4.29, delete WarningsNotAsErrors carve-out
global.json msbuild-sdks pinned to 3.4.29 (analyzer relaxations land:
14 NetAnalyzers + 11 AL style rules go warning → suggestion globally).
With that floor lowered, the 25-rule Directory.Build.props carve-out is
no longer needed — file deleted instead of trimmed.
Surfaced 76 real bug-class errors that the carve-out was hiding. Each
fixed at the code level, not re-suppressed:
TimeProvider migration (AL0026 + RS0030):
- Production: RichProblemDetailsFactory uses TimeProvider.System.GetUtcNow()
- Tests: replaced DateTime/DateTimeOffset.UtcNow / DateTime.Now across
PaperlessServices.Tests and PaperlessREST.Tests integration + unit suites
- PaperlessUI.Blazor Weather demo uses TimeProvider for current date
Disposable hygiene (CA2000):
- MinioOptions / ElasticsearchOptions: client construction inlined into
DI factory so disposal lifetime is owned by the container, not the
options-record helper
- ReportProcessor: XmlReader wrapped in using
- GlobalExceptionHandlerTests: Activity wrapped in using var
- Integration test fixtures: nullable backing fields with property
accessors (replaces null!/default! initializers that triggered IDE0370)
- DocumentEndpointTests: nested try/catch covers pre-Add() window for
ByteArrayContent; SuppressMessage justifies the standard HttpClient
content-ownership-transfer pattern
ValueTask hygiene (CA2012):
- TypedErrorOrAsyncExtensions.ToOkOr404 now extends ErrorOr<T> directly
(was ValueTask<ErrorOr<T>>), so callers await first and the analyzer
sees a direct extension call instead of a ValueTask pipeline
- DocumentEndpoints GetDocumentById/GetSummary updated to await-first
Cryptographic randomness (CA5394):
- Blazor Weather sample: RandomNumberGenerator.GetInt32 instead of Random
Naming (IDE1006):
- Test method renames from snake_case to PascalCase_When_Then style
(matches existing test naming convention in PaperlessREST.Tests/Unit)
Suppression hygiene (IDE0370):
- Removed redundant null-forgiving operators identified by the analyzer
Drive-by fix to Pipeline/Components/ITest.cs: the existing namespace filter
`*.Unit.*` / `*.Integration.*` matched zero tests because the project's
test namespaces are exactly `*.Tests.Unit` / `*.Tests.Integration` (no
sub-namespace). Filter now correctly discovers all 273 unit tests +
integration tests.
Verification (local, against packed 3.4.29):
- `./build.sh Compile --configuration Release`: 0 errors, 0 warnings
- `dotnet build Paperless.slnx -c Release`: 0 errors, 0 warnings
- `./build.sh UnitTests --configuration Release`:
PaperlessServices.Tests.Unit: 59/59 pass
PaperlessREST.Tests.Unit: 214/214 pass
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(rest): cover RichProblemDetailsFactory and ContractViolationException
Brings two zero-coverage Host/Extensions files to full coverage:
- RichProblemDetailsFactory (0/41 → 41/41): every ErrorType→status mapping,
RFC 7807 extensions for retryAfter/currentState, metadata camelCasing,
the ArgumentOutOfRangeException default, plus ErrorMetadataExtensions
helpers (StorageUnavailable, DocumentLocked, InvalidField, DocumentNotFound).
- ContractViolationException (0/8 → 8/8): every factory (ForNotFoundOnly,
ForValidationOnly, ForNotFoundOrConflict, ForCrudOperation, For),
GetDiagnostics projection, and the one-vs-many error message branches.
Pure unit tests; no new conventions. xUnit v3 + Moq strict elsewhere
in the suite — these files have no collaborators so the mock layer is unused.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(rest): cover TypedErrorOrAsyncExtensions result→typed-result conversions
Hits every public extension (sync ErrorOr<T>, ValueTask, Task overloads)
plus all four ToAcceptedAtRouteOrProblem branches:
- Ok / NoContent / AcceptedAtRoute happy paths.
- ValidationProblem produced from grouped Validation errors.
- 500 ProblemHttpResult from Failure (verifies the urn:paperless:error
kebab-case projection).
- 503 ProblemHttpResult from Unexpected, including the BuildService-
UnavailableExtensions branches: default retryAfter=30, override via
metadata, RetryAfter-only metadata yielding no extra extensions.
- ContractViolationException thrown for unsupported error types in
both the sync and async paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(rest): cover GenAi/Ocr listener ExecuteAsync happy-path lifecycles
Adds GenAiResultListenerExecuteAsyncTests + OcrResultListenerExecuteAsyncTests
covering the BackgroundService ExecuteAsync entry, the consume loop
(yields one event → processes → acks), and the generic-Exception catch
branch on the GenAI listener. Completion is signalled by a TaskCompletion-
Source in the AckAsync mock callback — per CLAUDE.md, never poll a log
snapshot.
The OperationInterruptedException 'no queue' shutdown branch is intentionally
not unit-tested: constructing that exception requires RabbitMQ.Client
internal types not surfaced through the test project's transitive deps.
Documented inline so a future contributor doesn't try the same path again.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: cover error factories, storage-exception mapping, ES throw path
Three small files to chase the long tail toward the coverage gate:
- BatchAndReportErrorsTests: every static factory in ReportErrors and
BatchErrors (Report.FileNotFound/InvalidXml/InvalidSchema/InvalidDate/
InvalidGuid plus Batch.PathRequired/InvalidPath/PathsNotDistinct/
InvalidTimeZone). Single-expression factories, shape-only assertions.
- DocumentServiceStorageMappingTests: exercises every TryMapStorageException
arm in UploadDocumentAsync (TimeoutException → StorageTimeout,
HttpRequestException 5xx → StorageServerError, IOException with
SocketException inner → StorageConnectionFailed) plus the rethrow path
for unrecognized exceptions and the 4xx HttpRequestException case that
falls through the `_ => null` arm.
- SearchIndexServiceThrowingTests: client configured with
ThrowExceptions(true) to hit the catch arm in IndexDocumentAsync that
the sibling tests (which use ThrowExceptions(false)) can't reach.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(rest): cover GenAi listener 'no queue' shutdown branch via reflection
Adds the OperationInterruptedException 'no queue' catch arm by constructing
the exception with GetUninitializedObject and injecting Message via reflection
on Exception._message. The listener's catch only matches on the literal
'no queue' substring in Message, so a real ShutdownEventArgs payload is not
required — and constructing one would need RabbitMQ.Client-internal types
that aren't surfaced through the test project's transitive deps.
Test signals the listener has entered the catch arm by polling for the
warning log, then cancels the stoppingToken to unblock the
Task.Delay(Timeout.Infinite, ct) inside the catch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(rest): cover DocumentService state-transition failure branch
Adds the two cases where ProcessOcrResultAsync exercises the
`transitionResult.IsError` arm in DocumentService that the existing
Pending-state tests never hit:
- AlreadyCompleted: feeding a "Completed" OCR result to an already-
completed document rejects via MarkAsCompleted's non-Pending guard
and logs a state-transition-failed warning.
- AlreadyFailed: same shape on the Failed path.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The org policy on ANcpLua/Paperless only permits actions from ANcpLua-owned repos, GitHub-owned repos, or Marketplace-verified publishers. `pnpm/action-setup@v4` falls outside that allowlist and broke the post-merge CI run on main. Corepack ships with Node and provides the same shim without an external action. Pinned to pnpm@10.30.2 to match PaperlessUI.Angular/package.json's `packageManager` field and the prior workflow's `version: 10` (which floated to latest 10.x). Corepack steps run BEFORE `actions/setup-node@v4` so pnpm is on PATH when setup-node executes its `cache: pnpm` logic. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ensions — superseded by DocumentErrors
…o callers) Verified by grep across PaperlessREST + PaperlessServices that these factories have zero production references outside their own files: DocumentErrors (deleted): - StorageTimeout, StorageServerError, StorageConnectionFailed: only used in DocumentServiceErrorMappingTests via the inline Error.Unexpected(...) calls in TryMapStorageException, not via the factory methods. The string codes are duplicated between the factory and the inline calls; tests assert against the inline codes. - StorageUnavailable, SearchUnavailable, StorageFailed, DeleteFailed, InvalidStateTransition, MessageBrokerUnavailable (both overloads): only referenced in XML doc comments in DocumentEndpoints.cs, never invoked. BatchErrors (deleted entirely): - PathRequired, InvalidPath, PathsNotDistinct, InvalidTimeZone: zero references anywhere. BatchOptions validation is handled inline in ServiceCollectionExtensions via AddOptionsWithValidateOnStart. Per CLAUDE.md: 'delete a file outright when the codebase is healthier without it.' Brings DocumentErrors.cs to 100% (only Used factories remain) and removes BatchErrors as dead code.
…ionExtensions
InitializeAsync had two uncovered branches inside the lock that the existing
SearchIndexIntegrationTests could not exercise because the shared fixture
populates the static `s_initializedIndices` cache before the first test runs.
Drives them via two new integration tests that mint a fresh
`test-{Guid.NewGuid():N}` index per test and construct their own
SearchIndexService (DI gives the fixture-bound singleton):
- Branch (b): two concurrent IndexDocumentAsync calls on a unique index race
the outer ContainsKey check, the semaphore serializes them, and the second
caller hits the inner ContainsKey and short-circuits. Proved by asserting
the "Created Elasticsearch index" log fires exactly once via FakeLogger.
- Branch (c): pre-create the index out-of-band, then assert InitializeAsync
finds Exists == true and emits no create-log. The document is still
indexed successfully against the pre-existing index.
A third test pins the "don't fake it" contract for null createdAt: the
field is absent from the persisted _source rather than substituted with
processedAt.
PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs's no-arg
AddOcrServices() and AddGenAiServices(IConfiguration) were unreachable from
the integration fixture (which uses the IConfiguration overload). A new
unit-test class covers them in isolation, plus the MinIO endpoint
parsing branches (schemeless host:port vs. full URI) and UseSsl=true.
…rrorOr, OpenApi, ServiceCollection ContractViolationException + ContractViolationDiagnostics + ErrorDetail (was 0%): - Every factory: ForNotFoundOnly, ForValidationOnly, ForNotFoundOrConflict, ForCrudOperation, For (custom params). - BuildMessage single-error and three-error branches (verifies the literal "(+ 2 more error(s))" suffix). - GetDiagnostics round-trip including AllErrors ordering and metadata population (present + null). - Record equality holds when array-typed members reference the same instance, and fails when arrays differ (documents synthesized equality's reference semantics on T[]). - with-expressions for both records. - CallerMemberName default for ForNotFoundOnly. TypedErrorOrAsyncExtensions (was 34.6% line / 15.8% branch): - Sync ErrorOr<T>.ToOkOr404: success, NotFound, ContractViolation on non-NotFound. - Sync ErrorOr<Deleted>.ToNoContentOr404: success, NotFound, ContractViolation on non-NotFound. - Task<ErrorOr<T>>/Task<ErrorOr<Deleted>> overloads (delegating to the ValueTask state machines exercise both layers). - ToAcceptedAtRouteOrProblem: success path, Validation grouping by Code, Failure → 500 (kebab-case URN + camelCase metadata extensions), Unexpected → 503 with retryAfter default 30 / metadata override / RetryAfter-key exclusion, unhandled ErrorType → ContractViolation with [Validation, Failure, Unexpected]. - Null / empty / populated Metadata variants for both Failure and Unexpected. - ToKebabCase exercised via assertions on document.-storage-failed, plain.-failure, document.-storage-unavailable, i-pad-or-not, simple (covers leading-lowercase no-dash + internal-uppercase dash branches). OpenApiMetadataExtensions (was 75%): - ProducesNotFound, ProducesConflict, ProducesServiceUnavailable, ProducesGetByIdErrors, ProducesDeleteErrors(canConflict=false/true), ProducesWriteErrors, ProducesDocumentUploadErrors — assert via IEndpointRouteBuilder.DataSources materialization so the Finally callbacks actually run and metadata is observable. - ProducesDeleteErrors returns the same builder instance for chaining. ServiceCollectionExtensions EnsureStorageBucketAsync (was 42.86%): - Bucket exists → MakeBucket never called. - Bucket missing → MakeBucket invoked + LogInformation fires. - Race condition → ArgumentException "already owned" swallowed + LogDebug "already exists" fires. - ArgumentException without "already owned" rethrows. ServiceCollectionExtensions RegisterRecurringJobs: - AddOrUpdate called with JobId, cron, and timezone from BatchOptions. - LogInformation includes JobId/cron/tz. ServiceCollectionExtensions property accessors: - Minio, MinioOpts, BatchOpts, DbFactory return the registered instances. WebApplication.IsDev — Development=true / Production=false. Remaining: MapEndpoints' if(app.IsDev) branch (lines 34–36, 42–43) covers five lines that require a fully-wired WebApplicationFactory in Development environment (Hangfire dashboard, Scalar, OpenAPI). The integration tests spin Test environment by design. Leaving these uncovered for now; they are host-only wiring and exercised in dev runtime.
… delete unreachable XmlException branch
DocumentService.cs gaps closed (100% line/branch):
- UploadDocumentAsync: added test for unknown storage exception type that
TryMapStorageException returns null for; asserts the original exception
propagates uncaught (covers the `throw;` re-raise branch).
- ProcessOcrResultAsync: added two tests for the transitionResult.IsError
short-circuit (lines 148-151): one with an already-Completed document
(Document.CannotComplete), one with an already-Failed document
(Document.CannotFail). Asserts MockBehavior.Strict on the repository to
prove UpdateAsync is NOT called when the state transition fails.
ReportProcessor.cs gaps closed:
- Added ProcessAsync_DateWithTimezone test: '2024-01-15+02:00' satisfies
xs:date schema validation but fails DateOnly.TryParseExact('yyyy-MM-dd'),
exercising the InvalidDate factory at lines 100-103.
- DELETED the `catch (XmlException ex)` branch (lines 125-127) as
unreachable: XmlSerializer.Deserialize wraps both XmlException and
XmlSchemaException as InvalidOperationException before the catch chain
sees them. Verified by writing a test against empty/malformed content and
observing it always hits InvalidOperationException → InvalidSchema.
- DELETED the corresponding ReportErrors.InvalidXml factory (sole caller
was the deleted catch block).
DTOs.cs (UploadDocumentRequest) gap closed:
- New UploadDocumentRequestDtoTests.cs covers the synthesized record
copy-constructor used by 'with' expressions (was the 50% uncovered
half; the File property is exercised by every upload test).
All 350 tests in PaperlessREST.Tests pass.
…ons(true)
The existing SearchIndexServiceTests use ThrowExceptions(false), which
routes transport failures through LogIndexResult's invalid-response
warn path rather than the catch block at lines 57-61. Production wires
the ElasticsearchClient with .ThrowExceptions() (true), so the catch
IS exercised in production but was dead under the current test setup.
Adds IndexDocumentAsync_WhenClientThrows_LogsWarningAndSwallowsException:
constructs a fresh client with ThrowExceptions(true) against the
unreachable host and asserts the catch-block log line ("Failed to
index document...") fires with the underlying TransportException
attached, while IndexDocumentAsync still completes without throwing.
This is the path that protects OCR processing when Elasticsearch is
genuinely unavailable in production.
…mDetails, Hangfire, OpenApi, ApiExplorer lambdas
Drives PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs from 41/46
(89.1%) to 46/46 (100% line + 100% branch) on the CI-aligned dotcov metric.
Adds 9 facts to ServiceCollectionExtensionsTests.cs covering the previously-
uncovered configuration lambda bodies that ASP.NET Core only invokes through
its options pipeline:
- MapEndpoints_WhenIsDev_RegistersDevelopmentOnlyRoutes — IsDev=true branch
(MapOpenApi + Scalar + Hangfire dashboard) read off IEndpointRouteBuilder.DataSources
- MapEndpoints_WhenNotDev_OmitsDevelopmentOnlyRoutes — IsDev=false branch
- MapEndpoints_WhenIsDev_ScalarConfigureCallback_SetsTitleServersAndTheme —
reflects into the Scalar request-delegate's captured configure Action since
Scalar defers options to HTTP-request time
- AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInstanceFromHttpContextWhenNoActivity
— Activity.Current == null branch, asserts fallback to HttpContext.TraceIdentifier
- AddDependencies_ProblemDetailsCustomization_UsesActivityIdWhenAvailable —
Activity.Current != null branch, asserts Activity.Id wins over TraceIdentifier
- AddDependencies_HangfireServerOptions_SetWorkerCountAndServerName —
invokes only the BackgroundJobServerHostedService factory (to avoid resolving
RabbitMQ listeners that would dial localhost), asserts WorkerCount == ProcessorCount
and ServerName == "{MachineName}-{32hex GUID}"
- AddDependencies_OpenApiCreateSchemaReferenceId_ReturnsNullForEnumAndDefaultForOther
— enum → null, POCO → OpenApiOptions.CreateDefaultSchemaReferenceId default
- AddDependencies_OpenApiDocumentTransformer_SetsTitleVersionAndDescription —
extracts DelegateOpenApiDocumentTransformer._documentTransformer and invokes it
on a fresh OpenApiDocument; asserts exact "Paperless OCR API" / "v1" / description
- AddDependencies_ApiExplorerOptions_SetsGroupNameFormatAndSubstituteApiVersionInUrl
— asserts exact "'v'VVV" + SubstituteApiVersionInUrl == true
CreateWiredBuilder helper wires AddDependencies against in-memory IConfiguration
and swaps PostgreSQL JobStorage for Hangfire MemoryStorage so IHostedService
resolution does not require a running database. GetInlineProblemDetailsConfigure
and GetOpenApiConfigure introspect the ServiceCollection for the production
ConfigureNamedOptions<T>.Action so the actual production lambda is exercised
rather than a copy.
No production code touched. UnitTests: +9 facts, suite remains green.
…k (L20-22)
Drives PaperlessREST/Features/EventProcessing/Presentation/GenAiResultListener.cs
from 58/61 (95.1%) to 60/61 (98.4%) on the CI-aligned dotcov metric. The state
machine <ExecuteAsync>d__5 moves from line-rate 88% / branch-rate 50% to
line-rate 96% / branch-rate 100%.
Adds one fact + one helper iterator to ListenerLifecycleTests.cs:
GenAi_ExecuteAsync_TokenCancelledBetweenYields_BodyBreakCheckFires hits the
`if (stoppingToken.IsCancellationRequested) { break; }` block inside the
await-foreach. The pre-existing StoppingTokenCancelled test cannot reach it —
cancellation through the [EnumeratorCancellation] token short-circuits the
iterator before the loop body re-enters, so the body's IsCancellationRequested
check never fires. The new YieldAfterCancel<T> iterator deliberately ignores
[EnumeratorCancellation], yields the second event after the test cancels the
CTS, and forces the body-internal check to trip + break. Asserts:
- DocumentService.UpdateDocumentSummaryAsync for event #2 → Times.Never
- AckAsync → Times.Once
- SseStream.Publish(e2) → Times.Never
- "GenAI Result Listener stopped" Information log present (clean break, not throw)
- No Error-level logs (proves it was a break, not a generic-catch rethrow)
Remaining uncovered line: <ExecuteAsync>d__5 L34 (closing brace of
`catch (OperationInterruptedException) when (...no queue...)`). That brace is
the leave-instruction sequence point for a catch body whose last statement is
`await Task.Delay(Timeout.Infinite, stoppingToken)`. Task.Delay with Infinite
has no normal-return path — every reachable case throws OperationCanceledException
out of the catch — so the leave target at L34 is unreachable Roslyn-emitted
state-machine noise. Documented as phantom; not chased.
No production code touched.
PR #16 added RichProblemDetailsFactoryTests.cs (exercises types deleted in efceded) and BatchAndReportErrorsTests.cs (exercises BatchErrors deleted in 4a1aa98 + ReportErrors.InvalidXml deleted in e97ff02). - Delete RichProblemDetailsFactoryTests.cs entirely — every symbol it references is gone (RichProblemDetailsFactory, ErrorMetadataExtensions). - Trim BatchAndReportErrorsTests.cs to the four surviving ReportErrors factories (FileNotFound, InvalidSchema, InvalidDate, InvalidGuid). The surface is the only test coverage of ReportErrors.* so it stays.
…sts + uppercase XmlContent - ListenerExecuteAsyncTests.cs was a PR #16 leftover that the rebase kept alongside ListenerLifecycleTests.cs. CI Release build flagged its line 254 empty catch as AL0115. Lifecycle tests already cover the same surface with sharper assertions and TaskCompletionSource synchronisation (no Task.Delay polling). Deleting the duplicate. - ReportProcessorTests.cs:554 had a `const string xmlContent` — Release build IDE1006-errors on camelCase constants. Renamed to XmlContent. Verified locally with `./build.sh Compile --configuration Release` (clean).
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis PR removes the ChangesError Factory Consolidation and Listener Lifecycle Test Migration
Comprehensive Test Coverage Expansion
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
✨ Simplify code
Comment |
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| CodeStyle | 3 minor |
🟢 Metrics 136 complexity · 93 duplication
Metric Results Complexity 136 Duplication 93
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
This PR successfully addresses the primary objective of unblocking CI by fixing constant casing and removing orphaned tests. However, it is currently not up to standards according to Codacy, and it introduces significant changes beyond the stated scope.
Of particular concern is the removal of the 'BatchErrors' production class and the introduction of fragile tests that rely on reflection to verify internal fields. Additionally, there is unreachable dead code in the ReportProcessor error handling logic that should be cleaned up. While the test coverage has expanded, the mismatch between the PR description and the actual changes necessitates a thorough verification of the deleted components.
About this PR
- The use of reflection in tests to verify internal fields and private targets (e.g., 'handler', 'DocumentTransformers') creates a high maintenance burden. These tests are fragile and likely to break during framework updates or internal refactoring.
- There is significant scope creep in this PR. The description focuses on lint fixes and orphaned test removal, but the diff reveals extensive removal of production code and new infrastructure tests. Please update the PR description to justify these changes or split them into separate PRs.
1 comment outside of the diff
PaperlessREST/Features/BatchProcessing/Application/ReportErrors.cs
line 28🟡 MEDIUM RISK
The removal of the 'BatchErrors' class and its factories is not mentioned in the PR description. Ensure this class is truly unused across the entire solution and not just in the modified tests.
Test suggestions
- Verify GenAiResultListener logs started/stopped events correctly.
- Verify OcrResultListener logs started/stopped events correctly.
- Verify the 'no queue' exception path in GenAiResultListener logs a warning and waits.
- Verify ReportProcessor handles dates with timezone suffixes that fail DateOnly.TryParseExact.
- Verify the concurrency logic in SearchIndexService.InitializeAsync using unique indices per test.
- Verify ProblemDetails customization correctly populates trace_id and instance from HttpContext.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| // separate `catch (XmlException)` branch is unreachable. | ||
| return ReportErrors.InvalidSchema(ex.Message); | ||
| } | ||
| catch (XmlSchemaException ex) |
There was a problem hiding this comment.
🟡 MEDIUM RISK
This catch block is unreachable. XmlSerializer.Deserialize wraps XmlSchemaException inside an InvalidOperationException, which is already handled at line 125. Removing this block will keep the error handling consistent with the notes regarding XmlException being unreachable.
| IMinioClient client = sp.GetRequiredService<IMinioClient>(); | ||
|
|
||
| // Assert — client constructs without throwing; the schemeless path | ||
| // (`if (!endpoint.Contains("://"))` => true) is executed. |
There was a problem hiding this comment.
⚪ LOW RISK
The documentation comment includes a code fragment that triggers a static analysis warning. You can rephrase this to describe the condition in plain text.
Suggested fix:
| // (`if (!endpoint.Contains("://"))` => true) is executed. | |
| // (the logic for schemeless endpoints) is executed. |
| _timeProvider, | ||
| logger); | ||
|
|
||
| // Act — the unreachable endpoint throws under ThrowExceptions(true); |
There was a problem hiding this comment.
⚪ LOW RISK
This comment contains a code-like fragment that triggers a Sonar S125 warning. Rephrasing it to avoid method call syntax will resolve the issue.
Suggested fix:
| // Act — the unreachable endpoint throws under ThrowExceptions(true); | |
| // Act — the unreachable endpoint throws when ThrowExceptions is enabled; |
| { | ||
| // XmlSerializer.Deserialize wraps both XmlException (well-formedness) | ||
| // and XmlSchemaException (validation) as InvalidOperationException, so a | ||
| // separate `catch (XmlException)` branch is unreachable. |
There was a problem hiding this comment.
⚪ LOW RISK
The linter identifies the code snippet in this comment as commented-out code. Rephrase to describe the logic without using C# keywords.
Suggested fix:
| // separate `catch (XmlException)` branch is unreachable. | |
| // separate branch for handling the underlying XML exception is unreachable. |
| { | ||
| // Arrange — "2024-01-15+02:00" satisfies xs:date (which permits timezone suffix) | ||
| // but DateOnly.TryParseExact("yyyy-MM-dd", ...) rejects it. | ||
| const string XmlContent = """ |
There was a problem hiding this comment.
⚪ LOW RISK
Suggestion: Renamed constant to 'XmlContent' to follow PascalCase naming conventions for constants.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 080669cf08
ℹ️ 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".
| GetResponse<JsonElement> response = await fixture.WaitForDocumentAsync<JsonElement>( | ||
| documentId.ToString(), TestContext.Current.CancellationToken); |
There was a problem hiding this comment.
Query the SUT index when asserting indexed documents
This test creates and uses a unique indexName via BuildSut(...), but the assertion reads through fixture.WaitForDocumentAsync, which (checked in WorkerTestBase) always queries client.ElasticsearchClientSettings.DefaultIndex from the shared fixture, not the per-test indexName. That means the lookup can miss documents that were successfully indexed into the custom index, causing deterministic failures in this scenario.
Useful? React with 👍 / 👎.
| catch (InvalidOperationException ex) | ||
| { | ||
| // XmlSerializer.Deserialize wraps both XmlException (well-formedness) | ||
| // and XmlSchemaException (validation) as InvalidOperationException, so a | ||
| // separate `catch (XmlException)` branch is unreachable. | ||
| return ReportErrors.InvalidSchema(ex.Message); |
There was a problem hiding this comment.
Keep malformed XML mapped to its dedicated error code
Catching InvalidOperationException and always returning ReportErrors.InvalidSchema(...) collapses malformed-XML and schema-validation failures into the same error code. XmlSerializer.Deserialize throws InvalidOperationException for well-formedness errors (with inner XmlException), so malformed input can no longer produce the distinct Report.InvalidXml classification, which is a user-visible contract change for batch error reporting.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@PaperlessREST.Tests/Unit/OpenApiMetadataExtensionsTests.cs`:
- Around line 48-49: The tests in OpenApiMetadataExtensionsTests.cs currently
use containment assertions like codes.Should().Contain(...) which only verify
presence; change each of those to assert exact status-code sets for the tested
endpoints by replacing the Contains assertions on the local variable codes with
an exact-set assertion (for example codes.Should().BeEquivalentTo(expectedCodes)
or codes.Should().Equal(expectedCodes)) and supply the full expected array of
StatusCodes (e.g. new[] { StatusCodes.Status200OK, StatusCodes.Status404NotFound
} as appropriate) so each test (the occurrences referencing codes and
StatusCodes.Status404NotFound and the other status constants at lines noted)
verifies exclusivity rather than just presence.
In `@PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs`:
- Around line 344-351: Replace the brittle reflection-based inspection that
reads private fields (handlerField, handlerDelegate, scalarClosure,
configureField, productionConfigure) with HTTP integration tests that exercise
the actual mapped endpoints (e.g., GET /docs/{documentName}) and assert
observable behaviors caused by the configuration (responses, headers, status
codes, middleware effects), and remove similar reflection usages found around
the other locations (the blocks referenced at lines 466–468 and 528–539); use
the test host or WebApplicationFactory to send real requests and validate how
ScalarOptions/HttpContext configuration manifests in the HTTP response rather
than probing private implementation details.
- Around line 369-379: The helper that scans service registrations is returning
the first ConfigureNamedOptions<ProblemDetailsOptions> action without verifying
the registration name; update the lookup in the method that iterates
ServiceDescriptor (the code handling IConfigureOptions<ProblemDetailsOptions>
and ConfigureNamedOptions<ProblemDetailsOptions>) to also check named.Name
before returning: require named.Name == Options.DefaultName for the
ProblemDetails helper (GetInlineProblemDetailsConfigure) and require named.Name
== "v1" for the OpenAPI helper (GetOpenApiConfigure), so only the correctly
named registration's named.Action is returned.
In `@PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs`:
- Around line 125-130: Update the comment above the exception handling to
mention that XmlSchemaException can be thrown both by XmlSerializer.Deserialize
(wrapped as InvalidOperationException) and directly by schemas.Compile() during
lazy schema initialization; then change the InvalidOperationException and the
XmlSchemaException handlers so they preserve the most specific error text by
using the inner exception message when present (e.g. use
ex.InnerException?.Message ?? ex.Message) and pass that string into
ReportErrors.InvalidSchema so actionable details are retained; locate the catch
block labeled "catch (InvalidOperationException ex)" and the code path that
calls "schemas.Compile()" to adjust the XmlSchemaException handler accordingly.
In `@PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs`:
- Around line 117-118: The test is asserting against the fixture's default index
because WaitForDocumentAsync in WorkerTestBase uses
client.ElasticsearchClientSettings.DefaultIndex; update the test to assert
against the unique per-test index by passing indexName into the assertion (e.g.,
call fixture.WaitForDocumentAsync(documentId.ToString(),
TestContext.Current.CancellationToken, indexName) or similar), or alternatively
bypass the helper and query the ElasticClient directly specifying indexName when
calling GetAsync/GetResponse; adjust WaitForDocumentAsync
signature/implementation if needed to accept an explicit index parameter so the
test verifies the correct index used by the SUT.
In `@PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs`:
- Around line 210-213: The assertion currently checks
ServiceType.FullName.Contains("GeminiOptions") which is too broad; update the
predicate inside the services.Should().Contain(...) call to assert the concrete
registration type explicitly (for example, replace the substring check with
d.ServiceType == typeof(IOptions<GeminiOptions>) or d.ServiceType ==
typeof(IConfigureOptions<GeminiOptions>)) so the test verifies the exact
contract registered by AddPaperlessGenAI while keeping the existing assertion
message.
- Around line 120-141: Extract the MinIO endpoint parsing logic currently
embedded in AddOcrServices into a pure helper method (e.g., ParseMinioEndpoint
or NormalizeMinioEndpoint) that takes the raw config string and returns a
deterministic tuple/object with scheme, host, and port; update AddOcrServices to
call that helper and use its outputs. Replace the existing tests
AddOcrServices_WithSchemelessEndpoint_PrefixesHttp and the two related SSL/port
tests so they call the new helper directly and assert exact values for scheme,
host, and port (instead of only client.Should().NotBeNull()), ensuring the
schemeless, explicit-http/https, and host:port branches are covered. Ensure the
helper lives in a testable internal/class-visible location and name it so it can
be referenced from unit tests (e.g., ParseMinioEndpoint or
NormalizeMinioEndpoint used by AddOcrServices).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 11b7a0f0-ecd8-4082-abac-c6528cd46471
📒 Files selected for processing (18)
PaperlessREST.Tests/Unit/BatchAndReportErrorsTests.csPaperlessREST.Tests/Unit/ContractViolationExceptionTests.csPaperlessREST.Tests/Unit/DocumentServiceTests.csPaperlessREST.Tests/Unit/ListenerExecuteAsyncTests.csPaperlessREST.Tests/Unit/ListenerLifecycleTests.csPaperlessREST.Tests/Unit/OpenApiMetadataExtensionsTests.csPaperlessREST.Tests/Unit/ReportProcessorTests.csPaperlessREST.Tests/Unit/RichProblemDetailsFactoryTests.csPaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.csPaperlessREST.Tests/Unit/TypedErrorOrAsyncExtensionsTests.csPaperlessREST.Tests/Unit/UploadDocumentRequestDtoTests.csPaperlessREST/Features/BatchProcessing/Application/ReportErrors.csPaperlessREST/Features/BatchProcessing/Application/ReportProcessor.csPaperlessREST/Features/DocumentManagement/Application/DocumentErrors.csPaperlessREST/Host/Extensions/RichProblemDetailsFactory.csPaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.csPaperlessServices.Tests/Unit/SearchIndexServiceTests.csPaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs
💤 Files with no reviewable changes (5)
- PaperlessREST.Tests/Unit/ListenerExecuteAsyncTests.cs
- PaperlessREST/Host/Extensions/RichProblemDetailsFactory.cs
- PaperlessREST/Features/BatchProcessing/Application/ReportErrors.cs
- PaperlessREST.Tests/Unit/RichProblemDetailsFactoryTests.cs
- PaperlessREST/Features/DocumentManagement/Application/DocumentErrors.cs
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: copilot-pull-request-reviewer
- GitHub Check: Codacy Static Code Analysis
🧰 Additional context used
📓 Path-based instructions (3)
**/*.cs
📄 CodeRabbit inference engine (.editorconfig)
**/*.cs: Do not qualify event, field, method, and property access with 'this.' in C# code
Use predefined types (like int, string) instead of BCL types (like Int32, String) in C#
Always use parentheses for clarity in arithmetic and relational binary operators in C#
Require explicit accessibility modifiers for non-interface members in C#
Prefer coalesce expressions, collection initializers, null propagation, and object initializers in C#
Prefer auto-properties and compound assignments in C#
Prefer conditional expressions over assignment and return statements in C#
Mark fields as readonly when possible in C#
Treat all unused parameters as code quality issues in C#
Use explicit types instead of 'var' in C# unless type is apparent
Prefer expression-bodied members for accessors, indexers, lambdas, methods, and properties in C#
Use pattern matching instead of 'as' with null checks and 'is' with cast checks in C#
Prefer switch expressions over traditional switch statements in C#
Use conditional delegate calls in C# to avoid null reference exceptions
Prefer static local functions over instance local functions in C#
Use modifier order: public, private, protected, internal, file, static, extern, new, virtual, abstract, sealed, override, readonly, unsafe, required, volatile, async in C#
Always use braces for code blocks in C#
Prefer file-scoped namespaces in C#
Prefer implicit object creation when type is apparent in C#
Prefer index and range operators in C#
Place 'using' directives outside of namespace declarations in C#
Always place opening brace on new line before catch, else, finally, and members in C#
Indent block contents and case contents in C#
Add space after comma, before and after binary operators, and after colon in inheritance clause in C#
Do not add space after dot, cast, or before open square brackets in C#
Name interfaces with 'I' prefix using PascalCase in C#
Name types (classes, structs, enums) using PascalCase in C#
Name non-field members (properties, events...
Files:
PaperlessREST.Tests/Unit/UploadDocumentRequestDtoTests.csPaperlessREST.Tests/Unit/ReportProcessorTests.csPaperlessREST.Tests/Unit/DocumentServiceTests.csPaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.csPaperlessREST.Tests/Unit/OpenApiMetadataExtensionsTests.csPaperlessServices.Tests/Unit/SearchIndexServiceTests.csPaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.csPaperlessREST/Features/BatchProcessing/Application/ReportProcessor.csPaperlessREST.Tests/Unit/ListenerLifecycleTests.csPaperlessREST.Tests/Unit/ContractViolationExceptionTests.csPaperlessREST.Tests/Unit/TypedErrorOrAsyncExtensionsTests.csPaperlessREST.Tests/Unit/BatchAndReportErrorsTests.csPaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs
PaperlessServices.Tests/Integration/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
Mock
ITextSummarizerin integration tests withFakeTextSummarizer; do not use the real Gemini API even with placeholder keys
Files:
PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs
PaperlessREST/Features/**/*.cs
📄 CodeRabbit inference engine (CLAUDE.md)
PaperlessREST/Features/**/*.cs: Use vertical-slice Feature folder organization with co-located endpoints, services, and validators per feature
Use DataAnnotations and FluentValidation at the boundary (endpoint/command level); validate inputs before passing to services
Files:
PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs
🪛 GitHub Check: Codacy Static Code Analysis
PaperlessServices.Tests/Unit/SearchIndexServiceTests.cs
[notice] 328-328: PaperlessServices.Tests/Unit/SearchIndexServiceTests.cs#L328
Remove this commented out code.
PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs
[notice] 139-139: PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs#L139
Remove this commented out code.
PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs
[notice] 129-129: PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs#L129
Remove this commented out code.
🔇 Additional comments (4)
PaperlessREST.Tests/Unit/ContractViolationExceptionTests.cs (1)
7-223: No actionable defects identified in the changed test coverage.PaperlessREST.Tests/Unit/DocumentServiceTests.cs (1)
602-684: No actionable defects identified in the changed test coverage.PaperlessREST.Tests/Unit/ListenerLifecycleTests.cs (1)
1-578: LGTM!PaperlessREST.Tests/Unit/BatchAndReportErrorsTests.cs (1)
6-8: No actionable architecture/correctness concern in this doc-comment update.
| codes.Should().Contain(StatusCodes.Status404NotFound); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | ⚡ Quick win
Tighten assertions to exact status-code sets to prevent silent OpenAPI contract drift.
At Line 48 and repeated through Line 125, these checks validate presence but not exclusivity. If an extension starts emitting unintended extra status codes, these tests will still pass. Assert exact sets per method.
Proposed update pattern
- codes.Should().Contain(StatusCodes.Status404NotFound);
+ codes.Should().BeEquivalentTo([StatusCodes.Status404NotFound]);
- codes.Should().Contain(StatusCodes.Status400BadRequest);
- codes.Should().Contain(StatusCodes.Status500InternalServerError);
- codes.Should().Contain(StatusCodes.Status503ServiceUnavailable);
+ codes.Should().BeEquivalentTo(
+ [
+ StatusCodes.Status400BadRequest,
+ StatusCodes.Status500InternalServerError,
+ StatusCodes.Status503ServiceUnavailable
+ ]);Also applies to: 58-59, 68-69, 78-79, 88-90, 99-101, 110-113, 122-125
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessREST.Tests/Unit/OpenApiMetadataExtensionsTests.cs` around lines 48 -
49, The tests in OpenApiMetadataExtensionsTests.cs currently use containment
assertions like codes.Should().Contain(...) which only verify presence; change
each of those to assert exact status-code sets for the tested endpoints by
replacing the Contains assertions on the local variable codes with an exact-set
assertion (for example codes.Should().BeEquivalentTo(expectedCodes) or
codes.Should().Equal(expectedCodes)) and supply the full expected array of
StatusCodes (e.g. new[] { StatusCodes.Status200OK, StatusCodes.Status404NotFound
} as appropriate) so each test (the occurrences referencing codes and
StatusCodes.Status404NotFound and the other status constants at lines noted)
verifies exclusivity rather than just presence.
| FieldInfo handlerField = generatedTarget.GetType() | ||
| .GetField("handler", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance)!; | ||
| Delegate handlerDelegate = (Delegate)handlerField.GetValue(generatedTarget)!; | ||
| object scalarClosure = handlerDelegate.Target!; | ||
| FieldInfo configureField = scalarClosure.GetType() | ||
| .GetField("configureOptions", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance)!; | ||
| Action<ScalarOptions, HttpContext> productionConfigure = | ||
| (Action<ScalarOptions, HttpContext>)configureField.GetValue(scalarClosure)!; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's explore the repository structure and find the test file
find . -type f -name "ServiceCollectionExtensionsTests.cs" | head -5Repository: ANcpLua/Paperless
Length of output: 191
🏁 Script executed:
# Get file size and then read the specific lines mentioned
wc -l PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.csRepository: ANcpLua/Paperless
Length of output: 126
🏁 Script executed:
# Read lines 344-351 to verify the snippet
sed -n '344,351p' PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.csRepository: ANcpLua/Paperless
Length of output: 637
🏁 Script executed:
# Also check the lines mentioned in "Also applies to": 466-468, 528-539
sed -n '466,468p' PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs
echo "---"
sed -n '528,539p' PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.csRepository: ANcpLua/Paperless
Length of output: 1108
🏁 Script executed:
# Get context around the snippets to understand what's being tested
sed -n '330,360p' PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.csRepository: ANcpLua/Paperless
Length of output: 1570
🏁 Script executed:
# Search for usage patterns of these types to understand available public APIs
rg -A 3 "ScalarOptions" PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs | head -30Repository: ANcpLua/Paperless
Length of output: 314
🏁 Script executed:
# Check if there's any public method to access or retrieve the ScalarOptions configuration
rg "ScalarOptions|BackgroundJobServerOptions|OpenApiOptions" . --type cs | grep -v "BindingFlags" | head -20Repository: ANcpLua/Paperless
Length of output: 2195
🏁 Script executed:
# Look for the actual extension method being tested to understand what public surface exists
rg "MapScalarApiReference|MapEndpoints" . --type cs -B 2 -A 2 | head -40Repository: ANcpLua/Paperless
Length of output: 3335
🏁 Script executed:
# Check project dependencies to confirm third-party libraries
fd -e csproj | head -5Repository: ANcpLua/Paperless
Length of output: 288
🏁 Script executed:
# Examine the csproj to see which packages are referenced
grep -i "scalar\|hangfire" ./PaperlessREST/*.csprojRepository: ANcpLua/Paperless
Length of output: 256
Replace private field reflection with HTTP integration testing.
These tests extract configurations via reflection on private fields (handler, configureOptions, _options, _documentTransformer) from third-party libraries (Scalar, Hangfire). Those fields have no public API contract and will break silently on package updates. While lazy binding prevents accessing these options at map time, actual HTTP requests to the mapped endpoints (e.g., GET /docs/{documentName}) would verify the configuration without depending on implementation details.
Current approach (lines 344–351, also 466–468, 528–539)
FieldInfo handlerField = generatedTarget.GetType()
.GetField("handler", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance)!;
Delegate handlerDelegate = (Delegate)handlerField.GetValue(generatedTarget)!;
object scalarClosure = handlerDelegate.Target!;
FieldInfo configureField = scalarClosure.GetType()
.GetField("configureOptions", BindingFlags.NonPublic | BindingFlags.Public | BindingFlags.Instance)!;
Action<ScalarOptions, HttpContext> productionConfigure =
(Action<ScalarOptions, HttpContext>)configureField.GetValue(scalarClosure)!;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs` around lines
344 - 351, Replace the brittle reflection-based inspection that reads private
fields (handlerField, handlerDelegate, scalarClosure, configureField,
productionConfigure) with HTTP integration tests that exercise the actual mapped
endpoints (e.g., GET /docs/{documentName}) and assert observable behaviors
caused by the configuration (responses, headers, status codes, middleware
effects), and remove similar reflection usages found around the other locations
(the blocks referenced at lines 466–468 and 528–539); use the test host or
WebApplicationFactory to send real requests and validate how
ScalarOptions/HttpContext configuration manifests in the HTTP response rather
than probing private implementation details.
| foreach (ServiceDescriptor d in services) | ||
| { | ||
| if (d.ServiceType != typeof(IConfigureOptions<ProblemDetailsOptions>) || | ||
| d.ImplementationInstance is not ConfigureNamedOptions<ProblemDetailsOptions> named || | ||
| named.Action is null) | ||
| { | ||
| continue; | ||
| } | ||
|
|
||
| return named.Action; | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
wc -l "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs"Repository: ANcpLua/Paperless
Length of output: 126
🏁 Script executed:
cat -n "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" | sed -n '350,410p'Repository: ANcpLua/Paperless
Length of output: 2562
🏁 Script executed:
cat -n "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" | sed -n '465,510p'Repository: ANcpLua/Paperless
Length of output: 2229
🏁 Script executed:
# Search for Options.DefaultName usage and "v1" references
rg "DefaultName|\"v1\"" --type cs -B 2 -A 2 | head -60Repository: ANcpLua/Paperless
Length of output: 2302
🏁 Script executed:
# Look at the AddDependencies extension method to understand the registrations
rg -A 20 "private static WebApplicationBuilder AddDependencies" "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs"Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Check what's being registered for ProblemDetails and OpenApi
rg "AddProblemDetails|AddOpenApi" "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" -B 3 -A 3Repository: ANcpLua/Paperless
Length of output: 737
🏁 Script executed:
# Look for the ServiceCollectionExtensions implementation to understand actual registrations
fd -t f -name "*ServiceCollectionExtensions.cs" | grep -v TestsRepository: ANcpLua/Paperless
Length of output: 292
🏁 Script executed:
fd "ServiceCollectionExtensions.cs" --type f | grep -v TestsRepository: ANcpLua/Paperless
Length of output: 187
🏁 Script executed:
# Look at the actual service registration logic
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | head -150Repository: ANcpLua/Paperless
Length of output: 5004
🏁 Script executed:
# Find AddProblemDetails and AddOpenApi registrations
rg "AddProblemDetails|AddOpenApi" "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 328
🏁 Script executed:
# Check for ConfigureNamedOptions usage in the codebase
rg "ConfigureNamedOptions" --type cs -B 1 -A 1Repository: ANcpLua/Paperless
Length of output: 1564
🏁 Script executed:
# Let's look for where Options.DefaultName or specific named configurations are used
rg "DefaultName|Configure.*\"v1\"" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 713
🏁 Script executed:
# Check the ConfigureOptions<ProblemDetailsEnricher> to understand naming
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | sed -n '150,250p'Repository: ANcpLua/Paperless
Length of output: 3415
🏁 Script executed:
# Check what the default name is for ProblemDetails configurations
rg "Options.DefaultName" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Search for how AddProblemDetails and AddOpenApi work with named options
web_search "AddProblemDetails named options default name C#"Repository: ANcpLua/Paperless
Length of output: 112
🏁 Script executed:
# Look for all places where IConfigureOptions<ProblemDetailsOptions> or IConfigureOptions<OpenApiOptions> might be registered
rg "IConfigureOptions<ProblemDetailsOptions>|IConfigureOptions<OpenApiOptions>" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 2235
🏁 Script executed:
# Check if there are any multiple registrations in the builder setup
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | sed -n '200,300p'Repository: ANcpLua/Paperless
Length of output: 3261
🏁 Script executed:
# Check what Options.DefaultName is - look in test for hints
rg "Options.DefaultName" --type cs -B 3 -A 3Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Check the actual registrations more carefully - look at the full AddApiLayer and AddCrossCuttingConcerns
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | sed -n '110,170p'Repository: ANcpLua/Paperless
Length of output: 2314
🏁 Script executed:
# Look at the complete AddApiLayer section to see all registrations
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | sed -n '285,340p'Repository: ANcpLua/Paperless
Length of output: 2128
🏁 Script executed:
# Look at the complete AddApiLayer to see OpenApi registration
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | sed -n '329,365p'Repository: ANcpLua/Paperless
Length of output: 989
🏁 Script executed:
# Check if ProblemDetailsEnricher is a different type that wouldn't match ConfigureNamedOptions
cat "PaperlessREST/API/GlobalExceptionHandler.cs" | head -50Repository: ANcpLua/Paperless
Length of output: 2094
🏁 Script executed:
# Let's understand what Options.DefaultName is - search in test comments and understand the issue
cat "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" | sed -n '364,382p'Repository: ANcpLua/Paperless
Length of output: 797
🏁 Script executed:
# Check if there are actually multiple registrations that could cause issues
rg "AddProblemDetails|AddOpenApi|ConfigureOptions.*ProblemDetails|ConfigureOptions.*OpenApi" "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 1121
🏁 Script executed:
# Look at the full test setup to understand if multiple registrations can happen
cat "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" | sed -n '1,100p'Repository: ANcpLua/Paperless
Length of output: 3739
🏁 Script executed:
# Search for Options.DefaultName or string.Empty references to understand the default name
rg "Options\.DefaultName|OptionsName.*=.*\"\"" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Let's check if ConfigureNamedOptions has a Name property and what it contains
rg "ConfigureNamedOptions.*Name" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 564
🏁 Script executed:
# Look for documentation or tests that show what names are used for ProblemDetails and OpenApi
rg "\"v1\"|DefaultName" "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" -B 3 -A 3Repository: ANcpLua/Paperless
Length of output: 928
🏁 Script executed:
# Check if there are other IConfigureOptions registrations in the service collection to understand the risk
rg "IConfigureOptions" --type cs -B 1 -A 1 | head -80Repository: ANcpLua/Paperless
Length of output: 1424
🏁 Script executed:
# Check what the actual name would be for ProblemDetailsOptions - look for Options.DefaultName
python3 << 'EOF'
# Options.DefaultName is a constant in Microsoft.Extensions.Options
# It's typically used to identify unnamed configurations
# Let's verify if there are any other registrations that might conflict
print("For ProblemDetailsOptions:")
print("- AddProblemDetails registers with Options.DefaultName (typically empty/null)")
print("- ConfigureOptions<ProblemDetailsEnricher>() also uses Options.DefaultName")
print("")
print("For OpenApiOptions:")
print("- AddOpenApi registers with explicit name 'v1'")
print("")
print("Risk: If multiple ConfigureNamedOptions are registered with same name,")
print("picking the first one without checking the name is unsafe.")
EOFRepository: ANcpLua/Paperless
Length of output: 440
🏁 Script executed:
# Verify: check if ConfigureNamedOptions has a Name property by looking at usage patterns
rg "named\.Name" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Look at the actual test to see if it would catch incorrect filtering
cat "PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs" | sed -n '384,420p'Repository: ANcpLua/Paperless
Length of output: 1152
🏁 Script executed:
# Check if ConfigureNamedOptions is from Microsoft.Extensions.Options and has Name property
rg "class ConfigureNamedOptions|\.Name" --type cs | grep -A 5 ConfigureNamedOptions | head -20Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Look at the comment in AddCrossCuttingConcerns to see if there are multiple problem details registrations
cat "PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs" | sed -n '126,145p'Repository: ANcpLua/Paperless
Length of output: 889
🏁 Script executed:
# Check if ProblemDetailsEnricher implements IConfigureOptions or if it would be picked up
cat "PaperlessREST/API/GlobalExceptionHandler.cs" | sed -n '44,80p'Repository: ANcpLua/Paperless
Length of output: 1178
Add name filtering to option lookup helpers.
The comment in GetOpenApiConfigure explicitly states to find the registration "whose Name == 'v1'", yet the code doesn't check named.Name. Similarly, GetInlineProblemDetailsConfigure should verify the name matches the expected option configuration. If additional named registrations are added, these helpers will extract the wrong lambda and tests will pass despite validating incorrect behavior.
Filter by named.Name (check for "v1" for OpenApi and Options.DefaultName for ProblemDetails) before returning.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs` around lines
369 - 379, The helper that scans service registrations is returning the first
ConfigureNamedOptions<ProblemDetailsOptions> action without verifying the
registration name; update the lookup in the method that iterates
ServiceDescriptor (the code handling IConfigureOptions<ProblemDetailsOptions>
and ConfigureNamedOptions<ProblemDetailsOptions>) to also check named.Name
before returning: require named.Name == Options.DefaultName for the
ProblemDetails helper (GetInlineProblemDetailsConfigure) and require named.Name
== "v1" for the OpenAPI helper (GetOpenApiConfigure), so only the correctly
named registration's named.Action is returned.
| catch (InvalidOperationException ex) | ||
| { | ||
| // XmlSerializer.Deserialize wraps both XmlException (well-formedness) | ||
| // and XmlSchemaException (validation) as InvalidOperationException, so a | ||
| // separate `catch (XmlException)` branch is unreachable. | ||
| return ReportErrors.InvalidSchema(ex.Message); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
In .NET, does XmlSerializer.Deserialize throw InvalidOperationException with XmlException/XmlSchemaException in InnerException, and is the outer Message typically generic?
💡 Result:
Yes, in.NET, the XmlSerializer.Deserialize method typically throws a System.InvalidOperationException when a deserialization error occurs [1][2][3]. This outer exception is often generic, frequently carrying the message "There is an error in XML document (line, position)" [1][4][2]. To determine the actual cause of the failure, you must inspect the InnerException property of the caught InvalidOperationException [1][2][3]. This inner exception contains the specific error details, which may be an XmlException (e.g., for malformed XML) or other relevant exception types (such as InvalidCastException or FormatException) depending on the nature of the deserialization issue [5][2].
Citations:
- 1: https://stackoverflow.com/questions/1326039/what-types-of-exceptions-can-the-xmlserializer-deserialize-method-throw
- 2: https://learn.microsoft.com/en-us/answers/questions/598839/how-to-debug-xmlserializer-deserialize-error-there
- 3: https://learn.microsoft.com/en-us/dotnet/api/system.xml.serialization.xmlserializer.deserialize?view=net-10.0
- 4: XmlSerializer.Deserialize fails when element contains XML content dotnet/runtime#102737
- 5: https://stackoverflow.com/questions/50278104/unable-to-deserialize-xml-document-inner-exception-arrayofstring-xmlns-wa
🏁 Script executed:
# First, locate the file and examine the context around lines 125-130
fd -t f "ReportProcessor.cs" | head -5Repository: ANcpLua/Paperless
Length of output: 131
🏁 Script executed:
# Once found, examine the file content around the target lines
if [ -f "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" ]; then
wc -l "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs"
fiRepository: ANcpLua/Paperless
Length of output: 135
🏁 Script executed:
# Look at the specific code section
if [ -f "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" ]; then
sed -n '120,135p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | cat -n
fiRepository: ANcpLua/Paperless
Length of output: 659
🏁 Script executed:
# Check for similar exception handling patterns in the codebase
rg "InnerException" --type cs -A 2 -B 2 | head -40Repository: ANcpLua/Paperless
Length of output: 3634
🏁 Script executed:
# Check ReportErrors.InvalidSchema signature to understand the parameter type
rg "InvalidSchema" --type cs -A 2 | head -30Repository: ANcpLua/Paperless
Length of output: 2326
🏁 Script executed:
# Get full method to see all exception sources
if [ -f "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" ]; then
sed -n '100,137p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | cat -n
fiRepository: ANcpLua/Paperless
Length of output: 1403
🏁 Script executed:
# Search for the method signature to understand the full context
rg "ParseAndValidateXmlAsync|public.*async" PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs -A 30 | head -50Repository: ANcpLua/Paperless
Length of output: 1611
🏁 Script executed:
# Check what exactly is being used for deserialization - look for XmlReader usage
sed -n '100,137p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs"Repository: ANcpLua/Paperless
Length of output: 1137
🏁 Script executed:
# Search for XmlReader.ReadAsync and related methods to understand the actual exception flow
rg "XmlReader|ReadAsync|Deserialize" PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs -B 5 -A 3Repository: ANcpLua/Paperless
Length of output: 1278
🏁 Script executed:
# Let me see the complete validation handler setup
sed -n '65,95p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | cat -nRepository: ANcpLua/Paperless
Length of output: 1118
🏁 Script executed:
# Check if XmlReader.Create or Schemas.Add can throw XmlSchemaException
# Look at the complete method initialization
sed -n '45,70p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | cat -nRepository: ANcpLua/Paperless
Length of output: 1036
🏁 Script executed:
# Let me check the actual Schemas property initialization
sed -n '1,50p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | cat -nRepository: ANcpLua/Paperless
Length of output: 2032
🏁 Script executed:
# Search for how XmlSchemaException is used or tested in the codebase
rg "XmlSchemaException" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 2654
🏁 Script executed:
# Check if there are any tests for this error path
fd -t f "ReportProcessorTests.cs" | xargs cat | head -100Repository: ANcpLua/Paperless
Length of output: 4900
🏁 Script executed:
# Verify when schemas.Compile() is called and confirm it can throw XmlSchemaException
rg "\.Compile\(\)" --type cs -B 5 -A 2Repository: ANcpLua/Paperless
Length of output: 963
🏁 Script executed:
# Check the test that's testing XmlSchemaException to understand the actual flow
rg "XmlSchemaException Handler Branch" PaperlessREST.Tests/Unit/ReportProcessorTests.cs -A 20Repository: ANcpLua/Paperless
Length of output: 1062
🏁 Script executed:
# Verify if LoadSchemas is called lazily or eagerly, and when Compile() would throw
grep -n "Schemas" PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs | head -10Repository: ANcpLua/Paperless
Length of output: 282
🏁 Script executed:
# Search for when the Schemas property is accessed in ParseAndValidateXmlAsync
sed -n '65,95p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | grep -n "Schemas\|settings"Repository: ANcpLua/Paperless
Length of output: 245
🏁 Script executed:
# Get the complete exception handling section including all handlers
sed -n '120,137p' "PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs" | cat -nRepository: ANcpLua/Paperless
Length of output: 678
🏁 Script executed:
# Check if the test confirms XmlSchemaException is thrown directly (not wrapped)
rg "ProcessAsync_MalformedSchema" PaperlessREST.Tests/Unit/ReportProcessorTests.cs -A 30 | tail -15Repository: ANcpLua/Paperless
Length of output: 654
Clarify exception sources and preserve detailed error messages.
The code comment (lines 127–129) describes only XmlSerializer.Deserialize exception wrapping, but XmlSchemaException is reachable directly from schemas.Compile() during lazy schema initialization. Update the comment to reflect both exception sources.
Additionally, both handlers discard actionable details: InvalidOperationException from deserialization and XmlSchemaException from schema compilation both carry inner exceptions with specific error context. Extract inner exception messages when available:
Proposed fix
catch (InvalidOperationException ex)
{
- // XmlSerializer.Deserialize wraps both XmlException (well-formedness)
- // and XmlSchemaException (validation) as InvalidOperationException, so a
- // separate `catch (XmlException)` branch is unreachable.
- return ReportErrors.InvalidSchema(ex.Message);
+ // XmlSerializer.Deserialize wraps XmlException (well-formedness) and other
+ // exceptions as InvalidOperationException with a generic outer message.
+ string message = ex.InnerException?.Message ?? ex.Message;
+ return ReportErrors.InvalidSchema(message);
}
catch (XmlSchemaException ex)
{
- return ReportErrors.InvalidSchema(ex.Message);
+ // Schema compilation errors (XmlSchemaSet.Compile) with detailed validation context.
+ string message = ex.InnerException?.Message ?? ex.Message;
+ return ReportErrors.InvalidSchema(message);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| catch (InvalidOperationException ex) | |
| { | |
| // XmlSerializer.Deserialize wraps both XmlException (well-formedness) | |
| // and XmlSchemaException (validation) as InvalidOperationException, so a | |
| // separate `catch (XmlException)` branch is unreachable. | |
| return ReportErrors.InvalidSchema(ex.Message); | |
| catch (InvalidOperationException ex) | |
| { | |
| // XmlSerializer.Deserialize wraps XmlException (well-formedness) and other | |
| // exceptions as InvalidOperationException with a generic outer message. | |
| string message = ex.InnerException?.Message ?? ex.Message; | |
| return ReportErrors.InvalidSchema(message); | |
| } | |
| catch (XmlSchemaException ex) | |
| { | |
| // Schema compilation errors (XmlSchemaSet.Compile) with detailed validation context. | |
| string message = ex.InnerException?.Message ?? ex.Message; | |
| return ReportErrors.InvalidSchema(message); | |
| } |
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 129-129: PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs#L129
Remove this commented out code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs` around
lines 125 - 130, Update the comment above the exception handling to mention that
XmlSchemaException can be thrown both by XmlSerializer.Deserialize (wrapped as
InvalidOperationException) and directly by schemas.Compile() during lazy schema
initialization; then change the InvalidOperationException and the
XmlSchemaException handlers so they preserve the most specific error text by
using the inner exception message when present (e.g. use
ex.InnerException?.Message ?? ex.Message) and pass that string into
ReportErrors.InvalidSchema so actionable details are retained; locate the catch
block labeled "catch (InvalidOperationException ex)" and the code path that
calls "schemas.Compile()" to adjust the XmlSchemaException handler accordingly.
| GetResponse<JsonElement> response = await fixture.WaitForDocumentAsync<JsonElement>( | ||
| documentId.ToString(), TestContext.Current.CancellationToken); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fixture_file="$(fd -i 'SharedContainerFixture.cs' | head -n1)"
echo "Fixture file: ${fixture_file}"
# Inspect how WaitForDocumentAsync resolves index names.
rg -n -C4 'WaitForDocumentAsync|DefaultIndex|SearchAsync|GetAsync|Indices|index' "${fixture_file}"Repository: ANcpLua/Paperless
Length of output: 135
🏁 Script executed:
# Search for SharedContainerFixture class definition across the codebase
rg -l 'class SharedContainerFixture' --type csRepository: ANcpLua/Paperless
Length of output: 115
🏁 Script executed:
# Also search for WaitForDocumentAsync method definition
rg -n 'def WaitForDocumentAsync|async.*WaitForDocumentAsync' --type cs -A 5Repository: ANcpLua/Paperless
Length of output: 593
🏁 Script executed:
# Look at the test file itself to understand fixture usage
fd -i 'SearchIndexConcurrencyTests.cs' --type cs -x cat -nRepository: ANcpLua/Paperless
Length of output: 231
🏁 Script executed:
# View WaitForDocumentAsync implementation
sed -n '146,170p' PaperlessServices.Tests/Integration/WorkerTestBase.csRepository: ANcpLua/Paperless
Length of output: 866
🏁 Script executed:
# View the full SearchIndexConcurrencyTests.cs file
fd 'SearchIndexConcurrencyTests.cs' -x cat -nRepository: ANcpLua/Paperless
Length of output: 8417
Assertion validates the fixture's default index instead of the unique test index.
Lines 117–118 call fixture.WaitForDocumentAsync(...) to verify the document, but WaitForDocumentAsync in WorkerTestBase.cs:161 hardcodes client.ElasticsearchClientSettings.DefaultIndex. This test constructs a unique per-test index (indexName at line 98), passes it to the SUT (line 105), and indexes the document into that index (line 109). The assertion then queries the fixture's default index—not the unique test index—so it validates the wrong location. The test can pass even if the document was never indexed into the pre-created unique index, hiding the actual failure mode that branch (c) is meant to exercise.
Pass indexName to the assertion or query ElasticClient directly with explicit index specification.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs` around
lines 117 - 118, The test is asserting against the fixture's default index
because WaitForDocumentAsync in WorkerTestBase uses
client.ElasticsearchClientSettings.DefaultIndex; update the test to assert
against the unique per-test index by passing indexName into the assertion (e.g.,
call fixture.WaitForDocumentAsync(documentId.ToString(),
TestContext.Current.CancellationToken, indexName) or similar), or alternatively
bypass the helper and query the ElasticClient directly specifying indexName when
calling GetAsync/GetResponse; adjust WaitForDocumentAsync
signature/implementation if needed to accept an explicit index parameter so the
test verifies the correct index used by the SUT.
| [Fact] | ||
| public void AddOcrServices_WithSchemelessEndpoint_PrefixesHttp() | ||
| { | ||
| // Arrange — host:port form (the common case from compose.yaml / Testcontainers) | ||
| ServiceCollection services = new(); | ||
| services.AddSingleton(BuildConfiguration(new Dictionary<string, string?> | ||
| { | ||
| ["Storage:Minio:Endpoint"] = "minio.local:9000" | ||
| })); | ||
| services.AddLogging(); | ||
|
|
||
| services.AddOcrServices(); | ||
|
|
||
| using ServiceProvider sp = services.BuildServiceProvider(); | ||
|
|
||
| // Act — resolving the singleton runs the parsing branch | ||
| IMinioClient client = sp.GetRequiredService<IMinioClient>(); | ||
|
|
||
| // Assert — client constructs without throwing; the schemeless path | ||
| // (`if (!endpoint.Contains("://"))` => true) is executed. | ||
| client.Should().NotBeNull(); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd -t f "ServiceCollectionExtensionsTests" --exec cat -n {}Repository: ANcpLua/Paperless
Length of output: 33948
🏁 Script executed:
fd -t f "ServiceCollectionExtensions" -path "*/Host/Extensions/*" --exec cat -n {}Repository: ANcpLua/Paperless
Length of output: 230
🏁 Script executed:
find . -name "ServiceCollectionExtensions.cs" -type f | head -5Repository: ANcpLua/Paperless
Length of output: 191
🏁 Script executed:
cat -n ./PaperlessServices/Host/Extensions/ServiceCollectionExtensions.csRepository: ANcpLua/Paperless
Length of output: 4837
🏁 Script executed:
rg -t cs "interface IMinioClient|class MinioClient" --max-count 5 -A 20Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
rg "WithEndpoint|WithCredentials|WithSSL|Build" --max-count 10 -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 50376
🏁 Script executed:
sed -n '120-186p' PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.csRepository: ANcpLua/Paperless
Length of output: 113
🏁 Script executed:
sed -n '120,186p' PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.csRepository: ANcpLua/Paperless
Length of output: 2042
🏁 Script executed:
rg "interface IMinioClient|\.Endpoint|\.Port|\.SSL" -t cs --max-count 15 -A 2Repository: ANcpLua/Paperless
Length of output: 3803
🏁 Script executed:
cat -n PaperlessREST/Configuration/MinioOptions.csRepository: ANcpLua/Paperless
Length of output: 1542
🏁 Script executed:
cat -n PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs | sed -n '39,70p'Repository: ANcpLua/Paperless
Length of output: 1118
Extract MinIO endpoint normalization into a pure, unit-testable helper.
The three MinIO endpoint/SSL tests (lines 120-141, 143-163, 165-186) only assert client.Should().NotBeNull(). This misses regressions in endpoint parsing (scheme detection, host/port extraction) and SSL branch logic. Extract the normalization code (lines 50-57) into a deterministic helper function and unit-test exact outputs (host, port, scheme) to catch silent behavior changes.
🧰 Tools
🪛 GitHub Check: Codacy Static Code Analysis
[notice] 139-139: PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs#L139
Remove this commented out code.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs` around
lines 120 - 141, Extract the MinIO endpoint parsing logic currently embedded in
AddOcrServices into a pure helper method (e.g., ParseMinioEndpoint or
NormalizeMinioEndpoint) that takes the raw config string and returns a
deterministic tuple/object with scheme, host, and port; update AddOcrServices to
call that helper and use its outputs. Replace the existing tests
AddOcrServices_WithSchemelessEndpoint_PrefixesHttp and the two related SSL/port
tests so they call the new helper directly and assert exact values for scheme,
host, and port (instead of only client.Should().NotBeNull()), ensuring the
schemeless, explicit-http/https, and host:port branches are covered. Ensure the
helper lives in a testable internal/class-visible location and name it so it can
be referenced from unit tests (e.g., ParseMinioEndpoint or
NormalizeMinioEndpoint used by AddOcrServices).
| services.Should().Contain(d => | ||
| d.ServiceType.FullName != null && | ||
| d.ServiceType.FullName.Contains("GeminiOptions", StringComparison.Ordinal), | ||
| "AddPaperlessGenAI binds GeminiOptions via BindConfiguration"); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let me find and read the test file
fd -t f "ServiceCollectionExtensionsTests.cs" --exec cat -n {}Repository: ANcpLua/Paperless
Length of output: 33948
🏁 Script executed:
# Find the AddPaperlessGenAI method
fd -t f "\.cs$" | xargs grep -l "AddPaperlessGenAI" | head -20Repository: ANcpLua/Paperless
Length of output: 191
🏁 Script executed:
# Read the ServiceCollectionExtensions.cs file to find AddPaperlessGenAI
cat -n PaperlessServices/Host/Extensions/ServiceCollectionExtensions.csRepository: ANcpLua/Paperless
Length of output: 4837
🏁 Script executed:
# Search for AddPaperlessGenAI definition
rg -A 10 "AddPaperlessGenAI" --type csRepository: ANcpLua/Paperless
Length of output: 4511
🏁 Script executed:
# Search for GeminiOptions type definition
rg "GeminiOptions" --type cs -B 2 -A 5Repository: ANcpLua/Paperless
Length of output: 1292
🏁 Script executed:
# Search for package references or project files to understand the GenAI library
fd -t f "\.(csproj|json)$" | xargs grep -l "Paperless\|GenAI" 2>/dev/null | head -10Repository: ANcpLua/Paperless
Length of output: 434
🏁 Script executed:
# Check the PaperlessServices.csproj for dependencies
cat PaperlessServices/PaperlessServices.csprojRepository: ANcpLua/Paperless
Length of output: 1607
🏁 Script executed:
# Search more broadly for AddPaperlessGenAI definition
rg "void AddPaperlessGenAI|IServiceCollection AddPaperlessGenAI" --type cs -B 3 -A 15Repository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Look for other options registrations in the test files to see the pattern
rg "IOptions<|IConfigureOptions<" --type csRepository: ANcpLua/Paperless
Length of output: 3538
🏁 Script executed:
# Search for all references to AddOptionsWithValidateOnStart to understand the pattern
rg "AddOptionsWithValidateOnStart|BindConfiguration" --type cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 4547
🏁 Script executed:
# Search for any types or classes named with "GeminiOptions"
rg "class.*GeminiOptions|interface.*GeminiOptions|record.*GeminiOptions" --type csRepository: ANcpLua/Paperless
Length of output: 43
🏁 Script executed:
# Search for all ServiceType references in the test to understand the pattern better
rg "ServiceType ==" PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs -B 2 -A 2Repository: ANcpLua/Paperless
Length of output: 1911
🏁 Script executed:
# Look for what AddPaperlessGenAI might register by checking all service registrations in the test
python3 << 'EOF'
import re
# Read the test file
with open('PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs', 'r') as f:
content = f.read()
# Find the AddGenAiServices_ReturnsSameCollectionAndRegistersGenAi test
test_start = content.find('AddGenAiServices_ReturnsSameCollectionAndRegistersGenAi')
test_end = content.find('\n\t}', test_start) + 3
test_content = content[test_start:test_end]
print("Test method:")
print(test_content)
# Also check if there are other assertions we should see
lines = test_content.split('\n')
for i, line in enumerate(lines):
if 'Should().Contain' in line or 'Should().NotContain' in line:
print(f"\nAssertion {i}: {line.strip()}")
EOFRepository: ANcpLua/Paperless
Length of output: 1072
Assert the concrete registration type instead of substring matching.
ServiceType.FullName.Contains("GeminiOptions") is too broad—it would pass for any IOptions<GeminiOptions>, IConfigureOptions<GeminiOptions>, IValidateOptions<GeminiOptions>, or other related types. The test doesn't specify which contract is actually being verified. Use d.ServiceType == typeof(IOptions<GeminiOptions>) or d.ServiceType == typeof(IConfigureOptions<GeminiOptions>) to keep the assertion explicit and match the pattern established throughout the rest of this test class.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs` around
lines 210 - 213, The assertion currently checks
ServiceType.FullName.Contains("GeminiOptions") which is too broad; update the
predicate inside the services.Should().Contain(...) call to assert the concrete
registration type explicitly (for example, replace the substring check with
d.ServiceType == typeof(IOptions<GeminiOptions>) or d.ServiceType ==
typeof(IConfigureOptions<GeminiOptions>)) so the test verifies the exact
contract registered by AddPaperlessGenAI while keeping the existing assertion
message.
There was a problem hiding this comment.
Pull request overview
This PR aims to unblock backend CI by addressing Release-mode analyzer failures in the test suite, while also performing broader cleanup of unused error factory code and expanding unit/integration test coverage across DI wiring, listener lifecycles, and indexing/report processing edge cases.
Changes:
- Remove the orphaned listener ExecuteAsync test file (which triggered AL0115 due to an empty catch) and replace/consolidate lifecycle coverage in a new test suite.
- Fix const naming to satisfy IDE1006 (
XmlContent), adding/adjusting coverage around report XML date parsing. - Remove unused REST-side error factory helpers (e.g.,
RichProblemDetailsFactory,ReportErrors.InvalidXml,BatchErrors) and add/extend tests for service registration, OpenAPI metadata extensions, and failure-path behaviors.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs | Adds unit coverage for PaperlessServices DI extension methods (OCR + GenAI registrations and MinIO endpoint parsing branches). |
| PaperlessServices.Tests/Unit/SearchIndexServiceTests.cs | Adds a unit test to exercise the IndexDocumentAsync catch/logging path when the ES client throws. |
| PaperlessServices.Tests/Integration/SearchIndexConcurrencyTests.cs | Adds integration coverage for SearchIndexService.InitializeAsync concurrency/existence branches and createdAt=null behavior. |
| PaperlessREST/Host/Extensions/RichProblemDetailsFactory.cs | Deletes an unused ProblemDetails factory + metadata helper extensions. |
| PaperlessREST/Features/DocumentManagement/Application/DocumentErrors.cs | Removes unused/legacy error factory members. |
| PaperlessREST/Features/BatchProcessing/Application/ReportProcessor.cs | Removes an unreachable catch (XmlException) branch and documents why. |
| PaperlessREST/Features/BatchProcessing/Application/ReportErrors.cs | Removes unused InvalidXml factory and deletes an embedded BatchErrors class. |
| PaperlessREST.Tests/Unit/UploadDocumentRequestDtoTests.cs | Adds coverage for record copy-constructor behavior used by with expressions. |
| PaperlessREST.Tests/Unit/TypedErrorOrAsyncExtensionsTests.cs | Refactors/expands tests for TypedErrorOrAsyncExtensions mapping to typed HTTP results and problem details extensions. |
| PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs | Adds unit tests covering REST host DI registration lambdas and dev/prod endpoint mapping behavior. |
| PaperlessREST.Tests/Unit/RichProblemDetailsFactoryTests.cs | Removes tests for the deleted RichProblemDetailsFactory/ErrorMetadataExtensions. |
| PaperlessREST.Tests/Unit/ReportProcessorTests.cs | Adds a new test case for xs:date-with-timezone passing schema validation but failing DateOnly.TryParseExact; renames XmlContent const. |
| PaperlessREST.Tests/Unit/OpenApiMetadataExtensionsTests.cs | Adds tests verifying OpenAPI metadata extension helpers attach expected response status metadata. |
| PaperlessREST.Tests/Unit/ListenerLifecycleTests.cs | Adds/centralizes BackgroundService lifecycle coverage for listeners, including cancellation and OperationInterruptedException filters. |
| PaperlessREST.Tests/Unit/ListenerExecuteAsyncTests.cs | Deletes the older duplicate/orphan lifecycle test file that triggered AL0115. |
| PaperlessREST.Tests/Unit/DocumentServiceTests.cs | Adds tests for unknown storage exception propagation and OCR state-transition failure behavior. |
| PaperlessREST.Tests/Unit/ContractViolationExceptionTests.cs | Refactors/extends coverage of ContractViolationException factories, message building, and diagnostics records. |
| PaperlessREST.Tests/Unit/BatchAndReportErrorsTests.cs | Updates tests to reflect removal of ReportErrors.InvalidXml and deletion of BatchErrors. |
Comments suppressed due to low confidence (1)
PaperlessREST/Features/BatchProcessing/Application/ReportErrors.cs:12
- The PR description says this change only deletes an orphan listener test file and renames a
constfor IDE1006, but the diff also removes production code (RichProblemDetailsFactory, multipleDocumentErrorsfactories,ReportErrors.InvalidXml+BatchErrors) and adds several new unit/integration test suites. Please update the PR description (or split the PR) so reviewers/CI expectations match the actual scope and any behavior changes from removing these APIs are explicitly called out.
namespace PaperlessREST.Features.BatchProcessing.Application;
public static class ReportErrors
{
public static Error FileNotFound(string path) => Error.NotFound(
"Report.FileNotFound",
$"File not found: {path}");
public static Error InvalidSchema(string details) => Error.Validation(
"Report.InvalidSchema",
$"XML does not match expected schema: {details}");
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| FakeLogger<SearchIndexService> logger) => | ||
| new( | ||
| client, | ||
| Options.Create(new ElasticsearchOptions | ||
| { | ||
| Uri = client.ElasticsearchClientSettings.NodePool.Nodes.First().Uri.ToString(), | ||
| DefaultIndex = indexName | ||
| }), | ||
| TimeProvider.System, | ||
| logger); |
PR #19 merged with the backend CI gate red. Two Release-mode lint errors:
PaperlessREST.Tests/Unit/ListenerExecuteAsyncTests.cs:254:3—AL0115: Catch block is empty. This file is a PR test: push backend coverage to 97.3% (CI gate metric) #16 leftover that survived the rebase alongsideListenerLifecycleTests.cs. The lifecycle file already covers the same surface with sharper assertions andTaskCompletionSourcesynchronisation (noTask.Delaypolling, per the BackgroundService gotcha in CLAUDE.md). Dropping the duplicate.PaperlessREST.Tests/Unit/ReportProcessorTests.cs:554:16—IDE1006: xmlContent must be PascalCase(it's aconst, so the camelCase exemption doesn't apply). Renamed toXmlContent.Verified locally with
./build.sh Compile --configuration Release(clean, 0 errors / 0 warnings).Test plan
Build & Test (backend)goes green🤖 Generated with Claude Code