From 57c173a426f1a4f11ed6b27d3a4b3eae247c87a5 Mon Sep 17 00:00:00 2001 From: ancplua Date: Sun, 2 Aug 2026 00:35:42 +0200 Subject: [PATCH 1/2] Complete backend coverage and simplify test infrastructure --- .env.example | 4 +- .env.test | 3 +- AGENTS.md | 4 +- Directory.Packages.props | 10 +- Paperless.TestSupport/AssemblyInfo.cs | 19 +- Paperless.TestSupport/ContainerFixtureBase.cs | 258 ++++++----- Paperless.TestSupport/FakeLoggerExtensions.cs | 24 +- Paperless.TestSupport/TestEnv.cs | 20 +- PaperlessREST.Tests/DocumentBuilder.cs | 20 - .../Integration/DocumentEndpointTests.cs | 426 +++++++++++++++--- .../GlobalExceptionHandlerMiddlewareTests.cs | 191 +++----- .../Integration/RabbitMqExtensionsTests.cs | 17 +- .../Integration/SharedRestContainerFixture.cs | 106 ++++- .../Unit/BatchOrchestratorTests.cs | 382 +++------------- .../Unit/DocumentSearchServiceTests.cs | 97 ---- .../Unit/DocumentServiceContractTests.cs | 298 +++++++++--- .../Unit/DocumentServiceTestBase.cs | 4 +- .../Unit/DocumentStorageServiceTests.cs | 42 +- .../Unit/ExceptionHandlerTests.cs | 313 ------------- .../Unit/GlobalExceptionHandlerTests.cs | 231 ++++------ .../Unit/PathNormalizationTests.cs | 69 +++ .../Unit/ServiceCollectionExtensionsTests.cs | 108 +++-- PaperlessREST/API/GlobalExceptionHandler.cs | 92 ++-- PaperlessREST/Configuration/Constraints.cs | 10 - PaperlessREST/Configuration/MinioOptions.cs | 23 +- .../DocumentManagement/DocumentDtos.cs | 16 - .../Application/BatchOrchestrator.cs | 21 +- .../Application/DocumentService.cs | 82 ++-- .../Search/DocumentSearchService.cs | 89 ++-- .../Storage/DocumentStorageService.cs | 25 +- .../Endpoints/DocumentEndpoints.cs | 37 +- .../Extensions/ServiceCollectionExtensions.cs | 23 +- .../Integration/OcrIntegrationTests.cs | 17 + .../Integration/StorageIntegrationTests.cs | 16 +- .../Integration/WorkerTestBase.cs | 25 +- .../Unit/CreatePdfExtractorTests.cs | 123 +---- .../Unit/FakeLoggerExtensionsTests.cs | 51 +++ .../Unit/OcrProcessorTests.cs | 37 +- .../Unit/ServiceCollectionExtensionsTests.cs | 74 +-- .../Unit/StorageServiceTests.cs | 21 +- .../Configuration/MinioOptions.cs | 3 +- .../OcrProcessing/Application/OcrProcessor.cs | 4 + .../PdfExtractor/CreatePdfExtractor.cs | 4 +- .../PdfExtractor/IPdfExtractor.cs | 4 +- .../Infrastructure/Storage/StorageService.cs | 22 +- .../Extensions/ServiceCollectionExtensions.cs | 24 +- .../src/app/core/api/generated/api-types.ts | 9 - Pipeline/Build.csproj | 12 +- 48 files changed, 1664 insertions(+), 1846 deletions(-) delete mode 100644 PaperlessREST.Tests/Unit/DocumentSearchServiceTests.cs delete mode 100644 PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs create mode 100644 PaperlessREST.Tests/Unit/PathNormalizationTests.cs create mode 100644 PaperlessServices.Tests/Unit/FakeLoggerExtensionsTests.cs diff --git a/.env.example b/.env.example index a6d08d6..d95b180 100644 --- a/.env.example +++ b/.env.example @@ -49,11 +49,11 @@ CONNECTIONSTRINGS__HANGFIRE= # ============================================ # Storage Configuration # ============================================ -STORAGE__MINIO__ENDPOINT= +# Compose service origin; host-run apps use http://localhost:${MINIO_PORT}. +STORAGE__MINIO__ENDPOINT=http://minio:9000 STORAGE__MINIO__ACCESSKEY= STORAGE__MINIO__SECRETKEY= STORAGE__MINIO__BUCKETNAME= -STORAGE__MINIO__USESSL= # ============================================ # Messaging Configuration diff --git a/.env.test b/.env.test index b03d136..890043b 100644 --- a/.env.test +++ b/.env.test @@ -38,11 +38,10 @@ MINIO_ROOT_PASSWORD=minioadmin CONNECTIONSTRINGS__PAPERLESSDB=Host=localhost;Port=5432;Database=paperless;Username=postgres;Password=postgres CONNECTIONSTRINGS__HANGFIRE=Host=localhost;Port=5432;Database=paperless;Username=postgres;Password=postgres RABBITMQ__URI=amqp://guest:guest@localhost:5672/ -STORAGE__MINIO__ENDPOINT=localhost:9000 +STORAGE__MINIO__ENDPOINT=http://localhost:9000 STORAGE__MINIO__ACCESSKEY=minioadmin STORAGE__MINIO__SECRETKEY=minioadmin STORAGE__MINIO__BUCKETNAME=paperless-test -STORAGE__MINIO__USESSL=false ELASTICSEARCH__URI=http://localhost:9200 ELASTICSEARCH__DEFAULTINDEX=paperless-test diff --git a/AGENTS.md b/AGENTS.md index 22ad4b0..7090f5e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,7 +72,7 @@ Coverage uploads to https://codecov.io/gh/ANcpLua/Paperless via tokenless OIDC. Coverage uses `DotCov.Nuke` (NuGet, owner `ANcpLua/dotcov`) as a NUKE component package — `Pipeline/Build.csproj:29` references it, `Pipeline/Build.cs:43` mixes the `ICoverageReport` interface into the build class. There is no `dotnet tool` CLI; the gate runs via `./build.sh ReportCoverage`. Two parameters matter on this repo: -- `--coverage-exclude-generated-param true` (the NUKE [Parameter] kebab-cased form of `--exclude-generated`) strips generators, migrations, designer files, async state-machine sequence points, `Program.cs`. With this flag, the gate metric is **99.8% line coverage (928/930)** — see the sequence-point note in Gotchas. Disable by passing `false` to see the raw numbers (currently ~73% with generated code mixed in). +- `--coverage-exclude-generated-param true` (the NUKE [Parameter] kebab-cased form of `--exclude-generated`) strips generators, migrations, designer files, async state-machine sequence points, `Program.cs`. With this flag, the gate metric is **99.9% line coverage (1359/1361)** — see the sequence-point note in Gotchas. Disable by passing `false` to see the raw numbers (currently 66.5% with generated code mixed in). - `--coverage-min-line` / `--coverage-min-branch` set the gate threshold. CI passes `0 / 0` (report-only mode); per-file numbers and the overall summary still publish to the workflow log as markdown. Codecov-side gating is configured in `codecov.yml` (project: auto, patch: 80%). ## NUKE Cohesion (build code quality bar) @@ -100,7 +100,7 @@ Anti-patterns that fail the self-check: - **BackgroundService race in tests**: `BackgroundService.StartAsync` returns before `ExecuteAsync` runs. Don't wait on a log predicate that's already true for an empty snapshot (`_ => true`). Signal via `TaskCompletionSource` from a mock's `DisposeAsync` or `AckAsync`, then await that. - **Hangfire NU1107**: Hangfire + Hangfire.AspNetCore must move together. Renovate split them once and broke restore on `main` for days. - **Gemini placeholder key**: `.env.test` ships `GEMINI__APIKEY=test-gemini-key-placeholder`. The integration test must mock `ITextSummarizer` (`FakeTextSummarizer` in `PaperlessServices.Tests/Integration/`), not hit the real API. -- **The "missing 2 lines" of coverage are sequence-point artifacts, not testable code**. Gate metric is 928/930 = 99.8%. The two unhit lines are the closing braces of try/catch blocks in `GenAiResultListener.cs` and `ReportProcessor.cs:120` — Roslyn emits a sequence point on the fall-through-after-catch path, but every code path inside those try blocks either `return`s early or unwinds via exception. No test can reach those sequence points without breaking the design intent. Leave them; do not chase 100% by restructuring around the coverage tool. +- **The "missing 2 lines" of coverage are sequence-point artifacts, not testable code**. Gate metric is 1359/1361 = 99.9%. The two unhit lines are the closing braces at `GenAiResultListener.cs:34` and `ReportProcessor.cs:114` — Roslyn emits sequence points on fall-through-after-catch paths that the methods cannot take. No test can reach those sequence points without breaking the design intent. Leave them; do not chase 100% by restructuring around the coverage tool. - **Custom SDK in Dockerfiles**: PaperlessREST.csproj uses `` (version pinned in `global.json` msbuild-sdks). For docker builds, `global.json` + `nuget.config` + `Directory.Packages.props` + `Version.props` must be COPYed into the build context BEFORE `dotnet restore`, otherwise the SDK resolver errors with "Could not resolve SDK". Both Dockerfiles do this; if you copy a Dockerfile for a new project, preserve those COPY lines. ## Rating-Matrix mapping (course grading) diff --git a/Directory.Packages.props b/Directory.Packages.props index 7116dd8..615fc35 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -84,15 +84,11 @@ - + - + diff --git a/Paperless.TestSupport/AssemblyInfo.cs b/Paperless.TestSupport/AssemblyInfo.cs index 0cc9877..cb2bf57 100644 --- a/Paperless.TestSupport/AssemblyInfo.cs +++ b/Paperless.TestSupport/AssemblyInfo.cs @@ -1,19 +1,4 @@ -// Test infrastructure is not product code, so it is not measured. -// -// Coverage answers "did the tests exercise this line". Asking that of the fixtures -// the tests are built from is circular: every helper the suites touch reports as -// covered by construction, and the only thing a number here can reveal is a dead -// helper — which an unused-symbol warning already tells us, more directly. -// -// Left measured it also distorts the gate. These 468 lines sat in the same -// aggregate as PaperlessREST and PaperlessServices, so ContainerFixtureBase -// (77.2%) and FakeLoggerExtensions (83.3%) depressed the number that decides -// whether a push to main passes, while saying nothing about product risk. -// -// This attribute excludes the assembly at collection time, so the raw Cobertura -// never contains it and every downstream consumer — the DotCov gate, Codecov, the -// ReportGenerator HTML — agrees without needing its own rule. The filters in -// Pipeline/Components/ICoverage.cs and codecov.yml still name TestSupport, but as -// a backstop should this attribute ever be dropped, not as the mechanism. +// Shared test infrastructure is not product behavior. Exclude it at collection time so +// local and hosted reports measure PaperlessREST and PaperlessServices consistently. [assembly: ExcludeFromCodeCoverage] diff --git a/Paperless.TestSupport/ContainerFixtureBase.cs b/Paperless.TestSupport/ContainerFixtureBase.cs index 12e8ce9..794c893 100644 --- a/Paperless.TestSupport/ContainerFixtureBase.cs +++ b/Paperless.TestSupport/ContainerFixtureBase.cs @@ -1,16 +1,10 @@ +using Elastic.Transport; +using System.Runtime.ExceptionServices; + namespace Paperless.TestSupport; /// -/// Template-method base for the integration-test container fixtures. Owns the -/// shared container lifecycle (RabbitMQ + MinIO + Elasticsearch, plus an optional -/// Postgres), MinIO bucket creation, Elasticsearch readiness polling, the -/// Elasticsearch document/search polling helpers, and ordered teardown. -/// -/// Derived fixtures supply the system-under-test by overriding -/// (which must assign ) -/// and tear it down in . The base never references -/// PaperlessREST or PaperlessServices. -/// +/// Owns shared integration infrastructure while derived fixtures own the system under test. /// public abstract class ContainerFixtureBase : IAsyncLifetime { @@ -21,22 +15,19 @@ public abstract class ContainerFixtureBase : IAsyncLifetime private readonly MinioContainer _minio = TestContainers.Minio(); private readonly ElasticsearchContainer _elastic = TestContainers.Elasticsearch(); - protected ContainerFixtureBase() + protected ContainerFixtureBase(bool usesPostgres) { - _postgres = UsesPostgres ? TestContainers.Postgres() : null; + _postgres = usesPostgres ? TestContainers.Postgres() : null; } - /// Whether to start a Postgres container (REST = true, Services = false). - protected abstract bool UsesPostgres { get; } - /// Unique per-fixture bucket name; the bucket is created during init. protected string BucketName { get; } = $"test-{Guid.NewGuid():N}"; /// Unique per-fixture default Elasticsearch index name. protected string IndexName { get; } = $"test_{Guid.NewGuid():N}"; - /// MinIO host:port endpoint string (valid after containers start). - protected string MinioEndpoint => MinioBucket.Endpoint(_minio); + /// MinIO endpoint URI (valid after containers start). + protected string MinioEndpoint => $"http://{MinioBucket.Endpoint(_minio)}"; protected string MinioAccessKey => _minio.GetAccessKey(); protected string MinioSecretKey => _minio.GetSecretKey(); @@ -45,10 +36,10 @@ protected ContainerFixtureBase() protected string ElasticsearchUri => $"http://{_elastic.Hostname}:{_elastic.GetMappedPublicPort(ElasticsearchPort)}"; - /// Postgres connection string; throws if is false. + /// Postgres connection string; throws when the fixture has no Postgres container. protected string PostgresConnectionString => (_postgres ?? throw new InvalidOperationException( - "This fixture did not request a Postgres container (UsesPostgres == false).")) + "This fixture did not request a Postgres container.")) .GetConnectionString(); /// Service provider for the constructed SUT. Assigned by . @@ -65,7 +56,6 @@ public async ValueTask InitializeAsync() if (_postgres is not null) starts.Add(_postgres.StartAsync()); await Task.WhenAll(starts); - await WaitForElasticsearchAsync(); await MinioBucket.CreateBucketAsync(_minio, BucketName); await ConfigureSutAsync(); @@ -73,18 +63,64 @@ public async ValueTask InitializeAsync() public async ValueTask DisposeAsync() { - // SUT first (it consumes the containers), then the infra. Plain awaits, no - // best-effort catch: a teardown failure is a real fault that must surface. - // DisposeSutAsync overrides null-guard their own SUT, so a failed - // InitializeAsync cannot NRE here, and Testcontainers' Ryuk reaper cleans up - // any container a mid-teardown throw leaves running. - await DisposeSutAsync(); - await _rabbit.DisposeAsync(); - await _minio.DisposeAsync(); - await _elastic.DisposeAsync(); + List failures = []; + + try + { + await DisposeSutAsync(); + } + catch (Exception exception) + { + failures.Add(exception); + } + + try + { + await _rabbit.DisposeAsync(); + } + catch (Exception exception) + { + failures.Add(exception); + } + + try + { + await _minio.DisposeAsync(); + } + catch (Exception exception) + { + failures.Add(exception); + } + + try + { + await _elastic.DisposeAsync(); + } + catch (Exception exception) + { + failures.Add(exception); + } + if (_postgres is not null) { - await _postgres.DisposeAsync(); + try + { + await _postgres.DisposeAsync(); + } + catch (Exception exception) + { + failures.Add(exception); + } + } + + if (failures.Count > 1) + { + throw new AggregateException("Fixture teardown failed.", failures); + } + + if (failures.Count == 1) + { + ExceptionDispatchInfo.Capture(failures[0]).Throw(); } } @@ -111,30 +147,46 @@ public async Task> WaitForDocumentAsync( pollInterval ??= TimeSpan.FromMilliseconds(100); var client = Services.GetRequiredService(); - using CancellationTokenSource cts = new(timeout.Value); - using var linked = - CancellationTokenSource.CreateLinkedTokenSource(cts.Token, cancellationToken); + using CancellationTokenSource timeoutCts = new(timeout.Value); + using CancellationTokenSource linkedCts = + CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); - while (!linked.Token.IsCancellationRequested) + try { - var response = await client.GetAsync( - documentId, - g => g.Index(client.ElasticsearchClientSettings.DefaultIndex), - linked.Token); - - if (response.Found) + while (true) { - return response; - } + var response = await ExecuteElasticsearchAsync( + token => client.GetAsync( + documentId, + g => g.Index(client.ElasticsearchClientSettings.DefaultIndex), + token), + linkedCts.Token); + + if (response.Found) + { + return response; + } - await Task.Delay(pollInterval.Value, linked.Token); + await Task.Delay(pollInterval.Value, linkedCts.Token); + } + } + catch (OperationCanceledException) when ( + timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested) + { + return await ExecuteElasticsearchAsync( + token => client.GetAsync( + documentId, + g => g.Index(client.ElasticsearchClientSettings.DefaultIndex), + token), + cancellationToken); + } + catch (OperationCanceledException exception) when (cancellationToken.IsCancellationRequested) + { + throw new OperationCanceledException( + "Elasticsearch polling was canceled.", + exception, + cancellationToken); } - - // Final attempt before throwing - return await client.GetAsync( - documentId, - g => g.Index(client.ElasticsearchClientSettings.DefaultIndex), - cancellationToken); } /// @@ -146,89 +198,69 @@ public async Task> WaitForSearchResultsAsync( TimeSpan? timeout = null, TimeSpan? pollInterval = null) { - // 30s overall budget: GitHub-hosted runners are markedly slower than local - // dev machines and the first SearchAsync after index creation can spend - // several seconds priming query caches even after Refresh.True returns. timeout ??= TimeSpan.FromSeconds(30); pollInterval ??= TimeSpan.FromMilliseconds(100); var client = Services.GetRequiredService(); - using CancellationTokenSource overallCts = new(timeout.Value); - using var overallLinked = - CancellationTokenSource.CreateLinkedTokenSource(overallCts.Token, cancellationToken); - - // Force an index-level refresh up front. SearchIndexService writes documents - // with Refresh.True (`?refresh=true`), which is supposed to guarantee - // immediate searchability — but on slow CI disks the per-document refresh - // is observed to not always propagate before the first SearchAsync. The - // explicit Indices.RefreshAsync here is defensive and idempotent: locally - // it's a no-op (everything's already refreshed), on CI it converts an - // invisible flake into a passing search. - try - { - await client.Indices.RefreshAsync( - r => r.Indices(client.ElasticsearchClientSettings.DefaultIndex), - overallLinked.Token); - } - catch (OperationCanceledException) when (overallLinked.Token.IsCancellationRequested) - { - // Fall through to the final attempt below. - } + using CancellationTokenSource timeoutCts = new(timeout.Value); + using CancellationTokenSource linkedCts = + CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); - while (!overallLinked.Token.IsCancellationRequested) + try { - try + // Refresh.True writes can still lag behind search on slow CI storage. + await ExecuteElasticsearchAsync( + token => client.Indices.RefreshAsync( + r => r.Indices(client.ElasticsearchClientSettings.DefaultIndex), + token), + linkedCts.Token); + + while (true) { - var response = await client.SearchAsync(configureSearch, overallLinked.Token); + var response = await ExecuteElasticsearchAsync( + token => client.SearchAsync(configureSearch, token), + linkedCts.Token); if (response.Documents.Count > 0) { return response; } - } - catch (OperationCanceledException) when (overallLinked.Token.IsCancellationRequested) - { - break; - } - try - { - await Task.Delay(pollInterval.Value, overallLinked.Token); - } - catch (OperationCanceledException) - { - break; + await Task.Delay(pollInterval.Value, linkedCts.Token); } } - - // Final attempt with the caller's token only so the assertion sees real - // "found nothing" data rather than a TaskCanceledException at the wait boundary. - return await client.SearchAsync(configureSearch, cancellationToken); + catch (OperationCanceledException) when ( + timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested) + { + return await ExecuteElasticsearchAsync( + token => client.SearchAsync(configureSearch, token), + cancellationToken); + } + catch (OperationCanceledException exception) when (cancellationToken.IsCancellationRequested) + { + throw new OperationCanceledException( + "Elasticsearch polling was canceled.", + exception, + cancellationToken); + } } - private async Task WaitForElasticsearchAsync() + private static async Task ExecuteElasticsearchAsync( + Func> operation, + CancellationToken cancellationToken) { - Uri elasticUri = new(ElasticsearchUri + "/"); - using HttpClient http = new() { Timeout = TimeSpan.FromSeconds(2) }; - - for (var i = 0; i < 30; i++) + try { - try - { - var response = await http.GetAsync($"{elasticUri}_cluster/health"); - if (response.IsSuccessStatusCode) - { - return; - } - } - catch (HttpRequestException) - { - // Container not ready yet - } - - await Task.Delay(500); + return await operation(cancellationToken); + } + catch (TransportException exception) when ( + cancellationToken.IsCancellationRequested && + exception.InnerException is OperationCanceledException) + { + throw new OperationCanceledException( + "Elasticsearch operation was canceled.", + exception, + cancellationToken); } - - throw new InvalidOperationException("Elasticsearch failed to become ready"); } } diff --git a/Paperless.TestSupport/FakeLoggerExtensions.cs b/Paperless.TestSupport/FakeLoggerExtensions.cs index 7eb00b8..c795ce4 100644 --- a/Paperless.TestSupport/FakeLoggerExtensions.cs +++ b/Paperless.TestSupport/FakeLoggerExtensions.cs @@ -35,28 +35,34 @@ public static async Task WaitForLogAsync( timeout ??= TimeSpan.FromSeconds(5); pollInterval ??= TimeSpan.FromMilliseconds(25); - using CancellationTokenSource cts = - CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - cts.CancelAfter(timeout.Value); + using CancellationTokenSource timeoutCts = new(timeout.Value); + using CancellationTokenSource linkedCts = + CancellationTokenSource.CreateLinkedTokenSource(timeoutCts.Token, cancellationToken); try { - while (!cts.Token.IsCancellationRequested) + while (true) { if (condition(source.GetSnapshot())) { return true; } - await Task.Delay(pollInterval.Value, cts.Token).ConfigureAwait(false); + await Task.Delay(pollInterval.Value, linkedCts.Token).ConfigureAwait(false); } } - catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) + catch (OperationCanceledException) when ( + timeoutCts.IsCancellationRequested && !cancellationToken.IsCancellationRequested) { - // Timeout expired, not user cancellation + return condition(source.GetSnapshot()); + } + catch (OperationCanceledException exception) when (cancellationToken.IsCancellationRequested) + { + throw new OperationCanceledException( + "Waiting for a log condition was canceled.", + exception, + cancellationToken); } - - return condition(source.GetSnapshot()); // Final check } /// diff --git a/Paperless.TestSupport/TestEnv.cs b/Paperless.TestSupport/TestEnv.cs index 5e00e5e..de57f3c 100644 --- a/Paperless.TestSupport/TestEnv.cs +++ b/Paperless.TestSupport/TestEnv.cs @@ -1,23 +1,14 @@ namespace Paperless.TestSupport; -/// -/// One-time .env.test loading and environment-variable image resolution -/// shared by every integration fixture. Replaces the three duplicated static -/// constructors (REST shared fixture, REST DatabaseFixture, Services fixture). -/// +/// Loads .env.test once and resolves container images from environment variables. public static class TestEnv { private static readonly Lock s_gate = new(); private static bool s_loaded; - /// - /// Loads .env.test exactly once per process via - /// . Idempotent and thread-safe so it can be - /// called from every fixture's static constructor without re-loading. - /// + /// Loads .env.test exactly once per process. public static void Load() { - if (s_loaded) return; lock (s_gate) { if (s_loaded) return; @@ -26,12 +17,7 @@ public static void Load() } } - /// - /// Returns the container image for , falling back to - /// when the variable is unset. Mirrors the - /// Environment.GetEnvironmentVariable(...) ?? "image:tag" pattern the - /// fixtures duplicated per container. - /// + /// Returns an environment-selected container image or its default. public static string Image(string envVar, string defaultImage) => Environment.GetEnvironmentVariable(envVar) ?? defaultImage; } diff --git a/PaperlessREST.Tests/DocumentBuilder.cs b/PaperlessREST.Tests/DocumentBuilder.cs index e25387a..bd38a5b 100644 --- a/PaperlessREST.Tests/DocumentBuilder.cs +++ b/PaperlessREST.Tests/DocumentBuilder.cs @@ -215,23 +215,3 @@ public UploadDocumentRequest Build() return new UploadDocumentRequest { File = fileMock.Object }; } } - -public sealed class SearchQueryBuilder -{ - private int _limit = 10; - private string _query = "search"; - - public SearchQueryBuilder WithQuery(string query) - { - _query = query; - return this; - } - - public SearchQueryBuilder WithLimit(int limit) - { - _limit = limit; - return this; - } - - public SearchQuery Build() => new() { Query = _query, Limit = _limit }; -} diff --git a/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs b/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs index 1120a96..13165a6 100644 --- a/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs +++ b/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs @@ -1,20 +1,33 @@ +using Elastic.Clients.Elasticsearch; +using AwesomeAssertions.Execution; + namespace PaperlessREST.Tests.Integration; -public sealed class DocumentEndpointTests : IClassFixture, IAsyncLifetime +[Collection(SharedRestContainerCollection.Name)] +public sealed class DocumentEndpointTests : IAsyncLifetime { #region Constructor public DocumentEndpointTests(SharedRestContainerFixture fixture) { _fixture = fixture; + _elastic = fixture.Services.GetRequiredService(); + _search = fixture.Services.GetRequiredService(); _cleanup = new AsyncCleanup(async () => { - if (_createdDocIds.Count == 0) return; - await using var scope = _fixture.CreateAsyncScope(); - var factory = - scope.ServiceProvider.GetRequiredService>(); - await using var db = await factory.CreateDbContextAsync(); - await db.Documents.Where(d => _createdDocIds.Contains(d.Id)).ExecuteDeleteAsync(); + if (_createdDocIds.Count > 0) + { + await using var scope = _fixture.CreateAsyncScope(); + var factory = + scope.ServiceProvider.GetRequiredService>(); + await using var db = await factory.CreateDbContextAsync(); + await db.Documents.Where(d => _createdDocIds.Contains(d.Id)).ExecuteDeleteAsync(); + } + + foreach (var id in _indexedDocIds) + { + await _search.DeleteAsync(id); + } }); } @@ -23,28 +36,94 @@ public DocumentEndpointTests(SharedRestContainerFixture fixture) #region Tests - GetDocuments [Fact] - public async Task Get_ReturnsOkWithDocuments() + public async Task Get_WithoutPageSize_UsesDefaultAndCursorReturnsRemainder() { - // Arrange & Act — explicit pageSize avoids any [AsParameters] - // default-binding edge cases on the cursor-paginated endpoint. - var response = await _fixture.Client.GetAsync( - $"{DocumentsEndpoint}?pageSize=20", + var documentIds = await SeedDocumentsAsync( + PaginationConstraints.DefaultPageSize + 1, + $"{TestFilePrefix}-page-{Guid.NewGuid():N}"); + + using var firstResponse = await _fixture.Client.GetAsync( + DocumentsEndpoint, TestContext.Current.CancellationToken); + var firstPage = await ReadSuccessJsonAsync(firstResponse); - // Assert — capture body on failure so future regressions show the - // actual problem-details payload, not a JSON-parser error. - if (!response.IsSuccessStatusCode) + using AssertionScope _ = new(); + firstPage.Items.Should().HaveCount(PaginationConstraints.DefaultPageSize); + firstPage.HasMore.Should().BeTrue(); + firstPage.NextCursor.Should().Be(firstPage.Items[^1].Id); + + using var secondResponse = await _fixture.Client.GetAsync( + $"{DocumentsEndpoint}?cursor={firstPage.NextCursor}", + TestContext.Current.CancellationToken); + var secondPage = await ReadSuccessJsonAsync(secondResponse); + + secondPage.Items.Should().ContainSingle(); + secondPage.HasMore.Should().BeFalse(); + secondPage.NextCursor.Should().BeNull(); + firstPage.Items.Select(d => d.Id) + .Concat(secondPage.Items.Select(d => d.Id)) + .Should().BeEquivalentTo(documentIds); + } + + #endregion + + #region Tests - Search endpoint + + [Fact] + public async Task Search_WithoutLimit_UsesDefaultAndMapsIndexedDocuments() + { + var marker = $"invoice{Guid.NewGuid():N}"; + List indexed = []; + for (var i = 0; i <= SearchConstraints.DefaultResultLimit; i++) { - var body = await response.Content.ReadAsStringAsync( - TestContext.Current.CancellationToken); - throw new InvalidOperationException( - $"GET {DocumentsEndpoint}?pageSize=20 returned {(int)response.StatusCode} {response.ReasonPhrase}. Body: {body}"); + indexed.Add(await IndexSearchDocumentAsync( + $"{TestFilePrefix}-search-{i}.pdf", + $"{marker} account statement {i}", + $"Summary {i}")); + } + + using var response = await _fixture.Client.GetAsync( + $"{DocumentsEndpoint}/search?query={marker}", + TestContext.Current.CancellationToken); + var results = await ReadSuccessJsonAsync>(response); + + results.Should().HaveCount(SearchConstraints.DefaultResultLimit); + foreach (var result in results) + { + var expected = indexed.Single(document => document.Id == result.Id); + result.FileName.Should().Be(expected.FileName); + result.Content.Should().Be(expected.Content); + result.Summary.Should().Be(expected.Summary); + result.CreatedAt.Should().Be(expected.CreatedAt); + result.Status.Should().Be(expected.Status); } + } - var page = await response.Content.ReadFromJsonAsync( + public static IEnumerable> InvalidSearchRequests() + { + yield return new TheoryDataRow($"{DocumentsEndpoint}/search") + .WithTestDisplayName("missing query"); + yield return new TheoryDataRow($"{DocumentsEndpoint}/search?query=") + .WithTestDisplayName("empty query"); + yield return new TheoryDataRow( + $"{DocumentsEndpoint}/search?query={new string('q', SearchConstraints.QueryMaxLength + 1)}") + .WithTestDisplayName("query above maximum"); + yield return new TheoryDataRow($"{DocumentsEndpoint}/search?query=invoice&limit=0") + .WithTestDisplayName("zero limit"); + yield return new TheoryDataRow( + $"{DocumentsEndpoint}/search?query=invoice&limit={SearchConstraints.MaxResultLimit + 1}") + .WithTestDisplayName("limit above maximum"); + } + + [Theory] + [MemberData(nameof(InvalidSearchRequests))] + public async Task Search_InvalidQueryOrLimit_ReturnsBadRequest(string requestUri) + { + using var response = await _fixture.Client.GetAsync( + requestUri, TestContext.Current.CancellationToken); - page.Should().NotBeNull(); - page!.Items.Should().NotBeNull(); + + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); } #endregion @@ -52,87 +131,246 @@ public async Task Get_ReturnsOkWithDocuments() #region Tests - Upload [Fact] - public async Task Upload_ValidPdf_Returns202WithLocation() + public async Task Upload_ValidPdf_ReturnsAcceptedDocument() { - // Arrange var uniqueFileName = $"{TestFilePrefix}-upload-{Guid.NewGuid():N}.pdf"; using var content = await CreatePdfUploadAsync(uniqueFileName); - // Act - var response = await _fixture.Client.PostAsync( + using var response = await _fixture.Client.PostAsync( DocumentsEndpoint, content, TestContext.Current.CancellationToken); - // Assert response.StatusCode.Should().Be(HttpStatusCode.Accepted); - var result = await response.Content.ReadFromJsonAsync( TestContext.Current.CancellationToken); + result.Should().NotBeNull(); result!.Id.Should().NotBeEmpty(); - response.Headers.Location?.ToString().Should().Contain(result.Id.ToString()); - + result.FileName.Should().Be(uniqueFileName); + result.Status.Should().Be(DocumentStatus.Pending.ToString()); _createdDocIds.Add(result.Id); } + [Fact] + public async Task Upload_AboveMaximumSize_ReturnsBadRequest() + { + using var content = CreateUpload( + $"{TestFilePrefix}-oversized.pdf", + ContentTypePdf, + new byte[FileUploadConstraints.MaxFileSizeBytes + 1]); + + using var response = await _fixture.Client.PostAsync( + DocumentsEndpoint, + content, + TestContext.Current.CancellationToken); + var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + + using AssertionScope _ = new(); + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + body.Should().Contain("File size cannot exceed 10 MB"); + } + + [Fact] + public async Task Upload_NonPdfContentType_ReturnsBadRequest() + { + using var content = CreateUpload( + $"{TestFilePrefix}-text.txt", + "text/plain", + [1, 2, 3]); + + using var response = await _fixture.Client.PostAsync( + DocumentsEndpoint, + content, + TestContext.Current.CancellationToken); + var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + + using AssertionScope _ = new(); + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + body.Should().Contain("Only PDF files are allowed"); + } + #endregion - #region Tests - GetById + #region Tests - GetById and summary [Fact] public async Task GetById_ExistingDocument_ReturnsDocument() { - // Arrange var docId = await SeedDocumentAsync($"{TestFilePrefix}-get-{Guid.NewGuid():N}.pdf"); - // Act - var response = await _fixture.Client.GetAsync( + using var response = await _fixture.Client.GetAsync( $"{DocumentsEndpoint}/{docId}", TestContext.Current.CancellationToken); + var doc = await ReadSuccessJsonAsync(response); + + doc.Id.Should().Be(docId); + } - // Assert - var doc = await response.Content.ReadFromJsonAsync( + [Fact] + public async Task GetSummary_ExistingDocument_ReturnsSummary() + { + const string summary = "A concise account statement summary."; + var docId = await SeedDocumentAsync( + $"{TestFilePrefix}-summary-{Guid.NewGuid():N}.pdf", + summary); + + using var response = await _fixture.Client.GetAsync( + $"{DocumentsEndpoint}/{docId}/summary", TestContext.Current.CancellationToken); - doc!.Id.Should().Be(docId); + var result = await ReadSuccessJsonAsync(response); + + result.Summary.Should().Be(summary); } #endregion - #region Tests - Delete + #region Tests - Delete endpoint [Fact] public async Task Delete_ExistingDocument_Returns204() { - // Arrange var docId = await SeedDocumentAsync($"{TestFilePrefix}-delete-{Guid.NewGuid():N}.pdf"); - // Act - var response = await _fixture.Client.DeleteAsync( + using var response = await _fixture.Client.DeleteAsync( $"{DocumentsEndpoint}/{docId}", TestContext.Current.CancellationToken); - // Assert response.StatusCode.Should().Be(HttpStatusCode.NoContent); - - // Remove from cleanup list since it's already deleted _createdDocIds.Remove(docId); } #endregion - #region Constants + #region Tests - DocumentSearchService - private const string DocumentsEndpoint = "/api/v1/documents"; - private const string ContentTypePdf = "application/pdf"; - private const string TestFilePrefix = "endpoint-test"; + [Fact] + public async Task SearchService_MatchingAndMissingQueries_ReturnExpectedDocuments() + { + var marker = $"receivable{Guid.NewGuid():N}"; + var expected = await IndexSearchDocumentAsync( + $"{TestFilePrefix}-matching.pdf", + $"Open {marker} balance"); + + var matches = await _search.SearchAsync( + marker, + 10, + TestContext.Current.CancellationToken); + var missing = await _search.SearchAsync( + $"absent{Guid.NewGuid():N}", + 10, + TestContext.Current.CancellationToken); + + matches.Should().ContainSingle().Which.Should().BeEquivalentTo(expected); + missing.Should().BeEmpty(); + } + + [Fact] + public async Task SearchService_LimitRestrictsReturnedDocuments() + { + var marker = $"limited{Guid.NewGuid():N}"; + List indexedIds = []; + for (var i = 0; i < 3; i++) + { + var document = await IndexSearchDocumentAsync( + $"{TestFilePrefix}-limit-{i}.pdf", + $"{marker} entry {i}"); + indexedIds.Add(document.Id); + } + + var results = await _search.SearchAsync( + marker, + 2, + TestContext.Current.CancellationToken); + + results.Should().HaveCount(2); + results.Select(result => result.Id).Should().BeSubsetOf(indexedIds); + } + + [Fact] + public async Task SearchService_DeleteExistingAndMissingIds_IsIdempotent() + { + var document = await IndexSearchDocumentAsync( + $"{TestFilePrefix}-search-delete.pdf", + $"delete{Guid.NewGuid():N}"); + + await _search.DeleteAsync(document.Id, TestContext.Current.CancellationToken); + var response = await _elastic.GetAsync( + document.Id.ToString(), + get => get.Index(_elastic.ElasticsearchClientSettings.DefaultIndex), + TestContext.Current.CancellationToken); + + response.Found.Should().BeFalse(); + await _search.DeleteAsync(document.Id, TestContext.Current.CancellationToken); + await _search.DeleteAsync(Guid.CreateVersion7(), TestContext.Current.CancellationToken); + } + + [Fact] + public async Task SearchService_CanceledSearch_StopsElasticsearchRequest() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + + Func act = async () => + await _search.SearchAsync("invoice", 10, cancellation.Token); + + var exception = await act.Should().ThrowAsync(); + exception.Which.CancellationToken.Should().Be(cancellation.Token); + } + + [Fact] + public async Task SearchService_CanceledDelete_PropagatesCallerToken() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + + Func act = () => _search.DeleteAsync(Guid.CreateVersion7(), cancellation.Token); + + var exception = await act.Should().ThrowAsync(); + exception.Which.CancellationToken.Should().Be(cancellation.Token); + } + + [Fact] + public async Task WaitForDocument_CallerCancellation_PropagatesCallerToken() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + + Func act = () => _fixture.WaitForDocumentAsync( + Guid.CreateVersion7().ToString(), + cancellation.Token); + + var exception = await act.Should().ThrowAsync(); + exception.Which.CancellationToken.Should().Be(cancellation.Token); + } + + [Fact] + public async Task WaitForSearchResults_CallerCancellation_PropagatesCallerToken() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + + Func act = () => _fixture.WaitForSearchResultsAsync( + search => search.Size(1), + cancellation.Token); + + var exception = await act.Should().ThrowAsync(); + exception.Which.CancellationToken.Should().Be(cancellation.Token); + } #endregion - #region Fields + #region Constants and fields + + private const string DocumentsEndpoint = "/api/v1/documents"; + private const string ContentTypePdf = "application/pdf"; + private const string TestFilePrefix = "endpoint-test"; private readonly SharedRestContainerFixture _fixture; + private readonly ElasticsearchClient _elastic; + private readonly IDocumentSearchService _search; private readonly List _createdDocIds = []; + private readonly List _indexedDocIds = []; private readonly AsyncCleanup _cleanup; #endregion @@ -145,9 +383,9 @@ public async Task Delete_ExistingDocument_Returns204() #endregion - #region Helper Methods + #region Helper methods - private async Task SeedDocumentAsync(string fileName) + private async Task SeedDocumentAsync(string fileName, string? summary = null) { await using var scope = _fixture.CreateAsyncScope(); var factory = @@ -155,26 +393,98 @@ private async Task SeedDocumentAsync(string fileName) await using var db = await factory.CreateDbContextAsync( TestContext.Current.CancellationToken); - var entity = new DocumentBuilder() - .WithFileName(fileName) - .BuildEntity(); + var builder = new DocumentBuilder().WithFileName(fileName); + if (summary is not null) + { + builder.WithSummary(summary); + } + var entity = builder.BuildEntity(); db.Documents.Add(entity); await db.SaveChangesAsync(TestContext.Current.CancellationToken); _createdDocIds.Add(entity.Id); return entity.Id; } + private async Task> SeedDocumentsAsync(int count, string fileNamePrefix) + { + await using var scope = _fixture.CreateAsyncScope(); + var factory = + scope.ServiceProvider.GetRequiredService>(); + await using var db = await factory.CreateDbContextAsync( + TestContext.Current.CancellationToken); + + var entities = Enumerable.Range(0, count) + .Select(i => new DocumentBuilder() + .WithFileName($"{fileNamePrefix}-{i}.pdf") + .BuildEntity()) + .ToList(); + db.Documents.AddRange(entities); + await db.SaveChangesAsync(TestContext.Current.CancellationToken); + + var ids = entities.ConvertAll(entity => entity.Id); + _createdDocIds.AddRange(ids); + return ids; + } + + private async Task IndexSearchDocumentAsync( + string fileName, + string content, + string? summary = null) + { + var document = new DocumentSearchResult + { + Id = Guid.CreateVersion7(), + FileName = fileName, + Status = DocumentStatus.Completed.ToString(), + CreatedAt = new DateTimeOffset(2026, 8, 1, 10, 0, 0, TimeSpan.Zero), + Content = content, + ProcessedAt = new DateTimeOffset(2026, 8, 1, 10, 1, 0, TimeSpan.Zero), + Summary = summary, + SummaryGeneratedAt = summary is null + ? null + : new DateTimeOffset(2026, 8, 1, 10, 2, 0, TimeSpan.Zero) + }; + + var response = await _elastic.IndexAsync( + document, + index => index + .Index(_elastic.ElasticsearchClientSettings.DefaultIndex) + .Id(document.Id.ToString()) + .Refresh(Refresh.True), + TestContext.Current.CancellationToken); + + response.IsValidResponse.Should().BeTrue(); + _indexedDocIds.Add(document.Id); + return document; + } + [SuppressMessage("Reliability", "CA2000:Dispose objects before losing scope", Justification = "The ByteArrayContent's ownership transfers to the returned MultipartFormDataContent, " + "which every caller disposes via 'using var content = ...'.")] - private static async Task CreatePdfUploadAsync(string fileName) + private static MultipartFormDataContent CreateUpload(string fileName, string contentType, byte[] bytes) { - var pdf = new ByteArrayContent(await TestPdf.BytesAsync("Test Document")) + var file = new ByteArrayContent(bytes) { - Headers = { ContentType = MediaTypeHeaderValue.Parse(ContentTypePdf) } + Headers = { ContentType = MediaTypeHeaderValue.Parse(contentType) } }; - return new MultipartFormDataContent { { pdf, "file", fileName } }; + return new MultipartFormDataContent { { file, "file", fileName } }; + } + + private static async Task CreatePdfUploadAsync(string fileName) => + CreateUpload(fileName, ContentTypePdf, await TestPdf.BytesAsync("Test Document")); + + private static async Task ReadSuccessJsonAsync(HttpResponseMessage response) + { + var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + if (!response.IsSuccessStatusCode) + { + throw new InvalidOperationException( + $"Request returned {(int)response.StatusCode} {response.ReasonPhrase}. Body: {body}"); + } + + return JsonSerializer.Deserialize(body, JsonSerializerOptions.Web) + ?? throw new InvalidOperationException("The response body was empty or invalid JSON"); } #endregion diff --git a/PaperlessREST.Tests/Integration/GlobalExceptionHandlerMiddlewareTests.cs b/PaperlessREST.Tests/Integration/GlobalExceptionHandlerMiddlewareTests.cs index a0dfbe5..5e637fd 100644 --- a/PaperlessREST.Tests/Integration/GlobalExceptionHandlerMiddlewareTests.cs +++ b/PaperlessREST.Tests/Integration/GlobalExceptionHandlerMiddlewareTests.cs @@ -28,14 +28,14 @@ public async Task Request_WhenNoException_ReturnsOk() #region Tests - ValidationException [Fact] - public async Task Request_ValidationException_Returns400WithProblemDetails() + public async Task Request_ValidationExceptionWithMembers_ReturnsEveryDistinctNonblankMember() { // Arrange await using TestHostContext ctx = await CreateTestHostAsync(); // Act HttpResponseMessage response = await ctx.Client.GetAsync( - $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeValidation}", + $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeValidationMembers}", TestContext.Current.CancellationToken); // Assert @@ -48,89 +48,76 @@ public async Task Request_ValidationException_Returns400WithProblemDetails() problem.Should().NotBeNull(); problem!.Status.Should().Be(Status400BadRequest); problem.Type.Should().Be(ValidationErrorType); - problem.Errors.Should().ContainKey(FieldName); - problem.Errors[FieldName].Should().Contain(FieldError); - } - - #endregion - - #region Tests - Forbidden Exception - - [Fact] - public async Task Request_UnauthorizedAccessException_Returns403WithProblemDetails() - { - // Arrange - await using TestHostContext ctx = await CreateTestHostAsync(); - - // Act - HttpResponseMessage response = await ctx.Client.GetAsync( - $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeUnauthorized}", - TestContext.Current.CancellationToken); - - // Assert - response.StatusCode.Should().Be(HttpStatusCode.Forbidden); - response.Content.Headers.ContentType?.MediaType.Should().Be(ContentTypeJson); - - ProblemDetails? problem = await response.Content.ReadFromJsonAsync( - TestContext.Current.CancellationToken); - - problem.Should().NotBeNull(); - problem!.Status.Should().Be(Status403Forbidden); - problem.Type.Should().Be(ForbiddenType); + problem.Errors.Should().BeEquivalentTo(new Dictionary + { + [FieldName] = [FieldError], + [SecondFieldName] = [FieldError] + }); } - #endregion - - #region Tests - Timeout Exception - [Fact] - public async Task Request_TimeoutException_Returns504WithProblemDetails() + public async Task Request_ValidationExceptionWithoutMembers_ReturnsModelLevelError() { // Arrange await using TestHostContext ctx = await CreateTestHostAsync(); // Act HttpResponseMessage response = await ctx.Client.GetAsync( - $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeTimeout}", + $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeValidationModel}", TestContext.Current.CancellationToken); // Assert - response.StatusCode.Should().Be(HttpStatusCode.GatewayTimeout); - response.Content.Headers.ContentType?.MediaType.Should().Be(ContentTypeJson); - - ProblemDetails? problem = await response.Content.ReadFromJsonAsync( + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + HttpValidationProblemDetails? problem = await response.Content.ReadFromJsonAsync( TestContext.Current.CancellationToken); problem.Should().NotBeNull(); - problem!.Status.Should().Be(Status504GatewayTimeout); - problem.Type.Should().Be(TimeoutType); + problem!.Errors.Should().ContainSingle() + .Which.Should().BeEquivalentTo(new KeyValuePair(string.Empty, [FieldError])); } #endregion #region Tests - Internal Server Error - [Fact] - public async Task Request_UnhandledException_Returns500WithProblemDetails() + [Theory] + [MemberData(nameof(UnownedExceptionTypes))] + public async Task Request_UnownedException_ReturnsSanitized500ProblemDetails(string exceptionType) { // Arrange await using TestHostContext ctx = await CreateTestHostAsync(); // Act HttpResponseMessage response = await ctx.Client.GetAsync( - $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeNotSupported}", + $"{ThrowEndpoint}?{ExceptionTypeParam}={exceptionType}", TestContext.Current.CancellationToken); // Assert response.StatusCode.Should().Be(HttpStatusCode.InternalServerError); response.Content.Headers.ContentType?.MediaType.Should().Be(ContentTypeJson); + string body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + body.Should().NotContain(SensitiveInternalErrorMessage); - ProblemDetails? problem = await response.Content.ReadFromJsonAsync( - TestContext.Current.CancellationToken); + ProblemDetails? problem = JsonSerializer.Deserialize(body, JsonSerializerOptions.Web); problem.Should().NotBeNull(); problem!.Status.Should().Be(Status500InternalServerError); problem.Type.Should().Be(InternalErrorType); + problem.Detail.Should().Be(GenericInternalErrorDetail); + problem.Extensions.Should().NotContainKey("debug"); + } + + public static IEnumerable UnownedExceptionTypes() + { + yield return new TheoryDataRow(ExceptionTypeArgument); + yield return new TheoryDataRow(ExceptionTypeArgumentNull); + yield return new TheoryDataRow(ExceptionTypeInvalidOperation); + yield return new TheoryDataRow(ExceptionTypeKeyNotFound); + yield return new TheoryDataRow(ExceptionTypeFileNotFound); + yield return new TheoryDataRow(ExceptionTypeUnauthorized); + yield return new TheoryDataRow(ExceptionTypeTimeout); + yield return new TheoryDataRow(ExceptionTypeOperationCanceled); + yield return new TheoryDataRow(ExceptionTypeNotSupported); } #endregion @@ -163,12 +150,11 @@ public async ValueTask DisposeAsync() private const string ValidationErrorType = "urn:paperless:error:validation_error"; private const string BadRequestType = "urn:paperless:error:bad_request"; - private const string NotFoundType = "urn:paperless:error:not_found"; - private const string ForbiddenType = "urn:paperless:error:forbidden"; - private const string TimeoutType = "urn:paperless:error:timeout"; private const string InternalErrorType = "urn:paperless:error:internal_error"; - private const string ExceptionTypeValidation = "validation"; + private const string ExceptionTypeValidationMembers = "validation-members"; + private const string ExceptionTypeValidationModel = "validation-model"; + private const string ExceptionTypeBadHttpRequest = "bad-http-request"; private const string ExceptionTypeArgument = "argument"; private const string ExceptionTypeArgumentNull = "argumentnull"; private const string ExceptionTypeInvalidOperation = "invalidoperation"; @@ -176,94 +162,48 @@ public async ValueTask DisposeAsync() private const string ExceptionTypeFileNotFound = "filenotfound"; private const string ExceptionTypeUnauthorized = "unauthorized"; private const string ExceptionTypeTimeout = "timeout"; + private const string ExceptionTypeOperationCanceled = "operation-canceled"; private const string ExceptionTypeNotSupported = "notsupported"; private const string FieldName = "Email"; + private const string SecondFieldName = "Name"; private const string FieldError = "Email is required"; - private const string NotFoundMessage = "Document not found"; - private const string BadRequestMessage = "Invalid request"; - private const string ForbiddenMessage = "Access denied"; - private const string TimeoutMessage = "Operation timed out"; - private const string InternalErrorMessage = "Something went wrong"; + private const string BadRequestMessage = "Malformed request containing sensitive input"; + private const string SafeBadRequestDetail = "The request was invalid."; + private const string SensitiveInternalErrorMessage = "Database password was exposed"; + private const string GenericInternalErrorDetail = + "An internal error occurred. Please contact support if the problem persists."; private const int Status400BadRequest = StatusCodes.Status400BadRequest; - private const int Status403Forbidden = StatusCodes.Status403Forbidden; - private const int Status404NotFound = StatusCodes.Status404NotFound; private const int Status500InternalServerError = StatusCodes.Status500InternalServerError; - private const int Status504GatewayTimeout = StatusCodes.Status504GatewayTimeout; #endregion - #region Tests - BadRequest Exceptions - - public static IEnumerable BadRequestExceptions() - { - yield return new TheoryDataRow(ExceptionTypeArgument) - .WithTestDisplayName("Argument → 400"); - yield return new TheoryDataRow(ExceptionTypeArgumentNull) - .WithTestDisplayName("ArgumentNull → 400"); - yield return new TheoryDataRow(ExceptionTypeInvalidOperation) - .WithTestDisplayName("InvalidOperation → 400"); - } + #region Tests - BadHttpRequestException - [Theory] - [MemberData(nameof(BadRequestExceptions))] - public async Task Request_BadRequestException_Returns400WithProblemDetails(string exceptionType) + [Fact] + public async Task Request_BadHttpRequestException_ReturnsSanitized400ProblemDetails() { // Arrange await using TestHostContext ctx = await CreateTestHostAsync(); // Act HttpResponseMessage response = await ctx.Client.GetAsync( - $"{ThrowEndpoint}?{ExceptionTypeParam}={exceptionType}", + $"{ThrowEndpoint}?{ExceptionTypeParam}={ExceptionTypeBadHttpRequest}", TestContext.Current.CancellationToken); // Assert response.StatusCode.Should().Be(HttpStatusCode.BadRequest); response.Content.Headers.ContentType?.MediaType.Should().Be(ContentTypeJson); - ProblemDetails? problem = await response.Content.ReadFromJsonAsync( - TestContext.Current.CancellationToken); + string body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken); + body.Should().NotContain(BadRequestMessage); + ProblemDetails? problem = JsonSerializer.Deserialize(body, JsonSerializerOptions.Web); problem.Should().NotBeNull(); problem!.Status.Should().Be(Status400BadRequest); problem.Type.Should().Be(BadRequestType); - } - - #endregion - - #region Tests - NotFound Exceptions - - public static IEnumerable NotFoundExceptions() - { - yield return new TheoryDataRow(ExceptionTypeKeyNotFound) - .WithTestDisplayName("KeyNotFound → 404"); - yield return new TheoryDataRow(ExceptionTypeFileNotFound) - .WithTestDisplayName("FileNotFound → 404"); - } - - [Theory] - [MemberData(nameof(NotFoundExceptions))] - public async Task Request_NotFoundException_Returns404WithProblemDetails(string exceptionType) - { - // Arrange - await using TestHostContext ctx = await CreateTestHostAsync(); - - // Act - HttpResponseMessage response = await ctx.Client.GetAsync( - $"{ThrowEndpoint}?{ExceptionTypeParam}={exceptionType}", - TestContext.Current.CancellationToken); - - // Assert - response.StatusCode.Should().Be(HttpStatusCode.NotFound); - response.Content.Headers.ContentType?.MediaType.Should().Be(ContentTypeJson); - - ProblemDetails? problem = await response.Content.ReadFromJsonAsync( - TestContext.Current.CancellationToken); - - problem.Should().NotBeNull(); - problem!.Status.Should().Be(Status404NotFound); - problem.Type.Should().Be(NotFoundType); + problem.Detail.Should().Be(SafeBadRequestDetail); } #endregion @@ -321,6 +261,7 @@ private static async Task CreateTestHostAsync() { webBuilder .UseTestServer() + .UseEnvironment(Environments.Production) .ConfigureServices(services => { services.AddFakeLogging(); @@ -355,15 +296,21 @@ private static async Task CreateTestHostAsync() private static Exception CreateException(string exceptionType) => exceptionType switch { - ExceptionTypeValidation => new ValidationException(new ValidationResult(FieldError, [FieldName]), null, null), - ExceptionTypeArgument => new ArgumentException(BadRequestMessage), - ExceptionTypeArgumentNull => new ArgumentNullException(nameof(exceptionType), BadRequestMessage), - ExceptionTypeInvalidOperation => new InvalidOperationException(BadRequestMessage), - ExceptionTypeKeyNotFound => new KeyNotFoundException(NotFoundMessage), - ExceptionTypeFileNotFound => new FileNotFoundException(NotFoundMessage), - ExceptionTypeUnauthorized => new UnauthorizedAccessException(ForbiddenMessage), - ExceptionTypeTimeout => new TimeoutException(TimeoutMessage), - ExceptionTypeNotSupported => new NotSupportedException(InternalErrorMessage), + ExceptionTypeValidationMembers => new ValidationException( + new ValidationResult(FieldError, [FieldName, SecondFieldName, FieldName, string.Empty, " "]), + null, + null), + ExceptionTypeValidationModel => new ValidationException(new ValidationResult(FieldError), null, null), + ExceptionTypeBadHttpRequest => new BadHttpRequestException(BadRequestMessage), + ExceptionTypeArgument => new ArgumentException(SensitiveInternalErrorMessage), + ExceptionTypeArgumentNull => new ArgumentNullException(nameof(exceptionType), SensitiveInternalErrorMessage), + ExceptionTypeInvalidOperation => new InvalidOperationException(SensitiveInternalErrorMessage), + ExceptionTypeKeyNotFound => new KeyNotFoundException(SensitiveInternalErrorMessage), + ExceptionTypeFileNotFound => new FileNotFoundException(SensitiveInternalErrorMessage), + ExceptionTypeUnauthorized => new UnauthorizedAccessException(SensitiveInternalErrorMessage), + ExceptionTypeTimeout => new TimeoutException(SensitiveInternalErrorMessage), + ExceptionTypeOperationCanceled => new OperationCanceledException(SensitiveInternalErrorMessage), + ExceptionTypeNotSupported => new NotSupportedException(SensitiveInternalErrorMessage), _ => new InvalidOperationException($"Unknown exception type: {exceptionType}") }; diff --git a/PaperlessREST.Tests/Integration/RabbitMqExtensionsTests.cs b/PaperlessREST.Tests/Integration/RabbitMqExtensionsTests.cs index cd43d8e..787eb80 100644 --- a/PaperlessREST.Tests/Integration/RabbitMqExtensionsTests.cs +++ b/PaperlessREST.Tests/Integration/RabbitMqExtensionsTests.cs @@ -2,42 +2,39 @@ namespace PaperlessREST.Tests.Integration; public class RabbitMqExtensionsTests { - static RabbitMqExtensionsTests() => TestEnv.Load(); - - private static string GetRabbitMqConnectionString() => - Environment.GetEnvironmentVariable("RABBITMQ__URI")!; + private const string RabbitMqUri = "amqp://localhost:5672/"; [Fact] - public void AddPaperlessRabbitMq_WithOcrStream_ShouldRegisterSseStream() + public async Task AddPaperlessRabbitMq_WithOcrStream_ShouldRegisterSseStream() { ServiceCollection services = []; services.AddLogging(); IConfigurationRoot config = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { - ["RabbitMQ:Uri"] = GetRabbitMqConnectionString() + ["RabbitMQ:Uri"] = RabbitMqUri }).Build(); services.AddPaperlessRabbitMq(config, true); - ServiceProvider provider = services.BuildServiceProvider(); + await using ServiceProvider provider = services.BuildServiceProvider(); provider.GetService>().Should().NotBeNull(); } [Fact] - public void AddPaperlessRabbitMq_WithGenAiStreamEnabled_RegistersSseStream() + public async Task AddPaperlessRabbitMq_WithGenAiStreamEnabled_RegistersSseStream() { ServiceCollection services = []; services.AddLogging(); IConfigurationRoot config = new ConfigurationBuilder().AddInMemoryCollection(new Dictionary { - ["RabbitMQ:Uri"] = GetRabbitMqConnectionString() + ["RabbitMQ:Uri"] = RabbitMqUri }).Build(); services.AddPaperlessRabbitMq(config, includeGenAiResultStream: true); - ServiceProvider provider = services.BuildServiceProvider(); + await using ServiceProvider provider = services.BuildServiceProvider(); ISseStream? sseStream = provider.GetService>(); sseStream.Should().NotBeNull(); } diff --git a/PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs b/PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs index 829d972..4056db0 100644 --- a/PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs +++ b/PaperlessREST.Tests/Integration/SharedRestContainerFixture.cs @@ -1,15 +1,21 @@ using PaperlessREST.Host; +using System.Runtime.ExceptionServices; [assembly: CaptureConsole] [assembly: CaptureTrace] namespace PaperlessREST.Tests.Integration; -public sealed class SharedRestContainerFixture : ContainerFixtureBase +[CollectionDefinition(Name, DisableParallelization = true)] +public sealed class SharedRestContainerCollection : ICollectionFixture { - static SharedRestContainerFixture() => TestEnv.Load(); + public const string Name = "Shared REST containers"; +} - protected override bool UsesPostgres => true; +public sealed class SharedRestContainerFixture() : ContainerFixtureBase(usesPostgres: true) +{ + static SharedRestContainerFixture() => TestEnv.Load(); + private readonly Dictionary _originalEnvironment = new(StringComparer.OrdinalIgnoreCase); public HttpClient Client { get; private set; } = null!; public IDbContextFactory DbFactory { get; private set; } = null!; @@ -17,27 +23,29 @@ public sealed class SharedRestContainerFixture : ContainerFixtureBase public AsyncServiceScope CreateAsyncScope() => Services.CreateAsyncScope(); private WebApplicationFactory? _factory; + private string? _batchRoot; protected override async ValueTask ConfigureSutAsync() { - // Point the REST host's infra config at the Testcontainers endpoints via environment - // variables. This is deliberate, not a regression: WebApplicationFactory + minimal hosting - // builds the app's own configuration (including the environment-variable source that - // `.env.test` populates process-globally), and that source OUTRANKS anything the factory adds - // via ConfigureAppConfiguration — even Sources.Clear() only touches the host-config layer, so - // an in-memory override is silently beaten by `.env.test`'s RABBITMQ__URI=localhost:5672 and - // every endpoint 500s (BrokerUnreachable). Setting the env vars to the real container values is - // the only thing the WAF host actually reads. (The Services fixture, a plain Host builder, can - // and does use Sources.Clear()+AddInMemoryCollection — minimal-hosting WAF cannot.) - Environment.SetEnvironmentVariable("CONNECTIONSTRINGS__PAPERLESSDB", PostgresConnectionString); - Environment.SetEnvironmentVariable("CONNECTIONSTRINGS__HANGFIRE", PostgresConnectionString); - Environment.SetEnvironmentVariable("RABBITMQ__URI", RabbitConnectionString); - Environment.SetEnvironmentVariable("STORAGE__MINIO__ENDPOINT", MinioEndpoint); - Environment.SetEnvironmentVariable("STORAGE__MINIO__ACCESSKEY", MinioAccessKey); - Environment.SetEnvironmentVariable("STORAGE__MINIO__SECRETKEY", MinioSecretKey); - Environment.SetEnvironmentVariable("STORAGE__MINIO__BUCKETNAME", BucketName); - Environment.SetEnvironmentVariable("ELASTICSEARCH__URI", ElasticsearchUri); - Environment.SetEnvironmentVariable("ELASTICSEARCH__DEFAULTINDEX", IndexName); + // WebApplicationFactory's environment provider outranks test-host configuration, + // so its process environment must contain the real container endpoints. + OverrideEnvironmentVariable("CONNECTIONSTRINGS__PAPERLESSDB", PostgresConnectionString); + OverrideEnvironmentVariable("CONNECTIONSTRINGS__HANGFIRE", PostgresConnectionString); + OverrideEnvironmentVariable("RABBITMQ__URI", RabbitConnectionString); + OverrideEnvironmentVariable("STORAGE__MINIO__ENDPOINT", MinioEndpoint); + OverrideEnvironmentVariable("STORAGE__MINIO__ACCESSKEY", MinioAccessKey); + OverrideEnvironmentVariable("STORAGE__MINIO__SECRETKEY", MinioSecretKey); + OverrideEnvironmentVariable("STORAGE__MINIO__BUCKETNAME", BucketName); + OverrideEnvironmentVariable("ELASTICSEARCH__URI", ElasticsearchUri); + OverrideEnvironmentVariable("ELASTICSEARCH__DEFAULTINDEX", IndexName); + + _batchRoot = Path.Combine(Path.GetTempPath(), $"paperless-batch-{Guid.NewGuid():N}"); + OverrideEnvironmentVariable("BATCH__INPUTPATH", Path.Combine(_batchRoot, "input")); + OverrideEnvironmentVariable("BATCH__ARCHIVEPATH", Path.Combine(_batchRoot, "archive")); + OverrideEnvironmentVariable("BATCH__ERRORPATH", Path.Combine(_batchRoot, "error")); + OverrideEnvironmentVariable("BATCH__FILEPATTERN", "*.xml"); + OverrideEnvironmentVariable("BATCH__CRONEXPRESSION", "0 2 * * *"); + OverrideEnvironmentVariable("BATCH__TIMEZONEID", "UTC"); _factory = new ConfiguredWebApplicationFactory(PostgresConnectionString); @@ -51,8 +59,60 @@ protected override async ValueTask ConfigureSutAsync() protected override async ValueTask DisposeSutAsync() { - if (_factory is not null) - await _factory.DisposeAsync(); + List failures = []; + + try + { + if (_factory is not null) + { + await _factory.DisposeAsync(); + } + } + catch (Exception exception) + { + failures.Add(exception); + } + + foreach ((string name, string? value) in _originalEnvironment) + { + try + { + Environment.SetEnvironmentVariable(name, value); + } + catch (Exception exception) + { + failures.Add(exception); + } + } + _originalEnvironment.Clear(); + + try + { + if (_batchRoot is not null && Directory.Exists(_batchRoot)) + { + Directory.Delete(_batchRoot, recursive: true); + } + } + catch (Exception exception) + { + failures.Add(exception); + } + + if (failures.Count > 1) + { + throw new AggregateException("REST fixture teardown failed.", failures); + } + + if (failures.Count == 1) + { + ExceptionDispatchInfo.Capture(failures[0]).Throw(); + } + } + + private void OverrideEnvironmentVariable(string name, string value) + { + _originalEnvironment.TryAdd(name, Environment.GetEnvironmentVariable(name)); + Environment.SetEnvironmentVariable(name, value); } private sealed class ConfiguredWebApplicationFactory(string postgresConnectionString) diff --git a/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs b/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs index f0d1576..a368c98 100644 --- a/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs +++ b/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs @@ -4,9 +4,7 @@ namespace PaperlessREST.Tests.Unit; /// -/// Unit tests for BatchOrchestrator organized by interaction pattern. -/// ProcessAsync uses MockFileSystem for full flow tests. -/// ProcessFile uses Mock<IFileSystem> for direct method testing. +/// Full-flow unit tests for BatchOrchestrator using MockFileSystem. /// public static class BatchOrchestratorTests { @@ -25,10 +23,8 @@ private static BatchOptions CreateOptions() => #endregion - // ═══════════════════════════════════════════════════════════════ // PROCESS ASYNC TESTS // Full flow tests using MockFileSystem (real fake file system) - // ═══════════════════════════════════════════════════════════════ public sealed class ProcessAsync : IDisposable { @@ -167,6 +163,38 @@ public async Task SourceFileDisappears_LogsWarningAndContinues() l.Message.Contains("no longer exists", StringComparison.OrdinalIgnoreCase)); } + [Fact] + public async Task ArchiveMoveFails_ThrowsInfrastructureErrorAndLeavesClaimedFile() + { + // Arrange + CreateTestFile("report.xml"); + + _reportProcessor.Setup(p => p.ProcessAsync( + It.Is(path => path.EndsWith("report.xml.processing", StringComparison.Ordinal)), + It.IsAny())) + .ReturnsAsync(new ProcessingResult(1, 0)); + + using var interception = _fileSystem.Intercept.Event( + _ => throw new UnauthorizedAccessException("Access denied"), + change => change.ChangeType == WatcherChangeTypes.Renamed && + change.Path.StartsWith(ArchivePath, StringComparison.Ordinal)); + BatchOrchestrator sut = CreateSut(); + + // Act + Func act = () => sut.ProcessAsync(CreateToken()); + + // Assert + IOException exception = (await act.Should().ThrowAsync()).Which; + exception.Message.Should().Contain("Infrastructure error moving file"); + exception.InnerException.Should().BeOfType(); + _fileSystem.Directory.GetFiles(InputPath) + .Should().ContainSingle(path => path.EndsWith("report.xml.processing", StringComparison.Ordinal)); + _logCollector.GetSnapshot() + .Should().Contain(log => + log.Level == LogLevel.Error && + log.Message.Contains("Hangfire will retry", StringComparison.OrdinalIgnoreCase)); + } + #endregion #region Tests - Cancellation @@ -342,6 +370,29 @@ public async Task SingleValidFile_LogsSuccessfulProcessing() l.Message.Contains("Successfully processed", StringComparison.OrdinalIgnoreCase)); } + [Fact] + public async Task OriginalFileNameContainingProcessing_PreservesEmbeddedTextWhenArchived() + { + // Arrange + const string originalFileName = "report.processing.xml"; + CreateTestFile(originalFileName); + + _reportProcessor.Setup(p => p.ProcessAsync( + It.Is(path => path.EndsWith($"{originalFileName}.processing", StringComparison.Ordinal)), + It.IsAny())) + .ReturnsAsync(new ProcessingResult(1, 0)); + + BatchOrchestrator sut = CreateSut(); + + // Act + await sut.ProcessAsync(CreateToken()); + + // Assert + _fileSystem.Directory.GetFiles(ArchivePath) + .Should().ContainSingle(path => Path.GetFileName(path) + .StartsWith($"{originalFileName}.", StringComparison.Ordinal)); + } + #endregion #region Tests - Multiple Files @@ -572,327 +623,6 @@ public async Task ErrorDirectoryMissing_CreatesDirectory() #endregion } - // ═══════════════════════════════════════════════════════════════ - // PROCESS FILE TESTS (requires internal access) - // Direct tests using strict mocks for ProcessFileAsync - // ═══════════════════════════════════════════════════════════════ - - public sealed class ProcessFile : IDisposable - { - #region Constructor - - public ProcessFile() - { - _fs = _mocks.Create(); - _processor = _mocks.Create(); - _logger = new FakeLogger(_logCollector); - } - - #endregion - - #region IDisposable - - public void Dispose() - { - TestContext.Current.SendDiagnosticMessage("Full logs:\n{0}", _logCollector.GetFullLoggerText()); - _mocks.VerifyAll(); - _mocks.VerifyNoOtherCalls(); - } - - #endregion - - #region Tests - File Not Found - - [Fact] - public async Task WhenSourceFileDisappears_LogsWarning() - { - // Arrange - SetupSuccessfulProcessing(); - SetupFileExists(false); - SetupDirectoryCreate(); - SetupPathCombine(); - // No file move setup - file doesn't exist - - BatchOrchestrator sut = CreateSut(); - - // Act - await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - _logCollector.GetSnapshot() - .Should().Contain(l => - l.Level == LogLevel.Warning && - l.Message.Contains("no longer exists", StringComparison.OrdinalIgnoreCase)); - } - - #endregion - - #region Constants - - private const string TestFilePath = "/batch/input/report.xml.processing"; - private const string OriginalFileName = "report.xml"; - private const int ProcessedCount = 5; - private const int SkippedCount = 2; - - #endregion - - #region Fields - - private readonly MockRepository _mocks = new(MockBehavior.Strict) { DefaultValue = DefaultValue.Empty }; - private readonly Mock _fs; - private readonly Mock _processor; - private readonly Mock _time = new(); - private readonly FakeLogCollector _logCollector = new(); - private readonly FakeLogger _logger; - - #endregion - - #region Helper Methods - - private BatchOrchestrator CreateSut() - { - _time.Setup(t => t.GetUtcNow()).Returns(TimeProvider.System.GetUtcNow()); - return new BatchOrchestrator( - Options.Create(CreateOptions()), - _fs.Object, - _time.Object, - _processor.Object, - _logger); - } - - private void SetupFileExists(bool exists = true) => - _fs.Setup(f => f.File.Exists(TestFilePath)).Returns(exists); - - private void SetupDirectoryCreate() => - _fs.Setup(f => f.DirectoryInfo.New(It.IsAny()).Create()); - - private void SetupPathCombine() => - _fs.Setup(f => f.Path.Combine(It.IsAny(), It.IsAny())) - .Returns((string a, string b) => $"{a}/{b}"); - - private void SetupFileMove(string destinationDir) => - _fs.Setup(f => f.File.Move( - TestFilePath, - It.Is(s => s.Contains(destinationDir)))); - - private void SetupFileMoveThrows(string destinationDir, Exception exception) => - _fs.Setup(f => f.File.Move( - TestFilePath, - It.Is(s => s.Contains(destinationDir)))) - .Throws(exception); - - private void SetupSuccessfulProcessing(int processed = ProcessedCount, int skipped = SkippedCount) => - _processor.Setup(p => p.ProcessAsync(TestFilePath, It.IsAny())) - .ReturnsAsync(new ProcessingResult(processed, skipped)); - - private void SetupFailedProcessing(string errorCode, string errorMessage) => - _processor.Setup(p => p.ProcessAsync(TestFilePath, It.IsAny())) - .ReturnsAsync(Error.Validation(errorCode, errorMessage)); - - #endregion - - #region Tests - Success Path - - [Fact] - public async Task WhenProcessorSucceeds_ReturnsTrue() - { - // Arrange - SetupSuccessfulProcessing(); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMove(ArchivePath); - - BatchOrchestrator sut = CreateSut(); - - // Act - bool result = await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - result.Should().BeTrue(); - } - - [Fact] - public async Task WhenProcessorSucceeds_MovesFileToArchive() - { - // Arrange - SetupSuccessfulProcessing(); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMove(ArchivePath); - - BatchOrchestrator sut = CreateSut(); - - // Act - await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - Move is verified via mock verification in Dispose - } - - [Fact] - public async Task WhenProcessorSucceeds_LogsSuccess() - { - // Arrange - SetupSuccessfulProcessing(); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMove(ArchivePath); - - BatchOrchestrator sut = CreateSut(); - - // Act - await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - _logCollector.GetSnapshot() - .Should().Contain(l => - l.Level == LogLevel.Information && - l.Message.Contains("Successfully processed", StringComparison.OrdinalIgnoreCase) && - l.Message.Contains(OriginalFileName, StringComparison.OrdinalIgnoreCase)); - } - - #endregion - - #region Tests - Failure Path - - [Fact] - public async Task WhenProcessorFails_ReturnsFalse() - { - // Arrange - SetupFailedProcessing("Report.InvalidXml", "XML parsing failed"); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMove(ErrorPath); - - BatchOrchestrator sut = CreateSut(); - - // Act - bool result = await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - result.Should().BeFalse(); - } - - [Fact] - public async Task WhenProcessorFails_MovesFileToErrorDirectory() - { - // Arrange - SetupFailedProcessing("Report.InvalidXml", "XML parsing failed"); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMove(ErrorPath); - - BatchOrchestrator sut = CreateSut(); - - // Act - await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - Move to error path verified via mock in Dispose - } - - [Fact] - public async Task WhenProcessorFails_LogsError() - { - // Arrange - SetupFailedProcessing("Report.InvalidXml", "XML parsing failed"); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMove(ErrorPath); - - BatchOrchestrator sut = CreateSut(); - - // Act - await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - _logCollector.GetSnapshot() - .Should().Contain(l => - l.Level == LogLevel.Error && - l.Message.Contains("quarantined", StringComparison.OrdinalIgnoreCase) && - l.Message.Contains("Report.InvalidXml", StringComparison.OrdinalIgnoreCase)); - } - - #endregion - - #region Tests - Move File Exceptions (MoveFileOrThrow branch) - - [Fact] - public async Task WhenFileMoveThrows_ThrowsIOExceptionWithInfrastructureMessage() - { - // Arrange - Simulate file system error during move - SetupSuccessfulProcessing(); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMoveThrows(ArchivePath, new UnauthorizedAccessException("Access denied")); - - BatchOrchestrator sut = CreateSut(); - - // Act - Func act = () => sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - - // Assert - MoveFileOrThrow catches and rethrows as IOException - IOException thrown = (await act.Should().ThrowAsync()).Which; - thrown.Message.Should().Contain("Infrastructure error moving file"); - thrown.InnerException.Should().BeOfType(); - } - - [Fact] - public async Task WhenFileMoveThrows_LogsError() - { - // Arrange - SetupSuccessfulProcessing(); - SetupFileExists(); - SetupDirectoryCreate(); - SetupPathCombine(); - SetupFileMoveThrows(ArchivePath, new UnauthorizedAccessException("Access denied")); - - BatchOrchestrator sut = CreateSut(); - - // Act - try - { - await sut.ProcessFileAsync( - TestFilePath, - TestContext.Current.CancellationToken); - } - catch (IOException) - { - // Expected - swallow for log verification - } - - // Assert - Should log error before rethrowing - _logCollector.GetSnapshot() - .Should().Contain(l => - l.Level == LogLevel.Error && - l.Message.Contains("Infrastructure error", StringComparison.OrdinalIgnoreCase) && - l.Message.Contains("Hangfire will retry", StringComparison.OrdinalIgnoreCase)); - } - - #endregion - } - #region Constants private const string InputPath = "/batch/input"; diff --git a/PaperlessREST.Tests/Unit/DocumentSearchServiceTests.cs b/PaperlessREST.Tests/Unit/DocumentSearchServiceTests.cs deleted file mode 100644 index 8d679e7..0000000 --- a/PaperlessREST.Tests/Unit/DocumentSearchServiceTests.cs +++ /dev/null @@ -1,97 +0,0 @@ -namespace PaperlessREST.Tests.Unit; - -public sealed class DocumentSearchServiceTests : IDisposable -{ - private const string QueryInvoice = "invoice"; - private const string QueryNonexistent = "nonexistent"; - private const int LimitTen = 10; - private const int LimitFive = 5; - private const int ExpectedCountTwo = 2; - - private readonly MockRepository _mocks = new(MockBehavior.Strict) { DefaultValue = DefaultValue.Empty }; - private readonly Mock _searchService; - - public DocumentSearchServiceTests() - { - _searchService = _mocks.Create(); - } - - public void Dispose() - { - _mocks.VerifyAll(); - _mocks.VerifyNoOtherCalls(); - } - - [Fact] - public async Task SearchAsync_ReturnsExpectedDocuments() - { - // Arrange - Document doc1 = new DocumentBuilder().Build(); - Document doc2 = new DocumentBuilder().Build(); - - _searchService - .Setup(s => s.SearchAsync(QueryInvoice, LimitTen, TestContext.Current.CancellationToken)) - .Returns(new[] { doc1, doc2 }.ToAsyncEnumerable()); - - // Act - List results = []; - await foreach (Document doc in _searchService.Object.SearchAsync(QueryInvoice, LimitTen, - TestContext.Current.CancellationToken)) - { - results.Add(doc); - } - - // Assert - results.Should().HaveCount(ExpectedCountTwo); - results[0].Should().BeSameAs(doc1); - results[1].Should().BeSameAs(doc2); - } - - [Fact] - public async Task SearchAsync_WithEmptyResults_ReturnsEmptySequence() - { - // Arrange - _searchService.Setup(s => - s.SearchAsync(QueryNonexistent, LimitFive, TestContext.Current.CancellationToken)) - .Returns(Array.Empty().ToAsyncEnumerable()); - - // Act - List results = []; - await foreach (Document doc in _searchService.Object.SearchAsync(QueryNonexistent, LimitFive, - TestContext.Current.CancellationToken)) - { - results.Add(doc); - } - - // Assert - results.Should().BeEmpty(); - } - - [Fact] - public async Task DeleteAsync_ReturnsTrue_WhenSuccessful() - { - // Arrange - Guid id = Guid.CreateVersion7(); - _searchService.Setup(s => s.DeleteAsync(id, TestContext.Current.CancellationToken)).ReturnsAsync(true); - - // Act - bool result = await _searchService.Object.DeleteAsync(id, TestContext.Current.CancellationToken); - - // Assert - result.Should().BeTrue(); - } - - [Fact] - public async Task DeleteAsync_ReturnsFalse_WhenUnsuccessful() - { - // Arrange - Guid id = Guid.CreateVersion7(); - _searchService.Setup(s => s.DeleteAsync(id, TestContext.Current.CancellationToken)).ReturnsAsync(false); - - // Act - bool result = await _searchService.Object.DeleteAsync(id, TestContext.Current.CancellationToken); - - // Assert - result.Should().BeFalse(); - } -} diff --git a/PaperlessREST.Tests/Unit/DocumentServiceContractTests.cs b/PaperlessREST.Tests/Unit/DocumentServiceContractTests.cs index 252b314..fb16016 100644 --- a/PaperlessREST.Tests/Unit/DocumentServiceContractTests.cs +++ b/PaperlessREST.Tests/Unit/DocumentServiceContractTests.cs @@ -1,5 +1,7 @@ using System.Net.Sockets; +using System.Text; using AwesomeAssertions.Execution; +using Minio.Exceptions; namespace PaperlessREST.Tests.Unit; @@ -35,12 +37,14 @@ public async Task UploadDocumentAsync_ValidPdf_CreatesExactDocumentUploadsExactO string? pathGivenToStorage = null; long lengthGivenToStorage = -1; CancellationToken tokenGivenToStorage = default; + Stream? streamGivenToStorage = null; string? routingKeyGivenToPublisher = null; OcrCommand? commandGivenToPublisher = null; Storage.Setup(s => s.UploadAsync(It.IsAny(), It.IsAny(), FileSize, ct)) .Callback((stream, path, length, token) => { + streamGivenToStorage = stream; pathGivenToStorage = path; lengthGivenToStorage = length; tokenGivenToStorage = token; @@ -81,6 +85,8 @@ public async Task UploadDocumentAsync_ValidPdf_CreatesExactDocumentUploadsExactO pathGivenToStorage.Should().Be(expectedStoragePath); lengthGivenToStorage.Should().Be(FileSize); tokenGivenToStorage.Should().Be(ct); + streamGivenToStorage.Should().NotBeNull(); + streamGivenToStorage!.CanRead.Should().BeFalse("DocumentService owns the opened upload stream"); routingKeyGivenToPublisher.Should().NotBeNullOrWhiteSpace(); commandGivenToPublisher.Should().BeEquivalentTo( @@ -89,64 +95,108 @@ public async Task UploadDocumentAsync_ValidPdf_CreatesExactDocumentUploadsExactO ShouldHaveLog(LogLevel.Information, "uploaded successfully", saved.Id.ToString()); } - public static IEnumerable> KnownStorageFailures() + [Fact] + public async Task UploadDocumentAsync_MinIoNetworkFailure_ReturnsRetriableConnectionError() { - yield return new TheoryDataRow( - new TimeoutException("storage timed out"), "Document.StorageTimeout") - .WithTestDisplayName("TimeoutException => StorageTimeout (503 + retryAfter)"); - - yield return new TheoryDataRow( - new HttpRequestException("storage unavailable", null, HttpStatusCode.ServiceUnavailable), - "Document.StorageServerError") - .WithTestDisplayName("HttpRequestException 5xx => StorageServerError (503 + retryAfter)"); - - yield return new TheoryDataRow( - new IOException("socket failed", new SocketException((int)SocketError.ConnectionRefused)), - "Document.StorageConnectionFailed") - .WithTestDisplayName("IOException(SocketException) => StorageConnectionFailed (503 + retryAfter)"); + using StubHttpMessageHandler handler = new((_, _) => + Task.FromException(new HttpRequestException( + "connection refused", + new SocketException((int)SocketError.ConnectionRefused)))); + using HttpClient http = new(handler, disposeHandler: false); + using MinioClient minio = new(); + IMinioClient client = ConfigureMinioClient(minio, http); + + ErrorOr result = await UploadThroughMinioAsync(client, TestContext.Current.CancellationToken); + + AssertRetriableStorageError( + result, + "Document.StorageConnectionFailed", + "The storage service could not be reached."); } - [Theory] - [MemberData(nameof(KnownStorageFailures))] - public async Task UploadDocumentAsync_KnownStorageFailure_ReturnsRetriableErrorAndDoesNotPersistOrPublish( - Exception storageException, string expectedCode) + [Fact] + public async Task UploadDocumentAsync_MinIoRequestTimeout_ReturnsRetriableTimeoutError() { - CancellationToken ct = TestContext.Current.CancellationToken; - UploadDocumentRequest request = UploadDocumentRequestBuilder.ValidPdf() - .WithFileName(FileName) - .WithFileSize(FileSize) - .Build(); - - Storage.Setup(s => s.UploadAsync(It.IsAny(), It.IsAny(), FileSize, ct)) - .ThrowsAsync(storageException); - - ErrorOr result = await CreateSut().UploadDocumentAsync(request, ct); + using StubHttpMessageHandler handler = new( + async (_, cancellationToken) => + { + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + return new HttpResponseMessage(HttpStatusCode.OK); + }); + using HttpClient http = new(handler, disposeHandler: false); + using MinioClient minio = new(); + IMinioClient client = ConfigureMinioClient(minio, http, requestTimeoutMilliseconds: 25); + + ErrorOr result = await UploadThroughMinioAsync(client, CancellationToken.None); + + AssertRetriableStorageError( + result, + "Document.StorageTimeout", + "The storage operation timed out."); + } - using AssertionScope _ = new(); - result.IsError.Should().BeTrue(); - ((int)result.FirstError.Type).Should().Be(503); - result.FirstError.Code.Should().Be(expectedCode); - result.FirstError.Description.Should().Contain("documents/2026-06/"); - result.FirstError.Metadata.Should().ContainKey("retryAfter").WhoseValue.Should().Be(30); - ShouldHaveLog(LogLevel.Warning, "Storage error", expectedCode); - // No repository/publisher setup is intentional: strict mocks prove nothing was persisted or published. + [Fact] + public async Task UploadDocumentAsync_MinIoTransientXmlError_ReturnsRetriableServerError() + { + using StubHttpMessageHandler handler = new((_, _) => Task.FromResult(new HttpResponseMessage( + HttpStatusCode.InternalServerError) + { + Content = new StringContent( + MinioErrorXml("InternalError"), + Encoding.UTF8, + "application/xml") + })); + using HttpClient http = new(handler, disposeHandler: false); + using MinioClient minio = new(); + IMinioClient client = ConfigureMinioClient(minio, http); + + ErrorOr result = await UploadThroughMinioAsync(client, TestContext.Current.CancellationToken); + + AssertRetriableStorageError( + result, + "Document.StorageServerError", + "The storage service is temporarily unavailable."); } [Fact] - public async Task UploadDocumentAsync_UnknownStorageFailure_PropagatesOriginalAndDoesNotPersistOrPublish() + public async Task UploadDocumentAsync_MinIoCallerCancellation_PropagatesExactToken() { - CancellationToken ct = TestContext.Current.CancellationToken; - UploadDocumentRequest request = UploadDocumentRequestBuilder.ValidPdf().WithFileName(FileName).Build(); - InvalidOperationException expected = new("bug outside the mapped storage failure set"); + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + using StubHttpMessageHandler handler = new((_, token) => + { + token.ThrowIfCancellationRequested(); + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK)); + }); + using HttpClient http = new(handler, disposeHandler: false); + using MinioClient minio = new(); + IMinioClient client = ConfigureMinioClient(minio, http); - Storage.Setup(s => s.UploadAsync(It.IsAny(), It.IsAny(), It.IsAny(), ct)) - .ThrowsAsync(expected); + Func act = async () => await UploadThroughMinioAsync(client, cancellation.Token); - Func act = () => CreateSut().UploadDocumentAsync(request, ct); + OperationCanceledException thrown = (await act.Should().ThrowAsync()).Which; + thrown.CancellationToken.Should().Be(cancellation.Token); + } - InvalidOperationException thrown = (await act.Should().ThrowAsync()).Which; - thrown.Should().BeSameAs(expected); - // No repository/publisher setup is intentional: strict mocks prove nothing was persisted or published. + [Fact] + public async Task UploadDocumentAsync_MinIoPermanentError_PropagatesSdkException() + { + using StubHttpMessageHandler handler = new((_, _) => Task.FromResult(new HttpResponseMessage( + HttpStatusCode.Forbidden) + { + Content = new StringContent( + MinioErrorXml("InvalidAccessKeyId"), + Encoding.UTF8, + "application/xml") + })); + using HttpClient http = new(handler, disposeHandler: false); + using MinioClient minio = new(); + IMinioClient client = ConfigureMinioClient(minio, http); + + Func act = async () => + await UploadThroughMinioAsync(client, TestContext.Current.CancellationToken); + + await act.Should().ThrowAsync(); } // ── ProcessOcrResultAsync ───────────────────────────────────────────── @@ -302,25 +352,86 @@ public async Task UpdateDocumentSummaryAsync_RepositoryUpdatesNoRows_ReturnsNotF // ── DeleteDocumentAsync ─────────────────────────────────────────────── [Fact] - public async Task DeleteDocumentAsync_DocumentExists_DeletesRepositoryAndStorageThenTreatsSearchDeleteAsBestEffort() + public async Task DeleteDocumentAsync_DocumentExists_DeletesSearchStorageAndRepository() { CancellationToken ct = TestContext.Current.CancellationToken; Document document = new DocumentBuilder().Build(); Repository.Setup(r => r.GetByIdAsync(document.Id, ct)).ReturnsAsync(document); + Search.Setup(s => s.DeleteAsync(document.Id, ct)).Returns(Task.CompletedTask); + Storage.Setup(s => s.DeleteAsync(document.StoragePath, ct)).Returns(Task.CompletedTask); Repository.Setup(r => r.DeleteAsync(document.Id, ct)).ReturnsAsync(true); - Storage.Setup(s => s.DeleteAsync(document.StoragePath, ct)).ReturnsAsync(true); - Search.Setup(s => s.DeleteAsync(document.Id, ct)).ThrowsAsync(new InvalidOperationException("search down")); ErrorOr result = await CreateSut().DeleteDocumentAsync(document.Id, ct); using AssertionScope _ = new(); result.IsError.Should().BeFalse(); result.Value.Should().Be(Result.Deleted); - ShouldHaveLog(LogLevel.Warning, "search index", document.Id.ToString()); ShouldHaveLog(LogLevel.Information, "deleted successfully", document.Id.ToString()); } + public static IEnumerable> SearchDeletionFailures() + { + yield return new TheoryDataRow(new InvalidOperationException("search down")) + .WithTestDisplayName("unexpected search failure"); + yield return new TheoryDataRow(new OperationCanceledException("search canceled")) + .WithTestDisplayName("search cancellation"); + } + + [Theory] + [MemberData(nameof(SearchDeletionFailures))] + public async Task DeleteDocumentAsync_SearchFailure_PropagatesAndLeavesStorageAndRepositoryForRetry( + Exception expected) + { + CancellationToken ct = TestContext.Current.CancellationToken; + Document document = new DocumentBuilder().Build(); + + Repository.Setup(r => r.GetByIdAsync(document.Id, ct)).ReturnsAsync(document); + Search.Setup(s => s.DeleteAsync(document.Id, ct)).ThrowsAsync(expected); + + Func act = () => CreateSut().DeleteDocumentAsync(document.Id, ct); + + Exception thrown = (await act.Should().ThrowAsync()).Which; + thrown.Should().BeSameAs(expected); + } + + [Fact] + public async Task DeleteDocumentAsync_StorageFailure_PropagatesAndLeavesRepositoryForRetry() + { + CancellationToken ct = TestContext.Current.CancellationToken; + Document document = new DocumentBuilder().Build(); + InvalidOperationException expected = new("storage down"); + + Repository.Setup(r => r.GetByIdAsync(document.Id, ct)).ReturnsAsync(document); + Search.Setup(s => s.DeleteAsync(document.Id, ct)).Returns(Task.CompletedTask); + Storage.Setup(s => s.DeleteAsync(document.StoragePath, ct)).ThrowsAsync(expected); + + Func act = () => CreateSut().DeleteDocumentAsync(document.Id, ct); + + InvalidOperationException thrown = (await act.Should().ThrowAsync()).Which; + thrown.Should().BeSameAs(expected); + } + + [Fact] + public async Task DeleteDocumentAsync_StorageCancellation_PropagatesCallerTokenAndLeavesRepositoryForRetry() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + CancellationToken ct = cancellation.Token; + Document document = new DocumentBuilder().Build(); + OperationCanceledException expected = new(ct); + + Repository.Setup(r => r.GetByIdAsync(document.Id, ct)).ReturnsAsync(document); + Search.Setup(s => s.DeleteAsync(document.Id, ct)).Returns(Task.CompletedTask); + Storage.Setup(s => s.DeleteAsync(document.StoragePath, ct)).ThrowsAsync(expected); + + Func act = () => CreateSut().DeleteDocumentAsync(document.Id, ct); + + OperationCanceledException thrown = (await act.Should().ThrowAsync()).Which; + thrown.Should().BeSameAs(expected); + thrown.CancellationToken.Should().Be(ct); + } + [Fact] public async Task DeleteDocumentAsync_MissingDocument_ReturnsNotFoundAndDoesNotTouchStorageOrSearch() { @@ -402,7 +513,7 @@ public async Task GetDocumentsPagedAsync_ForwardsExactPageSizeCursorAndToken_And // ── SearchDocumentsAsync ────────────────────────────────────────────── [Fact] - public async Task SearchDocumentsAsync_ForwardsExactQueryLimitAndToken_AndStreamsResults() + public async Task SearchDocumentsAsync_ForwardsExactQueryLimitAndToken_AndReturnsResults() { CancellationToken ct = TestContext.Current.CancellationToken; DocumentSearchResult[] expectedResults = @@ -419,15 +530,86 @@ public async Task SearchDocumentsAsync_ForwardsExactQueryLimitAndToken_AndStream } ]; - Search.Setup(s => s.SearchAsync("invoice", 25, ct)) - .Returns(expectedResults.ToAsyncEnumerable()); + Search.Setup(s => s.SearchAsync("invoice", 25, ct)) + .ReturnsAsync(expectedResults); - List actual = []; - await foreach (DocumentSearchResult result in CreateSut().SearchDocumentsAsync("invoice", 25, ct)) - { - actual.Add(result); - } + IReadOnlyCollection actual = + await CreateSut().SearchDocumentsAsync("invoice", 25, ct); actual.Should().Equal(expectedResults); } + + private async Task> UploadThroughMinioAsync( + IMinioClient minio, + CancellationToken cancellationToken) + { + DocumentStorageService storage = new( + minio, + Options.Create(new MinioOptions + { + Endpoint = new Uri("http://minio.test:9000"), + AccessKey = "access-key", + SecretKey = "secret-key", + BucketName = "test-bucket" + }), + NullLogger.Instance); + UploadDocumentRequest request = UploadDocumentRequestBuilder.ValidPdf() + .WithFileName(FileName) + .WithFileSize(FileSize) + .Build(); + + return await CreateSut(storage).UploadDocumentAsync(request, cancellationToken); + } + + private void AssertRetriableStorageError( + ErrorOr result, + string expectedCode, + string expectedDescription) + { + using AssertionScope _ = new(); + result.IsError.Should().BeTrue(); + ((int)result.FirstError.Type).Should().Be(503); + result.FirstError.Code.Should().Be(expectedCode); + result.FirstError.Description.Should().Be(expectedDescription); + result.FirstError.Description.Should().NotContain("documents/"); + result.FirstError.Metadata.Should().ContainKey("retryAfter").WhoseValue.Should().Be(30); + ShouldHaveLog(LogLevel.Warning, "Storage error", expectedCode, "documents/2026-06/"); + } + + private static IMinioClient ConfigureMinioClient( + MinioClient minio, + HttpClient http, + int requestTimeoutMilliseconds = 0) + { + IMinioClient configured = minio + .WithEndpoint("minio.test", 9000) + .WithCredentials("access-key", "secret-key") + .WithRegion("us-east-1") + .WithHttpClient(http) + .Build(); + + return requestTimeoutMilliseconds > 0 + ? configured.WithTimeout(requestTimeoutMilliseconds) + : configured; + } + + private static string MinioErrorXml(string code) => $$""" + + {{code}} + storage request failed + /test-bucket/document.pdf + test-bucket + test-request + test-host + + """; + + private sealed class StubHttpMessageHandler( + Func> sendAsync) : HttpMessageHandler + { + protected override Task SendAsync( + HttpRequestMessage request, + CancellationToken cancellationToken) => + sendAsync(request, cancellationToken); + } } diff --git a/PaperlessREST.Tests/Unit/DocumentServiceTestBase.cs b/PaperlessREST.Tests/Unit/DocumentServiceTestBase.cs index 111d827..e9375ec 100644 --- a/PaperlessREST.Tests/Unit/DocumentServiceTestBase.cs +++ b/PaperlessREST.Tests/Unit/DocumentServiceTestBase.cs @@ -43,8 +43,8 @@ public void Dispose() _mocks.VerifyNoOtherCalls(); } - protected DocumentService CreateSut() => - new(Repository.Object, Storage.Object, Search.Object, Publisher.Object, Clock, Logger); + protected DocumentService CreateSut(IDocumentStorageService? storage = null) => + new(Repository.Object, storage ?? Storage.Object, Search.Object, Publisher.Object, Clock, Logger); /// Asserts exactly one log entry at whose message contains every fragment. protected void ShouldHaveLog(LogLevel level, params string[] fragments) => diff --git a/PaperlessREST.Tests/Unit/DocumentStorageServiceTests.cs b/PaperlessREST.Tests/Unit/DocumentStorageServiceTests.cs index 28f9dc0..6a19fb1 100644 --- a/PaperlessREST.Tests/Unit/DocumentStorageServiceTests.cs +++ b/PaperlessREST.Tests/Unit/DocumentStorageServiceTests.cs @@ -2,7 +2,7 @@ namespace PaperlessREST.Tests.Unit; public sealed class DocumentStorageServiceTests : IDisposable { - private const string TestEndpoint = "localhost:9000"; + private const string TestEndpoint = "http://localhost:9000"; private const string TestAccessKey = "minioadmin"; private const string TestSecretKey = "minioadmin"; private const string TestBucketName = "test-bucket"; @@ -18,7 +18,7 @@ public DocumentStorageServiceTests() _minioClient = _mocks.Create(); _options = Options.Create(new MinioOptions { - Endpoint = TestEndpoint, + Endpoint = new Uri(TestEndpoint), AccessKey = TestAccessKey, SecretKey = TestSecretKey, BucketName = TestBucketName @@ -52,7 +52,7 @@ public async Task UploadAsync_PutsObjectOnce() } [Fact] - public async Task DeleteAsync_WhenMinioSucceeds_ReturnsTrue() + public async Task DeleteAsync_WhenMinioSucceeds_Completes() { // Arrange _minioClient.Setup(m => m.RemoveObjectAsync(It.IsAny(), It.IsAny())) @@ -60,31 +60,41 @@ public async Task DeleteAsync_WhenMinioSucceeds_ReturnsTrue() IDocumentStorageService sut = CreateSut(); - // Act - bool ok = await sut.DeleteAsync(ValidStoragePath, TestContext.Current.CancellationToken); + await sut.DeleteAsync(ValidStoragePath, TestContext.Current.CancellationToken); - // Assert - ok.Should().BeTrue(); _minioClient.Verify(m => m.RemoveObjectAsync(It.IsAny(), It.IsAny()), Times.Once); } [Fact] - public async Task DeleteAsync_WhenMinioThrows_ReturnsFalse() + public async Task DeleteAsync_WhenMinioThrows_PropagatesOriginalException() { - // Arrange + InvalidOperationException expected = new("boom"); _minioClient.Setup(m => m.RemoveObjectAsync(It.IsAny(), It.IsAny())) - .ThrowsAsync(new InvalidOperationException("boom")); + .ThrowsAsync(expected); IDocumentStorageService sut = CreateSut(); + Func act = () => sut.DeleteAsync(ValidStoragePath, TestContext.Current.CancellationToken); - // Act - bool ok = await sut.DeleteAsync(ValidStoragePath, TestContext.Current.CancellationToken); + InvalidOperationException thrown = (await act.Should().ThrowAsync()).Which; + thrown.Should().BeSameAs(expected); + } - // Assert - ok.Should().BeFalse(); - _minioClient.Verify(m => m.RemoveObjectAsync(It.IsAny(), It.IsAny()), - Times.Once); + [Fact] + public async Task DeleteAsync_WhenMinioCancels_PropagatesOriginalToken() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + OperationCanceledException expected = new(cancellation.Token); + _minioClient.Setup(m => m.RemoveObjectAsync(It.IsAny(), cancellation.Token)) + .ThrowsAsync(expected); + + IDocumentStorageService sut = CreateSut(); + Func act = () => sut.DeleteAsync(ValidStoragePath, cancellation.Token); + + OperationCanceledException thrown = (await act.Should().ThrowAsync()).Which; + thrown.Should().BeSameAs(expected); + thrown.CancellationToken.Should().Be(cancellation.Token); } private IDocumentStorageService CreateSut() => diff --git a/PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs b/PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs deleted file mode 100644 index ac66ebd..0000000 --- a/PaperlessREST.Tests/Unit/ExceptionHandlerTests.cs +++ /dev/null @@ -1,313 +0,0 @@ -namespace PaperlessREST.Tests.Unit; - -public static class ExceptionHandlerConstants -{ - public const string UrnPrefix = "urn:paperless:error:"; - public const string BadRequestCode = "bad_request"; - public const string ValidationErrorCode = "validation_error"; - public const string NotFoundCode = "not_found"; - public const string TestExceptionMessage = "Test"; - public const string PropertyName = "Name"; - public const string NameRequiredError = "Name is required"; -} - -public sealed class ExceptionHandlerSetup -{ - public ExceptionHandlerSetup() - { - Collector = new FakeLogCollector(); - Logger = new FakeLogger(Collector); - } - - public Mock ProblemDetails { get; } = new(); - public FakeLogger Logger { get; } - private FakeLogCollector Collector { get; } - - public GlobalExceptionHandler CreateHandler() => new(ProblemDetails.Object, Logger); - - public HttpContext CreateHttpContext() - { - DefaultHttpContext context = new() { Response = { Body = new MemoryStream() } }; - return context; - } - - public ExceptionHandlerSetup WithProblemDetailsWrite(bool success = true) - { - ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => - ctx.HttpContext.Response.StatusCode = ctx.ProblemDetails.Status ?? 500) - .ReturnsAsync(success); - return this; - } -} - -public sealed class ExceptionHandlerTests -{ - private readonly ExceptionHandlerSetup _setup = new(); - - public static IEnumerable> ExceptionCases() - { - // ArgumentNullException derives from ArgumentException, both map to "bad_request" - yield return new TheoryDataRow(typeof(ArgumentNullException), 400, - LogLevel.Warning, "bad_request") - .WithTestDisplayName("400 ArgumentNull"); - yield return new TheoryDataRow(typeof(ArgumentException), 400, LogLevel.Warning, - "bad_request") - .WithTestDisplayName("400 Argument"); - yield return new TheoryDataRow(typeof(InvalidOperationException), 400, - LogLevel.Warning, "bad_request") - .WithTestDisplayName("400 InvalidOperation"); - yield return new TheoryDataRow(typeof(JsonException), 400, LogLevel.Warning, - "bad_request") - .WithTestDisplayName("400 Json"); - yield return new TheoryDataRow(typeof(UnauthorizedAccessException), 403, - LogLevel.Warning, "forbidden") - .WithTestDisplayName("403 Unauthorized"); - yield return new TheoryDataRow(typeof(KeyNotFoundException), 404, - LogLevel.Information, "not_found") - .WithTestDisplayName("404 KeyNotFound"); - yield return new TheoryDataRow(typeof(FileNotFoundException), 404, - LogLevel.Information, "not_found") - .WithTestDisplayName("404 FileNotFound"); - yield return new TheoryDataRow(typeof(TimeoutException), 504, LogLevel.Error, - "timeout") - .WithTestDisplayName("504 Timeout"); - yield return new TheoryDataRow(typeof(Exception), 500, LogLevel.Error, - "internal_error") - .WithTestDisplayName("500 Exception"); - } - - [Theory] - [MemberData(nameof(ExceptionCases))] - public async Task TryHandleAsync_VariousExceptions_ReturnsExpectedStatusAndLogs( - Type exceptionType, int expectedStatus, LogLevel expectedLevel, string expectedCode) - { - var context = _setup.CreateHttpContext(); - var exception = (Exception)Activator.CreateInstance(exceptionType, "Test")!; - var handler = _setup.WithProblemDetailsWrite().CreateHandler(); - - var handled = await handler.TryHandleAsync(context, exception, TestContext.Current.CancellationToken); - - handled.Should().BeTrue(); - context.Response.StatusCode.Should().Be(expectedStatus); - _setup.Logger.Collector.GetSnapshot().Should().Contain(x => - x.Level == expectedLevel && x.Message.Contains(expectedCode, StringComparison.Ordinal)); - } - - [Fact] - public async Task TryHandleAsync_OperationCancelled_Returns499() - { - var context = _setup.CreateHttpContext(); - OperationCanceledException exception = new(); - CancellationToken token = new(true); - var handler = _setup.WithProblemDetailsWrite(false).CreateHandler(); - - var handled = await handler.TryHandleAsync(context, exception, token); - - handled.Should().BeTrue(); - context.Response.StatusCode.Should().Be(499); - // Handler logs at Debug level for cancelled requests - _setup.Logger.Collector.GetSnapshot() - .Should().OnlyContain(l => l.Level == LogLevel.Debug && l.Message.Contains("Request cancelled")); - } - - [Fact] - public async Task TryHandleAsync_OperationCancelledWithoutCancelledToken_TreatsAsInternalError() - { - // Arrange - OperationCanceledException but token is NOT cancelled - var context = _setup.CreateHttpContext(); - OperationCanceledException exception = new("Task was cancelled"); - var handler = _setup.WithProblemDetailsWrite().CreateHandler(); - - // Act - Use non-cancelled token - var handled = await handler.TryHandleAsync(context, exception, CancellationToken.None); - - // Assert - Should NOT take the early return path, but go through normal exception handling - handled.Should().BeTrue(); - // OperationCanceledException without cancelled token maps to 499 via ExceptionInfo - context.Response.StatusCode.Should().Be(499); - } - - [Fact] - public async Task TryHandleAsync_ValidationException_Returns400WithErrors() - { - // Arrange - var context = _setup.CreateHttpContext(); - ValidationException validationEx = new(new ValidationResult("Name is required", ["Name"]), null, null); - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => - { - ctx.HttpContext.Response.StatusCode = ctx.ProblemDetails.Status ?? 400; - // Verify it's an HttpValidationProblemDetails - ctx.ProblemDetails.Should().BeOfType(); - }) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - var handled = await handler.TryHandleAsync(context, validationEx, CancellationToken.None); - - // Assert - handled.Should().BeTrue(); - context.Response.StatusCode.Should().Be(400); - _setup.Logger.Collector.GetSnapshot() - .Should().Contain(l => l.Level == LogLevel.Information && l.Message.Contains("validation_error")); - } - - [Fact] - public async Task TryHandleAsync_BadHttpRequestException_Returns400() - { - // Arrange - var context = _setup.CreateHttpContext(); - BadHttpRequestException exception = new("Bad request body"); - var handler = _setup.WithProblemDetailsWrite().CreateHandler(); - - // Act - var handled = await handler.TryHandleAsync(context, exception, CancellationToken.None); - - // Assert - handled.Should().BeTrue(); - context.Response.StatusCode.Should().Be(400); - _setup.Logger.Collector.GetSnapshot() - .Should().Contain(l => l.Level == LogLevel.Warning && l.Message.Contains("bad_request")); - } - - [Fact] - public async Task TryHandleAsync_ArgumentException_PassesCorrectTypeUrn() - { - // Arrange - var context = _setup.CreateHttpContext(); - ArgumentException exception = new(ExceptionHandlerConstants.TestExceptionMessage); - ProblemDetailsContext? capturedContext = null; - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => capturedContext = ctx) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - await handler.TryHandleAsync(context, exception, TestContext.Current.CancellationToken); - - // Assert - capturedContext.Should().NotBeNull(); - capturedContext!.ProblemDetails.Type.Should().Be( - ExceptionHandlerConstants.UrnPrefix + ExceptionHandlerConstants.BadRequestCode); - } - - [Fact] - public async Task TryHandleAsync_KeyNotFoundException_PassesCorrectTypeUrn() - { - // Arrange - var context = _setup.CreateHttpContext(); - KeyNotFoundException exception = new(ExceptionHandlerConstants.TestExceptionMessage); - ProblemDetailsContext? capturedContext = null; - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => capturedContext = ctx) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - await handler.TryHandleAsync(context, exception, TestContext.Current.CancellationToken); - - // Assert - capturedContext.Should().NotBeNull(); - capturedContext!.ProblemDetails.Type.Should().Be( - ExceptionHandlerConstants.UrnPrefix + ExceptionHandlerConstants.NotFoundCode); - } - - [Fact] - public async Task TryHandleAsync_PassesHttpContextToWriter() - { - // Arrange - var context = _setup.CreateHttpContext(); - InvalidOperationException exception = new(ExceptionHandlerConstants.TestExceptionMessage); - ProblemDetailsContext? capturedContext = null; - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => capturedContext = ctx) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - await handler.TryHandleAsync(context, exception, TestContext.Current.CancellationToken); - - // Assert - capturedContext.Should().NotBeNull(); - capturedContext!.HttpContext.Should().BeSameAs(context); - } - - [Fact] - public async Task TryHandleAsync_PassesExceptionToWriter() - { - // Arrange - var context = _setup.CreateHttpContext(); - InvalidOperationException exception = new(ExceptionHandlerConstants.TestExceptionMessage); - ProblemDetailsContext? capturedContext = null; - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => capturedContext = ctx) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - await handler.TryHandleAsync(context, exception, TestContext.Current.CancellationToken); - - // Assert - capturedContext.Should().NotBeNull(); - capturedContext!.Exception.Should().BeSameAs(exception); - } - - [Fact] - public async Task TryHandleAsync_NonValidationException_SetsProblemDetailsStatus() - { - // Arrange - var context = _setup.CreateHttpContext(); - TimeoutException exception = new(ExceptionHandlerConstants.TestExceptionMessage); - ProblemDetailsContext? capturedContext = null; - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => capturedContext = ctx) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - await handler.TryHandleAsync(context, exception, TestContext.Current.CancellationToken); - - // Assert - capturedContext.Should().NotBeNull(); - capturedContext!.ProblemDetails.Status.Should().Be(504); - } - - [Fact] - public async Task TryHandleAsync_ValidationException_SetsValidationType() - { - // Arrange - var context = _setup.CreateHttpContext(); - ValidationException validationEx = new(new ValidationResult(ExceptionHandlerConstants.NameRequiredError, [ExceptionHandlerConstants.PropertyName]), null, null); - ProblemDetailsContext? capturedContext = null; - - _setup.ProblemDetails.Setup(p => p.TryWriteAsync(It.IsAny())) - .Callback(ctx => capturedContext = ctx) - .ReturnsAsync(true); - - var handler = _setup.CreateHandler(); - - // Act - await handler.TryHandleAsync(context, validationEx, TestContext.Current.CancellationToken); - - // Assert - capturedContext.Should().NotBeNull(); - capturedContext!.ProblemDetails.Type.Should().Be( - ExceptionHandlerConstants.UrnPrefix + ExceptionHandlerConstants.ValidationErrorCode); - } -} - -// NOTE: ProblemDetailsEnricherTests moved to GlobalExceptionHandlerTests.cs diff --git a/PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs b/PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs index 0dc3d96..3b80a16 100644 --- a/PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs +++ b/PaperlessREST.Tests/Unit/GlobalExceptionHandlerTests.cs @@ -10,18 +10,11 @@ namespace PaperlessREST.Tests.Unit; public sealed class GlobalExceptionHandlerTests : IDisposable { private const int Status400BadRequest = StatusCodes.Status400BadRequest; - private const int Status403Forbidden = StatusCodes.Status403Forbidden; - private const int Status404NotFound = StatusCodes.Status404NotFound; private const int Status499ClientClosedRequest = HttpStatusCodes.ClientClosedRequest; private const int Status500InternalServerError = StatusCodes.Status500InternalServerError; - private const int Status504GatewayTimeout = StatusCodes.Status504GatewayTimeout; private const string CodeValidationError = "validation_error"; private const string CodeBadRequest = "bad_request"; - private const string CodeForbidden = "forbidden"; - private const string CodeNotFound = "not_found"; - private const string CodeCancelled = "cancelled"; - private const string CodeTimeout = "timeout"; private const string CodeInternalError = "internal_error"; private const string TestMessage = "test"; @@ -32,9 +25,8 @@ public sealed class GlobalExceptionHandlerTests : IDisposable private const string FieldName = "Field"; private const string FieldError = "Error"; private const string EmailFieldName = "Email"; + private const string NameFieldName = "Name"; private const string EmailRequiredError = "Required"; - private const string DocNotFoundMessage = "Doc not found"; - private const string NotFoundMessage = "Not found"; private const string BadRequestMessage = "bad request"; private const string TestActivityName = "Test"; private readonly FakeLogCollector _logCollector = new(); @@ -71,34 +63,6 @@ public void FromException_ValidationException_Returns400WithValidationError() info.Code.Should().Be(CodeValidationError); } - [Theory] - [MemberData(nameof(BadRequestExceptions))] - public void FromException_BadRequestExceptions_Returns400(Type exceptionType) - { - // Arrange - var exception = (Exception)Activator.CreateInstance(exceptionType, TestMessage)!; - - // Act - var info = ExceptionInfo.FromException(exception); - - // Assert - info.StatusCode.Should().Be(Status400BadRequest); - info.Level.Should().Be(LogLevel.Warning); - info.Code.Should().Be(CodeBadRequest); - } - - public static IEnumerable BadRequestExceptions() - { - yield return new TheoryDataRow(typeof(ArgumentException)) - .WithTestDisplayName("ArgumentException → 400"); - yield return new TheoryDataRow(typeof(ArgumentNullException)) - .WithTestDisplayName("ArgumentNullException → 400"); - yield return new TheoryDataRow(typeof(InvalidOperationException)) - .WithTestDisplayName("InvalidOperationException → 400"); - yield return new TheoryDataRow(typeof(JsonException)) - .WithTestDisplayName("JsonException → 400"); - } - [Fact] public void FromException_BadHttpRequestException_Returns400() { @@ -110,106 +74,41 @@ public void FromException_BadHttpRequestException_Returns400() // Assert info.StatusCode.Should().Be(Status400BadRequest); - info.Code.Should().Be(CodeBadRequest); - } - - [Fact] - public void FromException_UnauthorizedAccessException_Returns403() - { - // Arrange - UnauthorizedAccessException exception = new(); - - // Act - var info = ExceptionInfo.FromException(exception); - - // Assert - info.StatusCode.Should().Be(Status403Forbidden); info.Level.Should().Be(LogLevel.Warning); - info.Code.Should().Be(CodeForbidden); + info.Code.Should().Be(CodeBadRequest); } [Theory] - [MemberData(nameof(NotFoundExceptions))] - public void FromException_NotFoundExceptions_Returns404(Type exceptionType) + [MemberData(nameof(UnownedExceptions))] + public void FromException_UnownedException_ReturnsSanitized500(Type exceptionType) { // Arrange - var exception = (Exception)Activator.CreateInstance(exceptionType, NotFoundMessage)!; + var exception = (Exception)Activator.CreateInstance(exceptionType, TestMessage)!; // Act var info = ExceptionInfo.FromException(exception); // Assert - info.StatusCode.Should().Be(Status404NotFound); - info.Level.Should().Be(LogLevel.Information); - info.Code.Should().Be(CodeNotFound); + info.StatusCode.Should().Be(Status500InternalServerError); + info.Level.Should().Be(LogLevel.Error); + info.Code.Should().Be(CodeInternalError); } - public static IEnumerable NotFoundExceptions() + public static IEnumerable UnownedExceptions() { + yield return new TheoryDataRow(typeof(ArgumentException)); + yield return new TheoryDataRow(typeof(ArgumentNullException)); + yield return new TheoryDataRow(typeof(InvalidOperationException)); + yield return new TheoryDataRow(typeof(JsonException)); + yield return new TheoryDataRow(typeof(UnauthorizedAccessException)); yield return new TheoryDataRow(typeof(KeyNotFoundException)) - .WithTestDisplayName("KeyNotFoundException → 404"); + .WithTestDisplayName("KeyNotFoundException → 500"); yield return new TheoryDataRow(typeof(FileNotFoundException)) - .WithTestDisplayName("FileNotFoundException → 404"); - } - - [Fact] - public void FromException_OperationCanceledException_Returns499() - { - // Arrange - OperationCanceledException exception = new(); - - // Act - var info = ExceptionInfo.FromException(exception); - - // Assert - info.StatusCode.Should().Be(Status499ClientClosedRequest); - info.Level.Should().Be(LogLevel.Debug); - info.Code.Should().Be(CodeCancelled); - } - - [Fact] - public void FromException_TaskCanceledException_Returns499() - { - // Arrange - TaskCanceledException derives from OperationCanceledException - TaskCanceledException exception = new(); - - // Act - var info = ExceptionInfo.FromException(exception); - - // Assert - info.StatusCode.Should().Be(Status499ClientClosedRequest); - info.Level.Should().Be(LogLevel.Debug); - info.Code.Should().Be(CodeCancelled); - } - - [Fact] - public void FromException_TimeoutException_Returns504() - { - // Arrange - TimeoutException exception = new(); - - // Act - var info = ExceptionInfo.FromException(exception); - - // Assert - info.StatusCode.Should().Be(Status504GatewayTimeout); - info.Level.Should().Be(LogLevel.Error); - info.Code.Should().Be(CodeTimeout); - } - - [Fact] - public void FromException_UnknownException_Returns500() - { - // Arrange - NotSupportedException exception = new(); - - // Act - var info = ExceptionInfo.FromException(exception); - - // Assert - info.StatusCode.Should().Be(Status500InternalServerError); - info.Level.Should().Be(LogLevel.Error); - info.Code.Should().Be(CodeInternalError); + .WithTestDisplayName("FileNotFoundException → 500"); + yield return new TheoryDataRow(typeof(OperationCanceledException)); + yield return new TheoryDataRow(typeof(TaskCanceledException)); + yield return new TheoryDataRow(typeof(TimeoutException)); + yield return new TheoryDataRow(typeof(NotSupportedException)); } [Fact] @@ -233,11 +132,12 @@ public async Task TryHandleAsync_OperationCanceledWithCancellationRequested_Retu } [Fact] - public async Task TryHandleAsync_OperationCanceledWithoutCancellationRequested_CallsProblemDetails() + public async Task TryHandleAsync_OperationCanceledWithoutCancellationRequested_Returns500ProblemDetails() { // Arrange var httpContext = CreateHttpContext(); - SetupProblemDetailsService(); + ProblemDetailsContext? captured = null; + SetupProblemDetailsServiceWithCapture(ctx => captured = ctx); var sut = CreateSut(); // Act @@ -246,14 +146,17 @@ public async Task TryHandleAsync_OperationCanceledWithoutCancellationRequested_C // Assert result.Should().BeTrue(); - _problemDetailsService.Verify(p => p.TryWriteAsync(It.IsAny()), Times.Once); + httpContext.Response.StatusCode.Should().Be(Status500InternalServerError); + captured!.ProblemDetails.Status.Should().Be(Status500InternalServerError); + captured.ProblemDetails.Type.Should().Be($"urn:paperless:error:{CodeInternalError}"); + captured.ProblemDetails.Detail.Should().BeNull(); } [Fact] - public async Task TryHandleAsync_ValidationException_CreatesHttpValidationProblemDetails() + public async Task TryHandleAsync_ValidationExceptionWithoutMemberNames_UsesModelLevelErrorKey() { // Arrange - ValidationException exception = new(new ValidationResult(EmailRequiredError, [EmailFieldName]), null, null); + ValidationException exception = new(new ValidationResult(EmailRequiredError), null, null); var httpContext = CreateHttpContext(); ProblemDetailsContext? captured = null; @@ -268,8 +171,38 @@ public async Task TryHandleAsync_ValidationException_CreatesHttpValidationProble captured!.ProblemDetails.Should().BeOfType(); var validation = (HttpValidationProblemDetails)captured.ProblemDetails; - validation.Errors.Should().ContainKey(EmailFieldName).WhoseValue.Should().ContainSingle(); - validation.Type.Should().Contain(CodeValidationError); + validation.Errors.Should().ContainSingle() + .Which.Should().BeEquivalentTo(new KeyValuePair(string.Empty, [EmailRequiredError])); + validation.Status.Should().Be(Status400BadRequest); + validation.Type.Should().Be($"urn:paperless:error:{CodeValidationError}"); + } + + [Fact] + public async Task TryHandleAsync_ValidationExceptionWithMembers_AddsMessageToEachDistinctNonblankMember() + { + // Arrange + ValidationException exception = new( + new ValidationResult( + EmailRequiredError, + [EmailFieldName, NameFieldName, EmailFieldName, string.Empty, " "]), + null, + null); + var httpContext = CreateHttpContext(); + + ProblemDetailsContext? captured = null; + SetupProblemDetailsServiceWithCapture(ctx => captured = ctx); + var sut = CreateSut(); + + // Act + await sut.TryHandleAsync(httpContext, exception, TestContext.Current.CancellationToken); + + // Assert + var validation = captured!.ProblemDetails.Should().BeOfType().Subject; + validation.Errors.Should().BeEquivalentTo(new Dictionary + { + [EmailFieldName] = [EmailRequiredError], + [NameFieldName] = [EmailRequiredError] + }); } [Fact] @@ -277,18 +210,23 @@ public async Task TryHandleAsync_NonValidationException_CreatesStandardProblemDe { // Arrange var httpContext = CreateHttpContext(); + InvalidOperationException exception = new(TestMessage); ProblemDetailsContext? captured = null; SetupProblemDetailsServiceWithCapture(ctx => captured = ctx); var sut = CreateSut(); // Act - await sut.TryHandleAsync(httpContext, new KeyNotFoundException(NotFoundMessage), + await sut.TryHandleAsync(httpContext, exception, TestContext.Current.CancellationToken); // Assert captured.Should().NotBeNull(); captured!.ProblemDetails.Should().NotBeOfType(); - captured.ProblemDetails.Status.Should().Be(Status404NotFound); + captured.ProblemDetails.Status.Should().Be(Status500InternalServerError); + captured.ProblemDetails.Type.Should().Be($"urn:paperless:error:{CodeInternalError}"); + captured.ProblemDetails.Detail.Should().BeNull(); + captured.HttpContext.Should().BeSameAs(httpContext); + captured.Exception.Should().BeSameAs(exception); } [Fact] @@ -300,14 +238,14 @@ public async Task TryHandleAsync_LogsWithCorrectLevelAndCode() var sut = CreateSut(); // Act - await sut.TryHandleAsync(httpContext, new KeyNotFoundException(DocNotFoundMessage), + await sut.TryHandleAsync(httpContext, new KeyNotFoundException(TestMessage), TestContext.Current.CancellationToken); // Assert _logCollector.GetSnapshot() .Should().Contain(log => - log.Level == LogLevel.Information && - log.Message.Contains(CodeNotFound, StringComparison.Ordinal)); + log.Level == LogLevel.Error && + log.Message.Contains(CodeInternalError, StringComparison.Ordinal)); } [Fact] @@ -392,7 +330,6 @@ public sealed class ProblemDetailsEnricherTests : IDisposable private const string InnerExceptionMessage = "Inner"; private const string OriginalDetailMessage = "Original"; private const string DetailedDevError = "Detailed dev error"; - private const string DocumentNotFoundMessage = "Document not found"; private const string TestEndpointName = "TestEndpoint"; private const string RoutePatternDocumentsId = "/api/documents/{id}"; @@ -552,19 +489,19 @@ public void Enrich_InProduction_With500_HidesDetails() } [Fact] - public void Enrich_InProduction_Non500_ShowsExceptionMessage() + public void Enrich_InProduction_Non500_PreservesSafeDetail() { // Arrange _hostEnvironment.Setup(e => e.EnvironmentName).Returns(Environments.Production); - ProblemDetails pd = new() { Status = Status404 }; - KeyNotFoundException exception = new(DocumentNotFoundMessage); + ProblemDetails pd = new() { Status = Status400, Detail = OriginalDetailMessage }; + BadHttpRequestException exception = new(DetailedDevError); (var sut, var context) = CreateSutAndContext(pd, exception); // Act InvokeEnrich(sut, context); // Assert - context.ProblemDetails.Detail.Should().Be(DocumentNotFoundMessage); + context.ProblemDetails.Detail.Should().Be(OriginalDetailMessage); } [Fact] @@ -583,10 +520,10 @@ public void Enrich_NoException_KeepsOriginalDetail() } [Fact] - public void Enrich_HttpValidationProblemDetails_EmptyErrors_FallsToExceptionMessage() + public void Enrich_HttpValidationProblemDetails_EmptyErrors_DoesNotExposeExceptionMessage() { - // Arrange - Covers Errors.Count is 0 branch - HttpValidationProblemDetails validationPd = new(); // Empty errors + // Arrange + HttpValidationProblemDetails validationPd = new() { Detail = OriginalDetailMessage }; InvalidOperationException exception = new(DetailedDevError); (var sut, var context) = CreateSutAndContext( validationPd, exception); @@ -594,8 +531,8 @@ public void Enrich_HttpValidationProblemDetails_EmptyErrors_FallsToExceptionMess // Act InvokeEnrich(sut, context); - // Assert - Falls through to exception message branch - context.ProblemDetails.Detail.Should().Be(DetailedDevError); + // Assert + context.ProblemDetails.Detail.Should().Be(OriginalDetailMessage); } [Fact] @@ -676,10 +613,10 @@ public void Enrich_InProduction_Status499_NoException_KeepsOriginalDetail() } [Fact] - public void Enrich_NullProblemDetailsStatus_WithException_ShowsExceptionMessage() + public void Enrich_NullProblemDetailsStatus_WithException_DoesNotExposeExceptionMessage() { - // Arrange - ProblemDetails with null status (edge case) - ProblemDetails pd = new() { Status = null }; + // Arrange + ProblemDetails pd = new() { Status = null, Detail = OriginalDetailMessage }; InvalidOperationException exception = new(DetailedDevError); (var sut, var context) = CreateSutAndContext( pd, exception); @@ -688,7 +625,7 @@ public void Enrich_NullProblemDetailsStatus_WithException_ShowsExceptionMessage( InvokeEnrich(sut, context); // Assert - context.ProblemDetails.Detail.Should().Be(DetailedDevError); + context.ProblemDetails.Detail.Should().Be(OriginalDetailMessage); } [Fact] diff --git a/PaperlessREST.Tests/Unit/PathNormalizationTests.cs b/PaperlessREST.Tests/Unit/PathNormalizationTests.cs new file mode 100644 index 0000000..ac6d7c9 --- /dev/null +++ b/PaperlessREST.Tests/Unit/PathNormalizationTests.cs @@ -0,0 +1,69 @@ +namespace PaperlessREST.Tests.Unit; + +public sealed class PathNormalizationTests +{ + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData(" \t")] + public void Normalize_NullOrWhitespace_ReturnsEmpty(string? value) => + PathNormalization.Normalize(value).Should().BeEmpty(); + + [Fact] + public void Normalize_RelativePath_ReturnsFullPathWithoutTrailingSeparator() + { + string relativePath = Path.Combine("batch", "input") + Path.DirectorySeparatorChar; + + string result = PathNormalization.Normalize(relativePath); + + result.Should().Be(Path.Combine(Environment.CurrentDirectory, "batch", "input")); + Path.IsPathFullyQualified(result).Should().BeTrue(); + } + + [Fact] + public void PlatformComparer_UsesRunningPlatformCaseSensitivity() + { + bool expectedToIgnoreCase = OperatingSystem.IsWindows() || OperatingSystem.IsMacOS(); + + PathNormalization.PlatformComparer + .Equals("Batch/Input", "batch/input") + .Should().Be(expectedToIgnoreCase); + } + + [Fact] + public void HasDistinctPaths_DifferentNormalizedPaths_ReturnsTrue() + { + BatchOptions options = CreateOptions("input", "archive", "error"); + + options.HasDistinctPaths.Should().BeTrue(); + } + + [Fact] + public void HasDistinctPaths_SamePathWithDifferentSyntax_ReturnsFalse() + { + string inputPath = Path.Combine("batch", "input"); + BatchOptions options = CreateOptions(inputPath, inputPath + Path.DirectorySeparatorChar, "error"); + + options.HasDistinctPaths.Should().BeFalse(); + } + + [Fact] + public void HasDistinctPaths_PathsDifferingOnlyByCase_UsesRunningPlatformPolicy() + { + BatchOptions options = CreateOptions("batch/input", "BATCH/INPUT", "batch/error"); + bool expectedToBeDistinct = !(OperatingSystem.IsWindows() || OperatingSystem.IsMacOS()); + + options.HasDistinctPaths.Should().Be(expectedToBeDistinct); + } + + private static BatchOptions CreateOptions(string inputPath, string archivePath, string errorPath) => + new() + { + InputPath = inputPath, + ArchivePath = archivePath, + ErrorPath = errorPath, + FilePattern = "*.xml", + CronExpression = "0 0 * * *", + TimeZoneId = "UTC" + }; +} diff --git a/PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs b/PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs index 7f09fc9..6e95d3f 100644 --- a/PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs +++ b/PaperlessREST.Tests/Unit/ServiceCollectionExtensionsTests.cs @@ -26,7 +26,7 @@ private static IServiceProvider BuildMinioServiceProvider(IMinioClient client) services.AddSingleton(client); services.AddSingleton>(Options.Create(new MinioOptions { - Endpoint = "localhost:9000", + Endpoint = new Uri("http://localhost:9000"), AccessKey = "k", SecretKey = "s", BucketName = Bucket @@ -179,7 +179,7 @@ public void MinioOpts_AccessorReturnsConfiguredOptions() { MinioOptions opts = new() { - Endpoint = "host:9000", + Endpoint = new Uri("http://host:9000"), AccessKey = "k", SecretKey = "s", BucketName = "b" @@ -241,12 +241,12 @@ public void IsDev_WhenEnvironmentIsProduction_ReturnsFalse() app.IsDev.Should().BeFalse(); } - // ────────────────────────────────────────────────────────────────── // AddDependencies-backed lambdas (ProblemDetails, Hangfire, OpenApi, ApiExplorer) // and MapEndpoints in Development environment. - // ────────────────────────────────────────────────────────────────── - private static WebApplicationBuilder CreateWiredBuilder(string environment) + private static WebApplicationBuilder CreateWiredBuilder( + string environment, + string minioEndpoint = "http://localhost:9000") { var builder = WebApplication.CreateBuilder(new WebApplicationOptions { @@ -256,19 +256,19 @@ private static WebApplicationBuilder CreateWiredBuilder(string environment) { ["ConnectionStrings:PaperlessDb"] = "Host=localhost;Database=test;Username=u;Password=p", ["ConnectionStrings:Hangfire"] = "Host=localhost;Database=hf;Username=u;Password=p", - ["RabbitMQ:Uri"] = "amqp://guest:guest@localhost:5672/", - ["Storage:Minio:Endpoint"] = "localhost:9000", + ["RabbitMQ:Uri"] = "amqp://localhost:5672/", + ["Storage:Minio:Endpoint"] = minioEndpoint, ["Storage:Minio:AccessKey"] = "k", ["Storage:Minio:SecretKey"] = "s", ["Storage:Minio:BucketName"] = "b", ["Elasticsearch:Uri"] = "http://localhost:9200", ["Elasticsearch:DefaultIndex"] = "docs", - ["BatchProcessing:InputPath"] = "/in", - ["BatchProcessing:ArchivePath"] = "/arch", - ["BatchProcessing:ErrorPath"] = "/err", - ["BatchProcessing:FilePattern"] = "*.xml", - ["BatchProcessing:CronExpression"] = "0 0 * * *", - ["BatchProcessing:TimeZoneId"] = "UTC" + ["Batch:InputPath"] = "/in", + ["Batch:ArchivePath"] = "/arch", + ["Batch:ErrorPath"] = "/err", + ["Batch:FilePattern"] = "*.xml", + ["Batch:CronExpression"] = "0 0 * * *", + ["Batch:TimeZoneId"] = "UTC" }); builder.AddDependencies(); @@ -287,6 +287,34 @@ private static HashSet CollectMappedPatterns(WebApplication app) => .Select(e => e.RoutePattern.RawText ?? string.Empty) .ToHashSet(StringComparer.Ordinal); + [Theory] + [InlineData("http://minio.local:9000")] + [InlineData("https://minio.local:9443")] + public void AddDependencies_WithAbsoluteMinioEndpoint_BuildsClient(string endpoint) + { + var builder = CreateWiredBuilder("Production", endpoint); + using var app = builder.Build(); + + app.Services.GetRequiredService().Should().NotBeNull(); + } + + [Theory] + [InlineData("minio.local:9000")] + [InlineData("ftp://minio.local:21")] + [InlineData("http://user:password@minio.local:9000")] + [InlineData("http://minio.local:9000/prefix")] + [InlineData("http://minio.local:9000?region=local")] + [InlineData("http://minio.local:9000#fragment")] + public void AddDependencies_WithInvalidMinioEndpoint_RejectsOptions(string endpoint) + { + var builder = CreateWiredBuilder("Production", endpoint); + using var app = builder.Build(); + Action resolve = () => _ = app.Services.GetRequiredService>().Value; + + resolve.Should().Throw() + .WithMessage("*absolute HTTP or HTTPS origin*"); + } + [Fact] public void MapEndpoints_WhenIsDev_RegistersDevelopmentOnlyRoutes() { @@ -361,34 +389,12 @@ public void MapEndpoints_WhenIsDev_ScalarConfigureCallback_SetsTitleServersAndTh scalarOpts.Theme.Should().Be(ScalarTheme.Kepler); } - private static Action GetInlineProblemDetailsConfigure(IServiceCollection services) - { - // AddProblemDetails(opts => ...) registers a ConfigureNamedOptions - // whose Action is the production lambda at L141-146. Find it (ImplementationInstance, NOT - // the ProblemDetailsEnricher transient). - foreach (var d in services) - { - if (d.ServiceType != typeof(IConfigureOptions) || - d.ImplementationInstance is not ConfigureNamedOptions named || - named.Action is null) - { - continue; - } - - return named.Action; - } - - throw new InvalidOperationException("Inline ProblemDetails configure action not found."); - } - [Fact] - public void AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInstanceFromHttpContextWhenNoActivity() + public void AddDependencies_ProblemDetailsCustomization_ResolvesCompleteProductionCallback() { var builder = CreateWiredBuilder("Production"); - var configure = GetInlineProblemDetailsConfigure(builder.Services); - - ProblemDetailsOptions opts = new(); - configure(opts); + using var app = builder.Build(); + var opts = app.Services.GetRequiredService>().Value; opts.CustomizeProblemDetails.Should().NotBeNull(); var saved = Activity.Current; @@ -399,10 +405,21 @@ public void AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInsta http.Request.Method = "POST"; http.Request.Path = "/api/v1/documents"; http.TraceIdentifier = "trace-from-context-42"; + http.SetEndpoint(new RouteEndpoint( + _ => Task.CompletedTask, + RoutePatternFactory.Parse("/api/v1/documents"), + 0, + new EndpointMetadataCollection(), + "UploadDocument")); ProblemDetailsContext ctx = new() { HttpContext = http, - ProblemDetails = new ProblemDetails() + ProblemDetails = new ProblemDetails + { + Status = StatusCodes.Status500InternalServerError, + Detail = "sensitive implementation detail" + }, + Exception = new InvalidOperationException("sensitive implementation detail") }; opts.CustomizeProblemDetails!(ctx); @@ -411,6 +428,13 @@ public void AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInsta .WhoseValue.Should().Be("trace-from-context-42"); ctx.ProblemDetails.Extensions.Should().ContainKey("instance") .WhoseValue.Should().Be("POST /api/v1/documents"); + ctx.ProblemDetails.Extensions.Should().ContainKey("timestamp") + .WhoseValue.Should().BeOfType().Which.Should().NotBeNullOrWhiteSpace(); + ctx.ProblemDetails.Extensions.Should().ContainKey("route") + .WhoseValue.Should().Be("/api/v1/documents"); + ctx.ProblemDetails.Extensions.Should().NotContainKey("debug"); + ctx.ProblemDetails.Detail.Should().Be( + "An internal error occurred. Please contact support if the problem persists."); } finally { @@ -422,10 +446,8 @@ public void AddDependencies_ProblemDetailsCustomization_PopulatesTraceIdAndInsta public void AddDependencies_ProblemDetailsCustomization_UsesActivityIdWhenAvailable() { var builder = CreateWiredBuilder("Production"); - var configure = GetInlineProblemDetailsConfigure(builder.Services); - - ProblemDetailsOptions opts = new(); - configure(opts); + using var app = builder.Build(); + var opts = app.Services.GetRequiredService>().Value; opts.CustomizeProblemDetails.Should().NotBeNull(); using Activity activity = new("unit-test-span"); diff --git a/PaperlessREST/API/GlobalExceptionHandler.cs b/PaperlessREST/API/GlobalExceptionHandler.cs index 4f55219..22c6cbb 100644 --- a/PaperlessREST/API/GlobalExceptionHandler.cs +++ b/PaperlessREST/API/GlobalExceptionHandler.cs @@ -12,15 +12,8 @@ public static ExceptionInfo FromException(Exception ex) => { ValidationException => new ExceptionInfo(StatusCodes.Status400BadRequest, LogLevel.Information, "validation_error"), - ArgumentException or InvalidOperationException or JsonException or BadHttpRequestException - => new ExceptionInfo(StatusCodes.Status400BadRequest, LogLevel.Warning, "bad_request"), - UnauthorizedAccessException => new ExceptionInfo(StatusCodes.Status403Forbidden, LogLevel.Warning, - "forbidden"), - KeyNotFoundException or FileNotFoundException - => new ExceptionInfo(StatusCodes.Status404NotFound, LogLevel.Information, "not_found"), - OperationCanceledException => new ExceptionInfo(HttpStatusCodes.ClientClosedRequest, LogLevel.Debug, - "cancelled"), - TimeoutException => new ExceptionInfo(StatusCodes.Status504GatewayTimeout, LogLevel.Error, "timeout"), + BadHttpRequestException => new ExceptionInfo(StatusCodes.Status400BadRequest, LogLevel.Warning, + "bad_request"), _ => new ExceptionInfo(StatusCodes.Status500InternalServerError, LogLevel.Error, "internal_error") }; } @@ -59,22 +52,43 @@ public async ValueTask TryHandleAsync( HttpContext = context, Exception = exception, ProblemDetails = exception is ValidationException validationEx - ? new HttpValidationProblemDetails( - new Dictionary - { - { - validationEx.ValidationResult.MemberNames.FirstOrDefault()!, - [validationEx.ValidationResult.ErrorMessage!] - } - }) + ? CreateValidationProblemDetails(validationEx, info) + : new ProblemDetails { - Status = info.StatusCode, Type = $"urn:paperless:error:{info.Code}" + Status = info.StatusCode, + Type = $"urn:paperless:error:{info.Code}", + Detail = exception is BadHttpRequestException ? "The request was invalid." : null } - : new ProblemDetails { Status = info.StatusCode, Type = $"urn:paperless:error:{info.Code}" } }; return await problemDetails.TryWriteAsync(problemDetailsContext); } + + private static HttpValidationProblemDetails CreateValidationProblemDetails( + ValidationException exception, + ExceptionInfo info) + { + var message = exception.ValidationResult.ErrorMessage ?? exception.Message; + Dictionary errors = []; + + foreach (var memberName in exception.ValidationResult.MemberNames + .Where(static name => !string.IsNullOrWhiteSpace(name)) + .Distinct(StringComparer.Ordinal)) + { + errors[memberName] = [message]; + } + + if (errors.Count == 0) + { + errors[string.Empty] = [message]; + } + + return new HttpValidationProblemDetails(errors) + { + Status = info.StatusCode, + Type = $"urn:paperless:error:{info.Code}" + }; + } } public sealed class ProblemDetailsEnricher( @@ -87,8 +101,10 @@ private void Enrich(ProblemDetailsContext context) { var pd = context.ProblemDetails; var httpContext = context.HttpContext; + bool isDevelopment = env.IsDevelopment(); pd.Extensions["trace_id"] = Activity.Current?.Id ?? httpContext.TraceIdentifier; + pd.Extensions["instance"] = $"{httpContext.Request.Method} {httpContext.Request.Path}"; pd.Extensions["timestamp"] = timeProvider.GetUtcNow().ToString("O"); if (httpContext.GetEndpoint() is RouteEndpoint { RoutePattern.RawText: var pattern }) @@ -96,30 +112,30 @@ private void Enrich(ProblemDetailsContext context) pd.Extensions["route"] = pattern; } - pd.Detail = (pd, context.Exception) switch + if (pd is HttpValidationProblemDetails validation && validation.Errors.Count > 0) { - (HttpValidationProblemDetails { Errors.Count: > 0 } validation, _) => - $"Validation failed with {validation.Errors.Values.Sum(arr => arr.Length)} error(s).", - (_, { } ex) when env.IsDevelopment() => ex.Message, - ({ Status: >= 500 }, _) when !env.IsDevelopment() => - "An internal error occurred. Please contact support if the problem persists.", - (_, { } ex) => ex.Message, - _ => pd.Detail - }; - - if (!env.IsDevelopment() || context.Exception is not ({ InnerException: not null } or { StackTrace: not null })) + pd.Detail = $"Validation failed with {validation.Errors.Values.Sum(arr => arr.Length)} error(s)."; + } + else if (isDevelopment && context.Exception is not null) + { + pd.Detail = context.Exception.Message; + } + else if (!isDevelopment && pd.Status >= StatusCodes.Status500InternalServerError) + { + pd.Detail = "An internal error occurred. Please contact support if the problem persists."; + } + var exception = context.Exception; + if (!isDevelopment || exception is null || + (exception.InnerException is null && exception.StackTrace is null)) { return; } + pd.Extensions["debug"] = new { - var ex = context.Exception; - pd.Extensions["debug"] = new - { - exception_type = ex.GetType().FullName, - inner_exception = ex.InnerException?.Message, - stack_trace = ex.StackTrace - }; - } + exception_type = exception.GetType().FullName, + inner_exception = exception.InnerException?.Message, + stack_trace = exception.StackTrace + }; } } diff --git a/PaperlessREST/Configuration/Constraints.cs b/PaperlessREST/Configuration/Constraints.cs index a32bffb..5b0a778 100644 --- a/PaperlessREST/Configuration/Constraints.cs +++ b/PaperlessREST/Configuration/Constraints.cs @@ -12,16 +12,6 @@ public static class FileUploadConstraints public const double BytesPerMegabyte = 1024 * 1024; } -/// -/// Server-side search constants that don't belong on the public contract. -/// Boundary-level covers query / limit ranges. -/// -public static class SearchServiceConstraints -{ - /// Maximum query length at service layer (truncation threshold). - public const int ServiceQueryMaxLength = 1000; -} - /// /// Rate limiting policy names. /// diff --git a/PaperlessREST/Configuration/MinioOptions.cs b/PaperlessREST/Configuration/MinioOptions.cs index 6c321eb..b75d302 100644 --- a/PaperlessREST/Configuration/MinioOptions.cs +++ b/PaperlessREST/Configuration/MinioOptions.cs @@ -6,7 +6,7 @@ public sealed record MinioOptions public const string SectionName = "Storage:Minio"; [Required(ErrorMessage = $"{SectionName}:Endpoint is required")] - public required string Endpoint { get; init; } + public required Uri Endpoint { get; init; } [Required(ErrorMessage = $"{SectionName}:AccessKey is required")] public required string AccessKey { get; init; } @@ -17,25 +17,4 @@ public sealed record MinioOptions [Required(ErrorMessage = $"{SectionName}:BucketName is required")] public required string BucketName { get; init; } - public bool UseSsl { get; init; } -} - -public static class MinioOptionsExtensions -{ - extension(MinioOptions opts) - { - /// - /// Parses the Endpoint string into a Uri, adding http(s):// scheme if missing. - /// - public Uri EndpointUri - { - get - { - string endpoint = opts.Endpoint.Contains("://", StringComparison.Ordinal) - ? opts.Endpoint - : $"{(opts.UseSsl ? "https" : "http")}://{opts.Endpoint}"; - return new Uri(endpoint); - } - } - } } diff --git a/PaperlessREST/Contracts/DocumentManagement/DocumentDtos.cs b/PaperlessREST/Contracts/DocumentManagement/DocumentDtos.cs index 0eae348..4c13f16 100644 --- a/PaperlessREST/Contracts/DocumentManagement/DocumentDtos.cs +++ b/PaperlessREST/Contracts/DocumentManagement/DocumentDtos.cs @@ -28,22 +28,6 @@ public sealed record PaginationQuery public Guid? Cursor { get; init; } } -/// -/// Query parameters for document search. -/// -[ExcludeFromCodeCoverage(Justification = "Pure transport DTO - compiler-generated record members only")] -public sealed record SearchQuery -{ - [StringLength(SearchConstraints.QueryMaxLength, MinimumLength = SearchConstraints.QueryMinLength, - ErrorMessage = "Search query must be between 1 and 100 characters")] - [Description("Search text to find in documents")] - public required string Query { get; init; } - - [Range(1, SearchConstraints.MaxResultLimit, ErrorMessage = "Limit must be between 1 and 100")] - [Description("Maximum number of results to return")] - public int Limit { get; init; } = SearchConstraints.DefaultResultLimit; -} - /// /// Represents document metadata returned by the API. /// diff --git a/PaperlessREST/Features/BatchProcessing/Application/BatchOrchestrator.cs b/PaperlessREST/Features/BatchProcessing/Application/BatchOrchestrator.cs index 25ce7ef..1dab9e7 100644 --- a/PaperlessREST/Features/BatchProcessing/Application/BatchOrchestrator.cs +++ b/PaperlessREST/Features/BatchProcessing/Application/BatchOrchestrator.cs @@ -31,14 +31,13 @@ public async Task ProcessAsync(IJobCancellationToken token) var paths = ClaimFiles(); int processed = 0, quarantined = 0; - switch (paths.Count) + if (paths.Count == 0) { - case 0: - logger.LogDebug("Batch job '{JobId}' found no files", BatchOptions.JobId); - break; - case > 0: - logger.LogDebug("Batch job '{JobId}' processing {Count} file(s)", BatchOptions.JobId, paths.Count); - break; + logger.LogDebug("Batch job '{JobId}' found no files", BatchOptions.JobId); + } + else + { + logger.LogDebug("Batch job '{JobId}' processing {Count} file(s)", BatchOptions.JobId, paths.Count); } foreach (var path in paths) @@ -100,14 +99,10 @@ private void ClaimNewFiles(IDirectoryInfo inputDir, List claimed) } } - internal async Task ProcessFileAsync(string path, CancellationToken ct) + private async Task ProcessFileAsync(string path, CancellationToken ct) { - // Trim the .processing suffix only; .Replace would strip embedded occurrences - // in pathological filenames like "report.processing.xml.processing". var fileName = Path.GetFileName(path); - var originalName = fileName.EndsWith(ProcessingExt, StringComparison.Ordinal) - ? fileName[..^ProcessingExt.Length] - : fileName; + var originalName = fileName[..^ProcessingExt.Length]; logger.LogInformation("Processing file: {File}", originalName); diff --git a/PaperlessREST/Features/DocumentManagement/Application/DocumentService.cs b/PaperlessREST/Features/DocumentManagement/Application/DocumentService.cs index 8d4a4a8..721858a 100644 --- a/PaperlessREST/Features/DocumentManagement/Application/DocumentService.cs +++ b/PaperlessREST/Features/DocumentManagement/Application/DocumentService.cs @@ -1,5 +1,4 @@ -using System.Net; -using System.Net.Sockets; +using Minio.Exceptions; using Result = ErrorOr.Result; namespace PaperlessREST.Features.DocumentManagement.Application; @@ -39,7 +38,7 @@ public interface IDocumentService Guid? cursor = null, CancellationToken cancellationToken = default); - IAsyncEnumerable SearchDocumentsAsync( + Task> SearchDocumentsAsync( string query, int limit, CancellationToken cancellationToken = default); @@ -105,13 +104,16 @@ public async Task> UploadDocumentAsync( } catch (Exception ex) { - if (TryMapStorageException(ex, document.StoragePath) is not { } storageError) + if (TryMapStorageException(ex, cancellationToken) is not { } storageError) { - // Unrecognized exception - let it propagate to GlobalExceptionHandler throw; } - logger.LogWarning(ex, "Storage error: {ErrorCode}", storageError.Code); + logger.LogWarning( + ex, + "Storage error {ErrorCode} for {StoragePath}", + storageError.Code, + document.StoragePath); return storageError; } @@ -186,11 +188,11 @@ public async Task> UpdateDocumentSummaryAsync( CancellationToken cancellationToken = default) => repository.GetDocumentsPagedAsync(pageSize, cursor, cancellationToken); - public IAsyncEnumerable SearchDocumentsAsync( + public Task> SearchDocumentsAsync( string query, int limit, CancellationToken cancellationToken = default) => - search.SearchAsync(query, limit, cancellationToken); + search.SearchAsync(query, limit, cancellationToken); public async ValueTask> GetDocumentByIdAsync( Guid id, @@ -211,20 +213,9 @@ public async Task> DeleteDocumentAsync( return DocumentErrors.NotFound(id); } - await Task.WhenAll( - repository.DeleteAsync(id, cancellationToken), - storage.DeleteAsync(document.StoragePath, cancellationToken)); - - try - { - await search.DeleteAsync(id, cancellationToken); - } - catch (Exception ex) - { - logger.LogWarning(ex, - "Failed to delete document {DocumentId} from search index - expected if not yet indexed", - id); - } + await search.DeleteAsync(id, cancellationToken); + await storage.DeleteAsync(document.StoragePath, cancellationToken); + await repository.DeleteAsync(id, cancellationToken); logger.LogInformation("Document {DocumentId} deleted successfully", id); return Result.Deleted; @@ -234,28 +225,37 @@ await Task.WhenAll( /// Maps storage exceptions to domain errors for proper HTTP status codes. /// /// Domain error for known infrastructure failures, null for unknown exceptions. - private static Error? TryMapStorageException(Exception ex, string storagePath) => ex switch + private static Error? TryMapStorageException( + Exception exception, + CancellationToken cancellationToken) { - // Transient storage failures → 503 + Retry-After. Error.Custom(503, …) carries the status - // in Error.Type and the retry hint in metadata; ErrorOrX renders a 503 ProblemDetails with - // a "retryAfter" extension. (Previously Error.Unexpected → 503 via the hand-rolled glue.) - TimeoutException => Error.Custom(503, - "Document.StorageTimeout", - $"Storage timeout while processing {storagePath}", - new Dictionary { ["retryAfter"] = 30 }), - - HttpRequestException { StatusCode: { } code and >= HttpStatusCode.InternalServerError } => - Error.Custom(503, + if (exception is OperationCanceledException && !cancellationToken.IsCancellationRequested) + { + return Error.Custom(503, + "Document.StorageTimeout", + "The storage operation timed out.", + new Dictionary { ["retryAfter"] = 30 }); + } + + if (exception is UnexpectedMinioException + { + Response.Code: "InternalError" or "ServiceUnavailable" or "SlowDown" or "RequestTimeout" + }) + { + return Error.Custom(503, "Document.StorageServerError", - $"Storage service returned {(int)code} for {storagePath}", - new Dictionary { ["retryAfter"] = 30 }), + "The storage service is temporarily unavailable.", + new Dictionary { ["retryAfter"] = 30 }); + } - IOException { InnerException: SocketException } => - Error.Custom(503, + if (exception is HttpRequestException { StatusCode: null } or ConnectionException) + { + return Error.Custom(503, "Document.StorageConnectionFailed", - $"Cannot connect to storage service for {storagePath}", - new Dictionary { ["retryAfter"] = 30 }), + "The storage service could not be reached.", + new Dictionary { ["retryAfter"] = 30 }); + } - _ => null - }; + return null; + } } diff --git a/PaperlessREST/Features/DocumentManagement/Infrastructure/Search/DocumentSearchService.cs b/PaperlessREST/Features/DocumentManagement/Infrastructure/Search/DocumentSearchService.cs index a2bbd87..0dd05fb 100644 --- a/PaperlessREST/Features/DocumentManagement/Infrastructure/Search/DocumentSearchService.cs +++ b/PaperlessREST/Features/DocumentManagement/Infrastructure/Search/DocumentSearchService.cs @@ -1,64 +1,79 @@ +using Elastic.Transport; + namespace PaperlessREST.Features.DocumentManagement.Infrastructure.Search; public interface IDocumentSearchService { - IAsyncEnumerable SearchAsync(string query, int limit = 10, CancellationToken cancellationToken = default) - where T : class; + Task> SearchAsync( + string query, + int limit, + CancellationToken cancellationToken = default); - Task DeleteAsync(Guid id, CancellationToken cancellationToken = default); + Task DeleteAsync(Guid id, CancellationToken cancellationToken = default); } - /// /// Elasticsearch search service implementation. /// -/// -/// Excluded from coverage because async IAsyncEnumerable generates complex state machines with -/// unreachable branches for sync/async completion paths and iterator disposal. The actual search -/// logic (query building, result iteration) is tested via integration tests. -/// -[ExcludeFromCodeCoverage(Justification = - "Async IAsyncEnumerable - compiler-generated dual state machine (async + iterator) creates unreachable branches for sync/async completion and disposal paths")] public sealed class DocumentSearchService( ElasticsearchClient elastic, ILogger logger) : IDocumentSearchService { - public async IAsyncEnumerable SearchAsync(string query, int limit = 10, - [EnumeratorCancellation] CancellationToken cancellationToken = default) where T : class + public async Task> SearchAsync( + string query, + int limit, + CancellationToken cancellationToken = default) { - logger.LogInformation("Searching for query: {Query} (limit: {Limit})", query, limit); + logger.LogInformation( + "Searching documents with query length {QueryLength} and limit {Limit}", + query.Length, + limit); - var searchQuery = query.Length > SearchServiceConstraints.ServiceQueryMaxLength - ? query[..SearchServiceConstraints.ServiceQueryMaxLength] - : query; - - var response = await elastic.SearchAsync( - s => s.Indices(elastic.ElasticsearchClientSettings.DefaultIndex) - .Query(q => q.MultiMatch(mm => mm - .Query(searchQuery) - .Fields("*") - .Type(TextQueryType.BestFields) - .Fuzziness(new Fuzziness("AUTO")) - .Operator(Operator.Or) - .Lenient())) - .Size(limit) - .TrackScores(), + var response = await ExecuteElasticsearchAsync( + token => elastic.SearchAsync( + s => s.Indices(elastic.ElasticsearchClientSettings.DefaultIndex) + .Query(q => q.MultiMatch(mm => mm + .Query(query) + .Fields("*") + .Type(TextQueryType.BestFields) + .Fuzziness(new Fuzziness("AUTO")) + .Operator(Operator.Or) + .Lenient())) + .Size(limit) + .TrackScores(), + token), cancellationToken); logger.LogInformation("Found {Count} results", response.Documents.Count); - - foreach (var doc in response.Documents) - { - yield return doc; - } + return response.Documents; } - public async Task DeleteAsync(Guid id, CancellationToken cancellationToken = default) + public async Task DeleteAsync(Guid id, CancellationToken cancellationToken = default) { DeleteRequest deleteRequest = new(elastic.ElasticsearchClientSettings.DefaultIndex, id.ToString()); - var response = await elastic.DeleteAsync(deleteRequest, cancellationToken); + await ExecuteElasticsearchAsync( + token => elastic.DeleteAsync(deleteRequest, token), + cancellationToken); logger.LogInformation("Document {DocumentId} removed from search index", id); - return response.IsValidResponse; + } + + private static async Task ExecuteElasticsearchAsync( + Func> operation, + CancellationToken cancellationToken) + { + try + { + return await operation(cancellationToken); + } + catch (TransportException exception) when ( + cancellationToken.IsCancellationRequested && + exception.InnerException is OperationCanceledException) + { + throw new OperationCanceledException( + "Elasticsearch operation was canceled.", + exception, + cancellationToken); + } } } diff --git a/PaperlessREST/Features/DocumentManagement/Infrastructure/Storage/DocumentStorageService.cs b/PaperlessREST/Features/DocumentManagement/Infrastructure/Storage/DocumentStorageService.cs index c2bbe18..6f8da98 100644 --- a/PaperlessREST/Features/DocumentManagement/Infrastructure/Storage/DocumentStorageService.cs +++ b/PaperlessREST/Features/DocumentManagement/Infrastructure/Storage/DocumentStorageService.cs @@ -3,7 +3,7 @@ namespace PaperlessREST.Features.DocumentManagement.Infrastructure.Storage; public interface IDocumentStorageService { Task UploadAsync(Stream stream, string storagePath, long length, CancellationToken cancellationToken = default); - Task DeleteAsync(string storagePath, CancellationToken cancellationToken = default); + Task DeleteAsync(string storagePath, CancellationToken cancellationToken = default); } public sealed class DocumentStorageService( @@ -27,23 +27,14 @@ await minio.PutObjectAsync( logger.LogInformation("Document uploaded to storage at {StoragePath}", storagePath); } - public async Task DeleteAsync(string storagePath, CancellationToken cancellationToken = default) + public async Task DeleteAsync(string storagePath, CancellationToken cancellationToken = default) { - try - { - await minio.RemoveObjectAsync( - new RemoveObjectArgs() - .WithBucket(_options.BucketName) - .WithObject(storagePath), - cancellationToken); + await minio.RemoveObjectAsync( + new RemoveObjectArgs() + .WithBucket(_options.BucketName) + .WithObject(storagePath), + cancellationToken); - logger.LogInformation("Document removed from storage at {StoragePath}", storagePath); - return true; - } - catch (Exception ex) - { - logger.LogError(ex, "Failed to remove document from storage at {StoragePath}", storagePath); - return false; - } + logger.LogInformation("Document removed from storage at {StoragePath}", storagePath); } } diff --git a/PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs b/PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs index b5bcf19..a168aee 100644 --- a/PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs +++ b/PaperlessREST/Features/DocumentManagement/Presentation/Endpoints/DocumentEndpoints.cs @@ -32,26 +32,41 @@ public static async Task> GetDocuments( { Items = items.ConvertAll(static d => d.ToDocumentDto()), HasMore = hasMore, - NextCursor = hasMore && items.Count > 0 ? items[^1].Id : null + NextCursor = hasMore ? items[^1].Id : null }; } /// - /// Full-text search over OCR content via Elasticsearch. query/limit bind from the - /// query string directly (ErrorOrX's [AsParameters] binder doesn't set required - /// init-only members, so the SearchQuery DTO can't be used as the bind target here). + /// Full-text search over OCR content via Elasticsearch. Nullable bind targets preserve the + /// optional HTTP limit; the handler applies the default and owns query/limit validation. /// [Get("/search")] [EnableRateLimiting(RateLimitPolicies.SearchOperations)] public static async Task>> SearchDocuments( - string query, + string? query, IDocumentService documentService, CancellationToken cancellationToken, - int limit = SearchConstraints.DefaultResultLimit) => - await documentService - .SearchDocumentsAsync(query, limit, cancellationToken) - .Select(static r => r.ToDocumentSearchResultDto()) - .ToListAsync(cancellationToken); + int? limit = null) + { + if (string.IsNullOrEmpty(query) || query.Length > SearchConstraints.QueryMaxLength) + { + return Error.Validation("Query", + $"Search query must be between {SearchConstraints.QueryMinLength} and {SearchConstraints.QueryMaxLength} characters"); + } + + if (limit is < 1 or > SearchConstraints.MaxResultLimit) + { + return Error.Validation("Limit", + $"Limit must be between 1 and {SearchConstraints.MaxResultLimit}"); + } + + var results = await documentService.SearchDocumentsAsync( + query, + limit ?? SearchConstraints.DefaultResultLimit, + cancellationToken); + + return results.Select(static r => r.ToDocumentSearchResultDto()).ToList(); + } /// Gets a document by id. DocumentErrors.NotFound → 404. [Get("/{id:guid}")] @@ -100,7 +115,7 @@ public static async Task> UploadDocument( $"File size cannot exceed {FileUploadConstraints.MaxFileSizeBytes / FileUploadConstraints.BytesPerMegabyte:F0} MB"); } - var contentType = file.ContentType?.Split(';')[0].Trim() ?? ""; + var contentType = file.ContentType.Split(';')[0].Trim(); if (!contentType.Equals("application/pdf", StringComparison.OrdinalIgnoreCase)) { return Error.Validation("File", "Only PDF files are allowed"); diff --git a/PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs b/PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs index c6ce59d..a277423 100644 --- a/PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs +++ b/PaperlessREST/Host/Extensions/ServiceCollectionExtensions.cs @@ -137,13 +137,7 @@ private IServiceCollection AddCrossCuttingConcerns() o.SerializerOptions.Converters.Add(new JsonStringEnumConverter())); services.AddExceptionHandler(); - services.AddProblemDetails(static options => - options.CustomizeProblemDetails = static ctx => - { - ctx.ProblemDetails.Extensions["trace_id"] = Activity.Current?.Id ?? ctx.HttpContext.TraceIdentifier; - ctx.ProblemDetails.Extensions["instance"] = - $"{ctx.HttpContext.Request.Method} {ctx.HttpContext.Request.Path}"; - }); + services.AddProblemDetails(); services.ConfigureOptions(); services.AddHealthChecks(); @@ -238,15 +232,24 @@ private IServiceCollection AddObjectStorage() { services.AddOptionsWithValidateOnStart() .BindConfiguration(MinioOptions.SectionName) - .ValidateDataAnnotations(); + .ValidateDataAnnotations() + .Validate( + static options => options.Endpoint is { IsAbsoluteUri: true } endpoint && + (endpoint.Scheme == Uri.UriSchemeHttp || + endpoint.Scheme == Uri.UriSchemeHttps) && + string.IsNullOrEmpty(endpoint.UserInfo) && + endpoint.AbsolutePath == "/" && + string.IsNullOrEmpty(endpoint.Query) && + string.IsNullOrEmpty(endpoint.Fragment), + $"{MinioOptions.SectionName}:Endpoint must be an absolute HTTP or HTTPS origin"); services.AddSingleton(static sp => { var opts = sp.GetRequiredService>().Value; return new MinioClient() - .WithEndpoint(opts.EndpointUri.Host, opts.EndpointUri.Port) + .WithEndpoint(opts.Endpoint.Host, opts.Endpoint.Port) .WithCredentials(opts.AccessKey, opts.SecretKey) - .WithSSL(opts.UseSsl) + .WithSSL(opts.Endpoint.Scheme == Uri.UriSchemeHttps) .Build(); }); diff --git a/PaperlessServices.Tests/Integration/OcrIntegrationTests.cs b/PaperlessServices.Tests/Integration/OcrIntegrationTests.cs index 5c693c9..ec5c1e2 100644 --- a/PaperlessServices.Tests/Integration/OcrIntegrationTests.cs +++ b/PaperlessServices.Tests/Integration/OcrIntegrationTests.cs @@ -25,6 +25,23 @@ public async Task ExtractsInvoiceNumber() }); } + [Fact] + public async Task BlankPdf_ReturnsEmptyDocumentError() + { + string storagePath = await fixture.UploadPdfAsync(string.Empty); + OcrCommand command = new( + Guid.CreateVersion7(), + "blank.pdf", + storagePath, + TimeProvider.System.GetUtcNow()); + + ErrorOr result = + await OcrProcessor.ProcessDocumentAsync(command, TestContext.Current.CancellationToken); + + result.IsError.Should().BeTrue(); + result.FirstError.Code.Should().Be("Ocr.EmptyDocument"); + } + [Fact] public async Task ProcessMultipleDocuments_Concurrently() { diff --git a/PaperlessServices.Tests/Integration/StorageIntegrationTests.cs b/PaperlessServices.Tests/Integration/StorageIntegrationTests.cs index 2db25bc..e9d3af8 100644 --- a/PaperlessServices.Tests/Integration/StorageIntegrationTests.cs +++ b/PaperlessServices.Tests/Integration/StorageIntegrationTests.cs @@ -6,17 +6,17 @@ public class StorageIntegrationTests(SharedContainerFixture fixture) private IStorageService Storage => fixture.Services.GetRequiredService(); [Fact] - public async Task UploadAndDownload_RoundTripSuccess() + public async Task UploadAndDownload_PreservesExactPdfBytesAndStartsAtBeginning() { - // Arrange - var storagePath = await fixture.UploadPdfAsync("Storage round trip test"); + byte[] expected = await TestPdf.BytesAsync("Storage round trip test"); + string storagePath = await fixture.UploadPdfAsync(expected); - // Act - await using var stream = await Storage.DownloadAsync(storagePath, TestContext.Current.CancellationToken); + await using Stream stream = await Storage.DownloadAsync(storagePath, TestContext.Current.CancellationToken); + stream.Position.Should().Be(0); + await using MemoryStream downloaded = new(); + await stream.CopyToAsync(downloaded, TestContext.Current.CancellationToken); - // Assert - stream.Should().NotBeNull(); - stream.Length.Should().BePositive(); + downloaded.ToArray().Should().Equal(expected); } [Fact] diff --git a/PaperlessServices.Tests/Integration/WorkerTestBase.cs b/PaperlessServices.Tests/Integration/WorkerTestBase.cs index fc43fc1..6f7a3e1 100644 --- a/PaperlessServices.Tests/Integration/WorkerTestBase.cs +++ b/PaperlessServices.Tests/Integration/WorkerTestBase.cs @@ -15,13 +15,11 @@ public class SharedContainerCollection : ICollectionFixture TestEnv.Load(); - protected override bool UsesPostgres => false; - - private IHost _host = null!; + private IHost? _host; protected override async ValueTask ConfigureSutAsync() { @@ -34,7 +32,6 @@ protected override async ValueTask ConfigureSutAsync() ["Storage:Minio:AccessKey"] = MinioAccessKey, ["Storage:Minio:SecretKey"] = MinioSecretKey, ["Storage:Minio:BucketName"] = BucketName, - ["Storage:Minio:UseSsl"] = Environment.GetEnvironmentVariable("MINIO_USE_SSL") ?? "false", ["Elasticsearch:Uri"] = ElasticsearchUri, ["Elasticsearch:DefaultIndex"] = IndexName }); @@ -61,24 +58,28 @@ protected override async ValueTask ConfigureSutAsync() protected override async ValueTask DisposeSutAsync() { - // Null-guarded because a failed InitializeAsync (e.g. a container wait-strategy - // timeout) returns before _host is assigned. When the host exists, stop it - // gracefully then dispose it — no best-effort catch: a shutdown fault is real - // and must surface, not hide behind the init exception. - if (_host is not null) + if (_host is null) return; + + try { await _host.StopAsync(); + } + finally + { _host.Dispose(); } } - public async Task UploadPdfAsync(string content) + public async Task UploadPdfAsync(string content) => + await UploadPdfAsync(await TestPdf.BytesAsync(content)); + + public async Task UploadPdfAsync(byte[] content) { var storageKey = $"documents/{TimeProvider.System.GetUtcNow():yyyy-MM}/{Guid.NewGuid():N}/test-{Guid.NewGuid():N}.pdf"; var client = Services.GetRequiredService(); - await using var stream = new MemoryStream(await TestPdf.BytesAsync(content)); + await using var stream = new MemoryStream(content, writable: false); await client.PutObjectAsync(new PutObjectArgs() .WithBucket(BucketName) .WithObject(storageKey) diff --git a/PaperlessServices.Tests/Unit/CreatePdfExtractorTests.cs b/PaperlessServices.Tests/Unit/CreatePdfExtractorTests.cs index a52eea2..6560ac7 100644 --- a/PaperlessServices.Tests/Unit/CreatePdfExtractorTests.cs +++ b/PaperlessServices.Tests/Unit/CreatePdfExtractorTests.cs @@ -1,126 +1,39 @@ namespace PaperlessServices.Tests.Unit; -/// -/// Unit tests for CreatePdfExtractor covering all three branches: -/// 1. Success with extracted text -/// 2. Empty/whitespace text (returns EmptyDocument error) -/// 3. Exception during extraction (returns ExtractionFailed error) -/// public sealed class CreatePdfExtractorTests { - private const string ValidPdfContent = "This is extracted PDF content with meaningful text."; - private readonly FakeLogger _logger = new(); private CreatePdfExtractor CreateSut() => new(_logger); - private static MemoryStream CreateValidPdfStream() - { - // Minimal PDF structure - may or may not produce OCR text depending on the library - byte[] minimalPdf = - [ - 0x25, 0x50, 0x44, 0x46, 0x2D, 0x31, 0x2E, 0x34, 0x0A, // %PDF-1.4\n - 0x25, 0xE2, 0xE3, 0xCF, 0xD3, 0x0A // Binary marker - ]; - return new MemoryStream(minimalPdf); - } - - [Fact] - public async Task ExtractTextAsync_WithValidPdf_ReturnsExtractedText() - { - // Arrange - Create a minimal valid PDF stream - // Note: The CreatePdf.NET library will attempt OCR on the stream - // This test verifies the success path when text is extracted - await using MemoryStream pdfStream = CreateValidPdfStream(); - CreatePdfExtractor sut = CreateSut(); - - // Act - ErrorOr result = await sut.ExtractTextAsync(pdfStream, TestContext.Current.CancellationToken); - - // Assert - // The actual OCR may return empty for a minimal PDF, which is expected behavior - // This test verifies the method doesn't throw and returns a result - result.Should().Match>(r => - !r.IsError || r.FirstError.Code == "Ocr.EmptyDocument"); - } - - [Fact] - public async Task ExtractTextAsync_WhenOcrReturnsEmpty_ReturnsEmptyDocumentError() - { - // Arrange - Use an empty stream that will produce no text - await using MemoryStream emptyPdfStream = new(); - CreatePdfExtractor sut = CreateSut(); - - // Act - ErrorOr result = await sut.ExtractTextAsync(emptyPdfStream, TestContext.Current.CancellationToken); - - // Assert - Either ExtractionFailed (exception) or EmptyDocument (no text) - result.IsError.Should().BeTrue("empty stream should not produce valid text"); - result.FirstError.Code.Should().BeOneOf("Ocr.EmptyDocument", "Ocr.ExtractionFailed"); - } - [Fact] - public async Task ExtractTextAsync_WhenOcrReturnsWhitespace_ReturnsEmptyDocumentError() + public async Task ExtractTextAsync_WhenCallerCancels_PropagatesCancellation() { - // Arrange - Create a stream that might produce only whitespace - await using MemoryStream whitespaceStream = new("%PDF"u8.ToArray()); // PDF magic bytes only - CreatePdfExtractor sut = CreateSut(); + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + await using MemoryStream pdfStream = new(); - // Act - ErrorOr result = await sut.ExtractTextAsync(whitespaceStream, TestContext.Current.CancellationToken); + Func act = async () => await CreateSut().ExtractTextAsync(pdfStream, cancellation.Token); - // Assert - result.IsError.Should().BeTrue("whitespace-only content should be treated as empty"); + OperationCanceledException thrown = + (await act.Should().ThrowAsync()).Which; + thrown.CancellationToken.Should().Be(cancellation.Token); + _logger.Collector.GetSnapshot().Should().NotContain(log => log.Level == LogLevel.Error); } [Fact] - public async Task ExtractTextAsync_WhenExceptionThrown_ReturnsExtractionFailedError() + public async Task ExtractTextAsync_WhenStreamIsDisposed_ReturnsExtractionFailureAndLogsError() { - // Arrange - Closed stream will throw when read - MemoryStream closedStream = new(); - await closedStream.DisposeAsync(); - CreatePdfExtractor sut = CreateSut(); + MemoryStream pdfStream = new(); + await pdfStream.DisposeAsync(); - // Act - ErrorOr result = await sut.ExtractTextAsync(closedStream, TestContext.Current.CancellationToken); + ErrorOr result = + await CreateSut().ExtractTextAsync(pdfStream, TestContext.Current.CancellationToken); - // Assert - result.IsError.Should().BeTrue("disposed stream should cause exception"); + result.IsError.Should().BeTrue(); result.FirstError.Code.Should().Be("Ocr.ExtractionFailed"); - } - - [Fact] - public async Task ExtractTextAsync_WhenExceptionThrown_LogsError() - { - // Arrange - MemoryStream closedStream = new(); - await closedStream.DisposeAsync(); - CreatePdfExtractor sut = CreateSut(); - - // Act - await sut.ExtractTextAsync(closedStream, TestContext.Current.CancellationToken); - - // Assert - _logger.Collector.GetSnapshot() - .Should().Contain(l => l.Level == LogLevel.Error && l.Message.Contains("OCR extraction failed")); - } - - [Fact] - public async Task ExtractTextAsync_WhenSuccessful_LogsCharacterCount() - { - // Arrange - await using MemoryStream pdfStream = CreateValidPdfStream(); - CreatePdfExtractor sut = CreateSut(); - - // Act - ErrorOr result = await sut.ExtractTextAsync(pdfStream, TestContext.Current.CancellationToken); - - // Assert - If successful, should log character count - if (!result.IsError) - { - _logger.Collector.GetSnapshot() - .Should().Contain(l => - l.Level == LogLevel.Information && l.Message.Contains("characters from PDF")); - } + _logger.Collector.GetSnapshot().Should().Contain(log => + log.Level == LogLevel.Error && + log.Message.Contains("OCR extraction failed", StringComparison.OrdinalIgnoreCase)); } } diff --git a/PaperlessServices.Tests/Unit/FakeLoggerExtensionsTests.cs b/PaperlessServices.Tests/Unit/FakeLoggerExtensionsTests.cs new file mode 100644 index 0000000..383899b --- /dev/null +++ b/PaperlessServices.Tests/Unit/FakeLoggerExtensionsTests.cs @@ -0,0 +1,51 @@ +namespace PaperlessServices.Tests.Unit; + +public sealed class FakeLoggerExtensionsTests +{ + [Fact] + public async Task WaitForLogAsync_WhenConditionIsAlreadyMet_ReturnsTrue() + { + FakeLogCollector collector = new(); + FakeLogger logger = new(collector); + logger.LogInformation("Ready"); + + bool result = await collector.WaitForLogAsync( + logs => logs.Any(log => log.Message == "Ready"), + timeout: TimeSpan.FromMilliseconds(50), + pollInterval: TimeSpan.FromMilliseconds(1), + cancellationToken: TestContext.Current.CancellationToken); + + result.Should().BeTrue(); + } + + [Fact] + public async Task WaitForLogAsync_WhenTimeoutExpires_ReturnsFalse() + { + FakeLogCollector collector = new(); + + bool result = await collector.WaitForLogAsync( + _ => false, + timeout: TimeSpan.FromMilliseconds(20), + pollInterval: TimeSpan.FromMilliseconds(1), + cancellationToken: TestContext.Current.CancellationToken); + + result.Should().BeFalse(); + } + + [Fact] + public async Task WaitForLogAsync_WhenCallerCancels_PropagatesCancellation() + { + FakeLogCollector collector = new(); + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + + Func act = () => collector.WaitForLogAsync( + _ => false, + timeout: TimeSpan.FromSeconds(1), + pollInterval: TimeSpan.FromMilliseconds(1), + cancellationToken: cancellation.Token); + + (await act.Should().ThrowExactlyAsync()) + .Which.CancellationToken.Should().Be(cancellation.Token); + } +} diff --git a/PaperlessServices.Tests/Unit/OcrProcessorTests.cs b/PaperlessServices.Tests/Unit/OcrProcessorTests.cs index 723878c..e58431a 100644 --- a/PaperlessServices.Tests/Unit/OcrProcessorTests.cs +++ b/PaperlessServices.Tests/Unit/OcrProcessorTests.cs @@ -129,9 +129,10 @@ public async Task ProcessDocumentAsync_ValidCommand_ReturnsCompletedOcrEvent() { // Arrange OcrCommand command = CreateCommand(); + MemoryStream pdfStream = CreateValidPdfStream(); _storage.Setup(s => s.DownloadAsync(ValidStoragePath, It.IsAny())) - .ReturnsAsync(CreateValidPdfStream()); + .ReturnsAsync(pdfStream); _pdfExtractor.Setup(p => p.ExtractTextAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(ExtractedOcrText); _searchIndex.Setup(i => i.IndexDocumentAsync( @@ -149,6 +150,7 @@ public async Task ProcessDocumentAsync_ValidCommand_ReturnsCompletedOcrEvent() result.Value.JobId.Should().Be(s_testJobId); result.Value.Status.Should().Be("Completed"); result.Value.Text.Should().Be(ExtractedOcrText); + pdfStream.CanRead.Should().BeFalse("OcrProcessor owns the downloaded stream"); } // ═══════════════════════════════════════════════════════════════ @@ -270,6 +272,35 @@ public async Task ProcessDocumentAsync_StorageFails_DoesNotCallOcr() _pdfExtractor.Verify(p => p.ExtractTextAsync(It.IsAny(), It.IsAny()), Times.Never); } + [Fact] + public async Task ProcessDocumentAsync_CallerCancelsDownload_PropagatesExactCancellationAndStopsProcessing() + { + OcrCommand command = CreateCommand(); + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + OperationCanceledException expected = new(cancellation.Token); + _storage.Setup(s => s.DownloadAsync(ValidStoragePath, cancellation.Token)) + .ThrowsAsync(expected); + + Func act = () => CreateSut().ProcessDocumentAsync(command, cancellation.Token); + + OperationCanceledException thrown = + (await act.Should().ThrowExactlyAsync()).Which; + thrown.Should().BeSameAs(expected); + thrown.CancellationToken.Should().Be(cancellation.Token); + _pdfExtractor.Verify( + p => p.ExtractTextAsync(It.IsAny(), It.IsAny()), + Times.Never); + _searchIndex.Verify( + s => s.IndexDocumentAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny()), + Times.Never); + } + // ═══════════════════════════════════════════════════════════════ // TESTS: ProcessDocumentAsync - OCR Failure // ═══════════════════════════════════════════════════════════════ @@ -279,9 +310,10 @@ public async Task ProcessDocumentAsync_OcrFails_ReturnsError() { // Arrange OcrCommand command = CreateCommand(); + MemoryStream pdfStream = CreateValidPdfStream(); _storage.Setup(s => s.DownloadAsync(ValidStoragePath, It.IsAny())) - .ReturnsAsync(CreateValidPdfStream()); + .ReturnsAsync(pdfStream); _pdfExtractor.Setup(p => p.ExtractTextAsync(It.IsAny(), It.IsAny())) .ReturnsAsync(Error.Failure("Ocr.Failed", "OCR extraction failed")); @@ -294,6 +326,7 @@ public async Task ProcessDocumentAsync_OcrFails_ReturnsError() // Assert result.IsError.Should().BeTrue(); result.FirstError.Code.Should().Be("Ocr.Failed"); + pdfStream.CanRead.Should().BeFalse("OcrProcessor owns the downloaded stream on failure"); } [Fact] diff --git a/PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs b/PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs index 38b4cbb..834421f 100644 --- a/PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs +++ b/PaperlessServices.Tests/Unit/ServiceCollectionExtensionsTests.cs @@ -20,11 +20,10 @@ private static IConfiguration BuildConfiguration(Dictionary? ov Dictionary settings = new() { // MinioOptions (Storage:Minio) — required for validate-on-start - ["Storage:Minio:Endpoint"] = "minio:9000", + ["Storage:Minio:Endpoint"] = "http://minio:9000", ["Storage:Minio:AccessKey"] = "minioadmin", ["Storage:Minio:SecretKey"] = "minioadmin", ["Storage:Minio:BucketName"] = "documents", - ["Storage:Minio:UseSsl"] = "false", // ElasticsearchOptions — required for validate-on-start ["Elasticsearch:Uri"] = "http://elasticsearch:9200", ["Elasticsearch:DefaultIndex"] = "documents", @@ -113,76 +112,45 @@ public void AddOcrServices_NoArg_MinioAndElasticAreSingletons() .Should().Be(ServiceLifetime.Singleton); } - // ═══════════════════════════════════════════════════════════════ - // AddOcrServices() — MinIO endpoint-parsing branches - // ═══════════════════════════════════════════════════════════════ - - [Fact] - public void AddOcrServices_WithSchemelessEndpoint_PrefixesHttp() + [Theory] + [InlineData("http://minio.local:9000")] + [InlineData("https://minio.local:9443")] + public void AddOcrServices_WithAbsoluteHttpEndpoint_BuildsClient(string endpoint) { - // Arrange — host:port form (the common case from compose.yaml / Testcontainers) ServiceCollection services = new(); services.AddSingleton(BuildConfiguration(new Dictionary { - ["Storage:Minio:Endpoint"] = "minio.local:9000" + ["Storage:Minio:Endpoint"] = endpoint })); services.AddLogging(); - services.AddOcrServices(); - using ServiceProvider sp = services.BuildServiceProvider(); - - // Act — resolving the singleton runs the parsing branch - IMinioClient client = sp.GetRequiredService(); - - // Assert — client constructs without throwing; the schemeless path - // (`if (!endpoint.Contains("://"))` => true) is executed. - client.Should().NotBeNull(); + using ServiceProvider provider = services.BuildServiceProvider(); + provider.GetRequiredService().Should().NotBeNull(); } - [Fact] - public void AddOcrServices_WithSchemedEndpoint_UsesEndpointVerbatim() + [Theory] + [InlineData("minio.local:9000")] + [InlineData("ftp://minio.local:21")] + [InlineData("http://user:password@minio.local:9000")] + [InlineData("http://minio.local:9000/prefix")] + [InlineData("http://minio.local:9000?region=local")] + [InlineData("http://minio.local:9000#fragment")] + public void AddOcrServices_WithInvalidEndpoint_RejectsOptions(string endpoint) { - // Arrange — full URI form. Production short-circuits the http:// prefix. ServiceCollection services = new(); services.AddSingleton(BuildConfiguration(new Dictionary { - ["Storage:Minio:Endpoint"] = "http://minio.local:9000" + ["Storage:Minio:Endpoint"] = endpoint })); services.AddLogging(); - services.AddOcrServices(); - using ServiceProvider sp = services.BuildServiceProvider(); - - // Act — `if (!endpoint.Contains("://"))` is false, so prefix is skipped - IMinioClient client = sp.GetRequiredService(); - - // Assert - client.Should().NotBeNull(); - } - - [Fact] - public void AddOcrServices_WithUseSslTrue_BuildsClientWithSsl() - { - // Arrange — flipping UseSsl exercises the WithSSL(true) branch - ServiceCollection services = new(); - services.AddSingleton(BuildConfiguration(new Dictionary - { - ["Storage:Minio:Endpoint"] = "minio.secure:443", - ["Storage:Minio:UseSsl"] = "true" - })); - services.AddLogging(); - - services.AddOcrServices(); - - using ServiceProvider sp = services.BuildServiceProvider(); - - // Act - IMinioClient client = sp.GetRequiredService(); + using ServiceProvider provider = services.BuildServiceProvider(); + Action resolve = () => _ = provider.GetRequiredService>().Value; - // Assert - client.Should().NotBeNull(); + resolve.Should().Throw() + .WithMessage("*absolute HTTP or HTTPS origin*"); } // ═══════════════════════════════════════════════════════════════ diff --git a/PaperlessServices.Tests/Unit/StorageServiceTests.cs b/PaperlessServices.Tests/Unit/StorageServiceTests.cs index 5d4b8ba..1927512 100644 --- a/PaperlessServices.Tests/Unit/StorageServiceTests.cs +++ b/PaperlessServices.Tests/Unit/StorageServiceTests.cs @@ -8,7 +8,7 @@ public sealed class StorageServiceTests : IDisposable private const string ValidFilePath = "documents/2024-01/test-document.pdf"; private const string TestBucketName = "test-bucket"; - private const string TestEndpoint = "localhost:9000"; + private const string TestEndpoint = "http://localhost:9000"; private const string TestAccessKey = "minioadmin"; private const string TestSecretKey = "minioadmin"; private readonly FakeLogCollector _logCollector = new(); @@ -28,7 +28,7 @@ public StorageServiceTests() _minioClient = _mocks.Create(); _options = Options.Create(new MinioOptions { - Endpoint = TestEndpoint, + Endpoint = new Uri(TestEndpoint), AccessKey = TestAccessKey, SecretKey = TestSecretKey, BucketName = TestBucketName @@ -217,4 +217,21 @@ public async Task DownloadAsync_BucketNotFound_ThrowsBucketNotFoundException() // Assert await act.Should().ThrowExactlyAsync(); } + + [Fact] + public async Task DownloadAsync_CanceledRequest_PropagatesExactCallerCancellation() + { + using CancellationTokenSource cancellation = new(); + await cancellation.CancelAsync(); + OperationCanceledException expected = new(cancellation.Token); + _minioClient.Setup(m => m.GetObjectAsync(It.IsAny(), cancellation.Token)) + .ThrowsAsync(expected); + + Func act = () => CreateSut().DownloadAsync(ValidFilePath, cancellation.Token); + + OperationCanceledException thrown = + (await act.Should().ThrowExactlyAsync()).Which; + thrown.Should().BeSameAs(expected); + thrown.CancellationToken.Should().Be(cancellation.Token); + } } diff --git a/PaperlessServices/Configuration/MinioOptions.cs b/PaperlessServices/Configuration/MinioOptions.cs index 3aca4c0..cb191e2 100644 --- a/PaperlessServices/Configuration/MinioOptions.cs +++ b/PaperlessServices/Configuration/MinioOptions.cs @@ -5,7 +5,7 @@ public class MinioOptions public const string SectionName = "Storage:Minio"; [Required(ErrorMessage = "MinIO endpoint is required")] - public string Endpoint { get; set; } = null!; + public Uri Endpoint { get; set; } = null!; [Required(ErrorMessage = "MinIO access key is required")] public string AccessKey { get; set; } = null!; @@ -16,5 +16,4 @@ public class MinioOptions [Required(ErrorMessage = "MinIO bucket name is required")] public string BucketName { get; set; } = null!; - public bool UseSsl { get; set; } } diff --git a/PaperlessServices/Features/OcrProcessing/Application/OcrProcessor.cs b/PaperlessServices/Features/OcrProcessing/Application/OcrProcessor.cs index 5ad7ea9..9d5828c 100644 --- a/PaperlessServices/Features/OcrProcessing/Application/OcrProcessor.cs +++ b/PaperlessServices/Features/OcrProcessing/Application/OcrProcessor.cs @@ -55,6 +55,10 @@ private async Task> DownloadAsync(string filePath, CancellationT Stream stream = await storageService.DownloadAsync(filePath, cancellationToken); return stream; } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } catch (Exception ex) { logger.LogError(ex, "Failed to download file: {FilePath}", filePath); diff --git a/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/CreatePdfExtractor.cs b/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/CreatePdfExtractor.cs index fb23500..a08920f 100644 --- a/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/CreatePdfExtractor.cs +++ b/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/CreatePdfExtractor.cs @@ -9,9 +9,7 @@ public async Task> ExtractTextAsync(Stream pdfStream, Cancellati { try { - cancellationToken.ThrowIfCancellationRequested(); string text = await Pdf.Load(pdfStream).OcrAsync(options: null, cancellationToken); - cancellationToken.ThrowIfCancellationRequested(); if (string.IsNullOrWhiteSpace(text)) { @@ -21,7 +19,7 @@ public async Task> ExtractTextAsync(Stream pdfStream, Cancellati logger.LogInformation("Extracted {CharCount} characters from PDF", text.Length); return text; } - catch (Exception ex) + catch (Exception ex) when (ex is not OperationCanceledException) { logger.LogError(ex, "OCR extraction failed"); return OcrErrors.ExtractionFailed(ex.Message); diff --git a/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/IPdfExtractor.cs b/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/IPdfExtractor.cs index 4b55f7d..432b008 100644 --- a/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/IPdfExtractor.cs +++ b/PaperlessServices/Features/OcrProcessing/Infrastructure/PdfExtractor/IPdfExtractor.cs @@ -10,9 +10,7 @@ public interface IPdfExtractor /// /// The PDF file stream to process. /// - /// Cooperative cancellation. Checked before and after the underlying OCR call - /// because the library does not natively accept a token; honours graceful - /// shutdown so the slowest worker step does not block container teardown. + /// Cooperative cancellation passed through to CreatePdf.NET and propagated to the caller. /// /// Extracted text or error. Task> ExtractTextAsync(Stream pdfStream, CancellationToken cancellationToken = default); diff --git a/PaperlessServices/Features/OcrProcessing/Infrastructure/Storage/StorageService.cs b/PaperlessServices/Features/OcrProcessing/Infrastructure/Storage/StorageService.cs index 326b3b3..4467bd2 100644 --- a/PaperlessServices/Features/OcrProcessing/Infrastructure/Storage/StorageService.cs +++ b/PaperlessServices/Features/OcrProcessing/Infrastructure/Storage/StorageService.cs @@ -10,13 +10,21 @@ public async Task DownloadAsync(string filePath, CancellationToken cance { MemoryStream stream = new(); - await minio.GetObjectAsync( - new GetObjectArgs() - .WithBucket(options.Value.BucketName) - .WithObject(filePath) - .WithCallbackStream(async (s, ct) => await s.CopyToAsync(stream, ct)), - cancellationToken - ); + try + { + await minio.GetObjectAsync( + new GetObjectArgs() + .WithBucket(options.Value.BucketName) + .WithObject(filePath) + .WithCallbackStream(async (s, ct) => await s.CopyToAsync(stream, ct)), + cancellationToken + ); + } + catch + { + stream.Dispose(); + throw; + } stream.Position = 0; logger.LogInformation("Downloaded file from storage: {FilePath}", filePath); diff --git a/PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs b/PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs index 158131a..f83667b 100644 --- a/PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs +++ b/PaperlessServices/Host/Extensions/ServiceCollectionExtensions.cs @@ -33,25 +33,25 @@ private IServiceCollection AddMinioStorage() services .AddOptionsWithValidateOnStart() .BindConfiguration(MinioOptions.SectionName) - .ValidateDataAnnotations(); + .ValidateDataAnnotations() + .Validate( + static options => options.Endpoint is { IsAbsoluteUri: true } endpoint && + (endpoint.Scheme == Uri.UriSchemeHttp || + endpoint.Scheme == Uri.UriSchemeHttps) && + string.IsNullOrEmpty(endpoint.UserInfo) && + endpoint.AbsolutePath == "/" && + string.IsNullOrEmpty(endpoint.Query) && + string.IsNullOrEmpty(endpoint.Fragment), + $"{MinioOptions.SectionName}:Endpoint must be an absolute HTTP or HTTPS origin"); services.AddSingleton(sp => { MinioOptions options = sp.GetRequiredService>().Value; - // Parse endpoint to handle both "host:port" and "http://host:port" formats - string endpoint = options.Endpoint; - if (!endpoint.Contains("://")) - { - endpoint = $"http://{endpoint}"; - } - - Uri uri = new(endpoint); - return new MinioClient() - .WithEndpoint(uri.Host, uri.Port) + .WithEndpoint(options.Endpoint.Host, options.Endpoint.Port) .WithCredentials(options.AccessKey, options.SecretKey) - .WithSSL(options.UseSsl) + .WithSSL(options.Endpoint.Scheme == Uri.UriSchemeHttps) .Build(); }); diff --git a/PaperlessUI.Angular/src/app/core/api/generated/api-types.ts b/PaperlessUI.Angular/src/app/core/api/generated/api-types.ts index eb441cc..2ff81de 100644 --- a/PaperlessUI.Angular/src/app/core/api/generated/api-types.ts +++ b/PaperlessUI.Angular/src/app/core/api/generated/api-types.ts @@ -24,14 +24,6 @@ export interface components { cursor?: string | null; }; - /** Query parameters for full-text document search. */ - SearchQuery: { - /** 1..100 chars. */ - query: string; - /** 1..100. Non-nullable C# `int` (server-defaulted to DefaultResultLimit). */ - limit: number; - }; - /** Document metadata returned by the API (GET /documents, GET /documents/{id}). */ DocumentDto: { /** uuid */ @@ -102,7 +94,6 @@ export interface components { } export type PaginationQuery = components['schemas']['PaginationQuery']; -export type SearchQuery = components['schemas']['SearchQuery']; export type DocumentDto = components['schemas']['DocumentDto']; export type CreateDocumentResponse = components['schemas']['CreateDocumentResponse']; export type DocumentSearchResultDto = components['schemas']['DocumentSearchResultDto']; diff --git a/Pipeline/Build.csproj b/Pipeline/Build.csproj index 95d09e8..930423a 100644 --- a/Pipeline/Build.csproj +++ b/Pipeline/Build.csproj @@ -27,16 +27,10 @@ - + - + From e57bf512e47cfae268616b461d4e287e6e217e88 Mon Sep 17 00:00:00 2001 From: ancplua Date: Sun, 2 Aug 2026 00:41:35 +0200 Subject: [PATCH 2/2] Fix CI naming analyzer violations --- PaperlessREST.Tests/Integration/DocumentEndpointTests.cs | 6 +++--- PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs b/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs index 13165a6..43fb377 100644 --- a/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs +++ b/PaperlessREST.Tests/Integration/DocumentEndpointTests.cs @@ -210,17 +210,17 @@ public async Task GetById_ExistingDocument_ReturnsDocument() [Fact] public async Task GetSummary_ExistingDocument_ReturnsSummary() { - const string summary = "A concise account statement summary."; + const string Summary = "A concise account statement summary."; var docId = await SeedDocumentAsync( $"{TestFilePrefix}-summary-{Guid.NewGuid():N}.pdf", - summary); + Summary); using var response = await _fixture.Client.GetAsync( $"{DocumentsEndpoint}/{docId}/summary", TestContext.Current.CancellationToken); var result = await ReadSuccessJsonAsync(response); - result.Summary.Should().Be(summary); + result.Summary.Should().Be(Summary); } #endregion diff --git a/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs b/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs index a368c98..c48f796 100644 --- a/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs +++ b/PaperlessREST.Tests/Unit/BatchOrchestratorTests.cs @@ -374,11 +374,11 @@ public async Task SingleValidFile_LogsSuccessfulProcessing() public async Task OriginalFileNameContainingProcessing_PreservesEmbeddedTextWhenArchived() { // Arrange - const string originalFileName = "report.processing.xml"; - CreateTestFile(originalFileName); + const string OriginalFileName = "report.processing.xml"; + CreateTestFile(OriginalFileName); _reportProcessor.Setup(p => p.ProcessAsync( - It.Is(path => path.EndsWith($"{originalFileName}.processing", StringComparison.Ordinal)), + It.Is(path => path.EndsWith($"{OriginalFileName}.processing", StringComparison.Ordinal)), It.IsAny())) .ReturnsAsync(new ProcessingResult(1, 0)); @@ -390,7 +390,7 @@ public async Task OriginalFileNameContainingProcessing_PreservesEmbeddedTextWhen // Assert _fileSystem.Directory.GetFiles(ArchivePath) .Should().ContainSingle(path => Path.GetFileName(path) - .StartsWith($"{originalFileName}.", StringComparison.Ordinal)); + .StartsWith($"{OriginalFileName}.", StringComparison.Ordinal)); } #endregion