From 53ca097e4c29ddf39bfeb2c65f8a94288443225d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vedran=20Bilopavlovi=C4=87?= Date: Sat, 13 Jun 2026 13:11:23 +0200 Subject: [PATCH 1/5] =?UTF-8?q?fix(docker):=20JIT=20image=20=E2=80=94=20co?= =?UTF-8?q?py=20NpgsqlRest.Common=20shared=20source;=20CI:=20stop=20publis?= =?UTF-8?q?hing=20on=20PRs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The JIT Docker build (docker/Dockerfile.jit) compiles from source inside the container, and NpgsqlRest.csproj now compiles + global-uses the shared source in NpgsqlRest.Common/ (). That folder was never copied into the build context, so the publish failed with CS0234 'NpgsqlRest.Common'. Added the COPY; validated by building the image locally. (The other Docker variants copy a prebuilt binary, so they were fine.) CI: removed the pull_request trigger so the publish/release pipeline no longer runs on every PR (dependabot PRs were flooding Actions with failing publish runs — PRs can't publish anyway, no secrets). TEMP (this commit only): build-test-publish is if:false and build-docker-jit is detached from create-release, so this push runs ONLY build-docker-jit to push the missing v3.17.0-jit / latest-jit image. To be reverted once green. --- .github/workflows/build-test-publish.yml | 8 +++++--- docker/Dockerfile.jit | 3 +++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build-test-publish.yml b/.github/workflows/build-test-publish.yml index 3e5b8e75..fb10a49c 100644 --- a/.github/workflows/build-test-publish.yml +++ b/.github/workflows/build-test-publish.yml @@ -3,8 +3,6 @@ name: Build, Test, Publish and Release on: push: branches: [ master ] - pull_request: - branches: [ master ] workflow_dispatch: # Add workflow-level permissions @@ -14,6 +12,9 @@ permissions: jobs: build-test-publish: + # TEMP (jit-only republish of v3.17.0): disabled so the whole release chain is skipped — 3.17.0 is + # already published, we only need the missing -jit Docker image. RESTORE: delete this `if: false`. + if: false runs-on: ubuntu-22.04 services: postgres: @@ -275,7 +276,8 @@ jobs: vbilopav/npgsqlrest:latest-aot build-docker-jit: - needs: create-release + # TEMP (jit-only republish): `needs: create-release` removed so this runs standalone while the rest of + # the chain is disabled. It reads the version from version.txt itself. RESTORE: re-add `needs: create-release`. runs-on: ubuntu-22.04 permissions: contents: read diff --git a/docker/Dockerfile.jit b/docker/Dockerfile.jit index f5c86336..5b0ce475 100644 --- a/docker/Dockerfile.jit +++ b/docker/Dockerfile.jit @@ -21,6 +21,9 @@ RUN dotnet restore NpgsqlRestClient/NpgsqlRestClient.csproj COPY LICENSE LICENSE COPY README.md README.MD COPY NpgsqlRest/ NpgsqlRest/ +# Shared source compiled into NpgsqlRest.csproj ( + global +# usings). Without this the JIT build fails: NpgsqlRest.GlobalUsings.g.cs -> CS0234 'NpgsqlRest.Common'. +COPY NpgsqlRest.Common/ NpgsqlRest.Common/ COPY NpgsqlRestClient/*.cs NpgsqlRestClient/ COPY NpgsqlRestClient/*.csproj NpgsqlRestClient/ COPY NpgsqlRestClient/appsettings.json NpgsqlRestClient/ From 1f6e4074620f85bcec8a073c8683ddfec2e7d78e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vedran=20Bilopavlovi=C4=87?= Date: Sat, 13 Jun 2026 13:12:25 +0200 Subject: [PATCH 2/5] docs(changelog): mark v3.17.0 released (2026-06-13); CI: restore full release workflow Re-enables the full release chain (build-test-publish no longer if:false; build-docker-jit needs create-release again) now that the v3.17.0-jit image is published. The pull_request trigger stays removed, so the publish/release pipeline runs only on push to master + manual dispatch (no PR/dependabot noise). --- .github/workflows/build-test-publish.yml | 6 +----- changelog/v3.17.0.md | 2 +- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/.github/workflows/build-test-publish.yml b/.github/workflows/build-test-publish.yml index fb10a49c..e9d337b2 100644 --- a/.github/workflows/build-test-publish.yml +++ b/.github/workflows/build-test-publish.yml @@ -12,9 +12,6 @@ permissions: jobs: build-test-publish: - # TEMP (jit-only republish of v3.17.0): disabled so the whole release chain is skipped — 3.17.0 is - # already published, we only need the missing -jit Docker image. RESTORE: delete this `if: false`. - if: false runs-on: ubuntu-22.04 services: postgres: @@ -276,8 +273,7 @@ jobs: vbilopav/npgsqlrest:latest-aot build-docker-jit: - # TEMP (jit-only republish): `needs: create-release` removed so this runs standalone while the rest of - # the chain is disabled. It reads the version from version.txt itself. RESTORE: re-add `needs: create-release`. + needs: create-release runs-on: ubuntu-22.04 permissions: contents: read diff --git a/changelog/v3.17.0.md b/changelog/v3.17.0.md index f5b52704..4ee8ac7d 100644 --- a/changelog/v3.17.0.md +++ b/changelog/v3.17.0.md @@ -1,6 +1,6 @@ # Changelog v3.17.0 -## Version [3.17.0](https://github.com/NpgsqlRest/NpgsqlRest/tree/3.17.0) (unreleased) +## Version [3.17.0](https://github.com/NpgsqlRest/NpgsqlRest/tree/3.17.0) (2026-06-13) [Full Changelog](https://github.com/NpgsqlRest/NpgsqlRest/compare/3.16.3...3.17.0) From bf78434dc6c95436d34f710d11851ae36e164d0a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vedran=20Bilopavlovi=C4=87?= Date: Tue, 23 Jun 2026 11:47:02 +0200 Subject: [PATCH 3/5] fix(http-types): fire one outbound call per distinct HTTP type (v3.17.1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A DB-function composite parameter is expanded into one parameter per field, each carrying the same TypeDescriptor.CustomType. The per-request list of HTTP types therefore held the type name once per field, and InvokeAllAsync fired InvokeAsync once per entry — making N identical outbound calls (N = field count) while the fill loop resolved handlers by distinct type name. SQL-file endpoints kept the type as a single composite parameter and were unaffected. Guard the firing loop to request each distinct HTTP type once, reusing the dictionary the fill loop already keys on. Preserves the established one-call-per-distinct-type contract. Adds two regression tests that count actual outbound calls via a WireMock callback: a 6-field type fires exactly 1 call (was 6), and two distinct types fire 1 call each. Verified failing pre-fix (found 6). See changelog/v3.17.1.md. --- .../HttpClientType/HttpClientTypeHandler.cs | 8 +++ .../HttpClientTypeTests.cs | 72 +++++++++++++++++++ changelog/v3.17.1.md | 45 ++++++++++++ npm/package.json | 2 +- version.txt | 2 +- 5 files changed, 127 insertions(+), 2 deletions(-) create mode 100644 changelog/v3.17.1.md diff --git a/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs b/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs index 41a126c1..96add620 100644 --- a/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs +++ b/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs @@ -284,6 +284,14 @@ public static async Task InvokeAllAsync( foreach (var typeName in typeNames) { + // A DB-function composite parameter is expanded into one parameter per field, + // each carrying the same CustomType, so typeNames can contain the same type + // multiple times. Fire exactly one outbound call per distinct HTTP type - the + // fill loop below resolves handlers by distinct type name as well. + if (handlers.ContainsKey(typeName)) + { + continue; + } if (HttpClientTypes.Definitions.TryGetValue(typeName, out var definition)) { var handler = new HttpClientTypeHandler(definition, replacements); diff --git a/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeTests.cs b/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeTests.cs index d4e6bfad..9cd232b7 100644 --- a/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeTests.cs +++ b/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeTests.cs @@ -2,6 +2,9 @@ using WireMock.RequestBuilders; using WireMock.ResponseBuilders; using WireMock.Settings; +using WireMock; +using WireMock.Types; +using WireMock.Util; namespace NpgsqlRestTests; @@ -879,4 +882,73 @@ public async Task Test_get_http_api_multi_types_one_fails() content.Should().Contain("\"users_status\" : 200"); content.Should().Contain("\"products_status\" : 500"); } + + // Regression: a DB-function composite parameter is expanded into one parameter per field, + // each carrying the same HTTP CustomType. Before the InvokeAllAsync dedup guard, the request + // fired once per field (6 outbound calls for the 6-field all-fields type). It must fire exactly + // once per distinct HTTP type. Counter-based to count actual outbound calls (same pattern as + // the retry tests). + [Fact] + public async Task Test_multi_field_type_fires_exactly_one_outbound_call() + { + int calls = 0; + _server + .Given(Request.Create().WithPath("/api/all-fields").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + Interlocked.Increment(ref calls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "response body", DetectedBodyType = BodyType.String } + }; + })); + + using var response = await _test.Client.GetAsync("/api/get-http-api-all-fields/"); + var content = await response.Content.ReadAsStringAsync(); + + response?.StatusCode.Should().Be(HttpStatusCode.OK); + content.Should().Contain("\"body\" : \"response body\""); + calls.Should().Be(1); + } + + // Regression: two distinct HTTP types referenced by a single function must each fire exactly + // once - one outbound call per distinct type, not per expanded field. + [Fact] + public async Task Test_multi_types_each_fire_exactly_one_outbound_call() + { + int usersCalls = 0; + int productsCalls = 0; + _server + .Given(Request.Create().WithPath("/api/users").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + Interlocked.Increment(ref usersCalls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "[{\"id\": 1}]", DetectedBodyType = BodyType.String } + }; + })); + _server + .Given(Request.Create().WithPath("/api/products").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + Interlocked.Increment(ref productsCalls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "[{\"id\": 101}]", DetectedBodyType = BodyType.String } + }; + })); + + using var response = await _test.Client.GetAsync("/api/get-http-api-multi-types/"); + var content = await response.Content.ReadAsStringAsync(); + + response?.StatusCode.Should().Be(HttpStatusCode.OK); + content.Should().Contain("\"users_status\" : 200"); + content.Should().Contain("\"products_status\" : 200"); + usersCalls.Should().Be(1); + productsCalls.Should().Be(1); + } } diff --git a/changelog/v3.17.1.md b/changelog/v3.17.1.md new file mode 100644 index 00000000..3d659752 --- /dev/null +++ b/changelog/v3.17.1.md @@ -0,0 +1,45 @@ +# Changelog v3.17.1 + +## Version [3.17.1](https://github.com/NpgsqlRest/NpgsqlRest/tree/3.17.1) + +[Full Changelog](https://github.com/NpgsqlRest/NpgsqlRest/compare/3.17.0...3.17.1) + +Patch release that fixes **duplicate outbound HTTP calls for HTTP Custom Types on database-function endpoints**. + +## The bug + +An endpoint backed by a **database function/procedure** whose parameter is an HTTP Custom Type fired **one outbound HTTP call per field of the type** on every inbound request. A 4-field type made 4 identical calls; a 6-field type made 6 — multiplying latency and load on the target service. SQL-file endpoints were not affected. + +## Cause + +A composite function parameter is expanded into one parameter **per field**, and each of those expanded parameters carries the same `TypeDescriptor.CustomType` (the HTTP type name). The type name was therefore collected once per field, so the per-request list of HTTP types to invoke contained the same name N times. The firing loop in `HttpClientTypeHandler.InvokeAllAsync` then called `InvokeAsync` once per entry, with no dedup — while the fill loop immediately below resolves handlers by **distinct** type name out of a dictionary. The design already assumes exactly one call per distinct type; the firing loop just failed to match, so the extra calls were made and their handlers discarded. + +SQL-file endpoints keep the HTTP type as a single composite parameter (`CustomTypeName` null), so the name was collected once → exactly one call, which is why they were unaffected. + +## What changed + +A one-line guard in `HttpClientTypeHandler.InvokeAllAsync` makes the firing loop request each distinct HTTP type once: + +```csharp +foreach (var typeName in typeNames) +{ + if (handlers.ContainsKey(typeName)) continue; // fire once per distinct type + if (HttpClientTypes.Definitions.TryGetValue(typeName, out var definition)) + { + var handler = new HttpClientTypeHandler(definition, replacements); + handlers[typeName] = handler; + tasks.Add((typeName, handler, handler.InvokeAsync(cancellationToken))); + } +} +``` + +This is the single choke point for firing outbound calls and reuses the same dictionary the fill loop already keys on, so it is robust regardless of how the parameter list is built. The established contract is preserved: one call per **distinct** HTTP type, shared from one response — two parameters referencing the same type still share a single call, and two different types remain two separate calls. + +## Tests + +Two regression tests added (`NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeTests.cs`) that count **actual** outbound calls via a WireMock response callback (the prior suite asserted response content but never call counts): + +- A 6-field HTTP type fires **exactly one** outbound call (was 6 before the fix). +- Two distinct HTTP types in one function each fire **exactly one** call. + +Verified against the pre-fix code: the multi-field test failed with `Expected calls to be 1, but found 6`, reproducing the reported behavior. All 46 HTTP-client-type tests pass with the fix. diff --git a/npm/package.json b/npm/package.json index 484cd0f8..859024b3 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "npgsqlrest", - "version": "3.17.0", + "version": "3.17.1", "description": "Automatic REST API for PostgreSQL Databases Client Build", "scripts": { "postinstall": "node postinstall.js", diff --git a/version.txt b/version.txt index f85bf6e3..0caba260 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -3.17.0 \ No newline at end of file +3.17.1 \ No newline at end of file From 9f67d878a885fb1a4731f57b040efbd545bf1b6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vedran=20Bilopavlovi=C4=87?= Date: Tue, 23 Jun 2026 12:27:35 +0200 Subject: [PATCH 4/5] feat(http-types): response caching via @cache; parse directives after headers (v3.18.0) Add opt-in HTTP Custom Type response caching with a @cache type-comment directive (alongside @timeout/@retry_delay): - GET-only (warn + ignore on non-GET); success-only storage so a transient upstream failure is never pinned for the TTL. - New HttpResponseCache: in-memory store with Lazy stampede coalescing (one outbound call per key), max-entries cap, prune timer. - Cache key = method + resolved URL + content-type + headers + body. - HttpClientOptions.CacheEnabled (global kill switch), MaxCacheEntries, CachePruneIntervalSeconds; full config round-trip wired (Builder, appsettings.json, ConfigTemplate, ConfigDefaults, ConfigSchemaGenerator). Also fix: @timeout/@retry_delay/@cache are now parsed both before the request line AND after the headers. Previously only the leading position was recognized, so a directive after the headers (as the docs showed) was silently ignored. Real headers (e.g. Cache-Control) are unaffected. Folds in the v3.17.1 duplicate-outbound-call fix: version.txt and npm/package.json bumped to 3.18.0, changelog renamed v3.17.1 -> v3.18.0 covering both the feature and the fix. Tests: parse-level @cache forms + both-placement directive tests; integration tests for cache hit, 6-field dedup+cache, error-not-cached, POST-ignored, TTL expiry. Full suite green (2284). --- .../HttpClientType/HttpClientTypeHandler.cs | 72 ++++- NpgsqlRest/HttpClientType/HttpClientTypes.cs | 126 +++++++- .../HttpClientType/HttpResponseCache.cs | 160 ++++++++++ .../HttpClientType/HttpTypeDefinition.cs | 13 + NpgsqlRest/NpgsqlRestBuilder.cs | 9 + NpgsqlRest/Options/HttpClientOptions.cs | 20 ++ NpgsqlRestClient/Builder.cs | 5 +- NpgsqlRestClient/ConfigDefaults.cs | 5 +- NpgsqlRestClient/ConfigSchemaGenerator.cs | 3 + NpgsqlRestClient/ConfigTemplate.cs | 17 +- NpgsqlRestClient/appsettings.json | 17 +- .../HttpClientTypeCacheTests.cs | 289 ++++++++++++++++++ .../ParseHttpTypeDefinitionTests.cs | 144 +++++++++ changelog/v3.17.1.md | 45 --- changelog/v3.18.0.md | 53 ++++ npm/package.json | 2 +- version.txt | 2 +- 17 files changed, 929 insertions(+), 53 deletions(-) create mode 100644 NpgsqlRest/HttpClientType/HttpResponseCache.cs create mode 100644 NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeCacheTests.cs delete mode 100644 changelog/v3.17.1.md create mode 100644 changelog/v3.18.0.md diff --git a/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs b/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs index 96add620..76e6127b 100644 --- a/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs +++ b/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs @@ -160,6 +160,76 @@ public async Task InvokeAsync(CancellationToken cancellationToken = default) } } + /// + /// Runs the outbound call, routing it through when the type opts in + /// via @cache and caching is enabled globally. On a cache hit (or a coalesced concurrent call) + /// the cached response is applied to this handler so the fill loop reads it exactly as a live response. + /// + public async Task InvokeWithCacheAsync(CancellationToken cancellationToken = default) + { + if (!Options.HttpClientOptions.CacheEnabled || !definition.CacheEnabled) + { + await InvokeAsync(cancellationToken); + return; + } + + var key = ComputeCacheKey(); + var cached = await HttpResponseCache.GetOrCreateAsync( + key, + definition.CacheDuration, + async ct => + { + await InvokeAsync(ct); + return SnapshotResponse(); + }, + cancellationToken); + + ApplyResponse(cached); + } + + /// + /// Cache key for this request: method + resolved URL + resolved content-type + resolved headers + /// (sorted) + resolved body. Placeholders are resolved so per-request values vary the key; a type + /// with no placeholders produces a constant key (one shared cached response). + /// + private string ComputeCacheKey() + { + var sb = new StringBuilder(); + sb.Append(definition.Method).Append('\n'); + sb.Append(ResolveValue(definition.Url)).Append('\n'); + if (definition.ContentType is not null) + { + sb.Append(ResolveValue(definition.ContentType)); + } + sb.Append('\n'); + if (definition.Headers is { Count: > 0 }) + { + foreach (var header in definition.Headers.OrderBy(h => h.Key, StringComparer.Ordinal)) + { + sb.Append(header.Key).Append(':').Append(ResolveValue(header.Value)).Append('\n'); + } + } + sb.Append('\n'); + if (definition.Body is not null) + { + sb.Append(ResolveValue(definition.Body)); + } + return sb.ToString(); + } + + private CachedHttpResponse SnapshotResponse() => + new(StatusCode, Body, ResponseHeaders, ContentType, IsSuccess, ErrorMessage); + + private void ApplyResponse(CachedHttpResponse r) + { + StatusCode = r.StatusCode; + Body = r.Body; + ResponseHeaders = r.ResponseHeaders; + ContentType = r.ContentType; + IsSuccess = r.IsSuccess; + ErrorMessage = r.ErrorMessage; + } + private bool ShouldRetry(int statusCode) { if (definition.RetryOnStatusCodes is null) @@ -296,7 +366,7 @@ public static async Task InvokeAllAsync( { var handler = new HttpClientTypeHandler(definition, replacements); handlers[typeName] = handler; - tasks.Add((typeName, handler, handler.InvokeAsync(cancellationToken))); + tasks.Add((typeName, handler, handler.InvokeWithCacheAsync(cancellationToken))); } } diff --git a/NpgsqlRest/HttpClientType/HttpClientTypes.cs b/NpgsqlRest/HttpClientType/HttpClientTypes.cs index 7debf487..49c40c25 100644 --- a/NpgsqlRest/HttpClientType/HttpClientTypes.cs +++ b/NpgsqlRest/HttpClientType/HttpClientTypes.cs @@ -91,6 +91,8 @@ public HttpClientTypes(IApplicationBuilder? builder, RetryStrategy? retryStrateg TimeSpan? timeout = null; TimeSpan[]? retryDelays = null; HashSet? retryOnStatusCodes = null; + bool cacheEnabled = false; + TimeSpan? cacheDuration = null; // Parse directives before the request line while (pos < span.Length) @@ -129,6 +131,17 @@ public HttpClientTypes(IApplicationBuilder? builder, RetryStrategy? retryStrateg continue; } + // Check for cache directive + if (TryParseCacheDirective(trimmedLine, typeName, out var parsedCacheDuration)) + { + cacheEnabled = true; + cacheDuration = parsedCacheDuration; + pos += lineEnd == -1 ? line.Length : lineEnd; + if (pos < span.Length && span[pos] == '\r') pos++; + if (pos < span.Length && span[pos] == '\n') pos++; + continue; + } + // Check for # comment (non-timeout directive) if (trimmedLine[0] == '#') { @@ -198,12 +211,15 @@ public HttpClientTypes(IApplicationBuilder? builder, RetryStrategy? retryStrateg Url = new string(urlSpan), Timeout = timeout, RetryDelays = retryDelays, - RetryOnStatusCodes = retryOnStatusCodes + RetryOnStatusCodes = retryOnStatusCodes, + CacheEnabled = cacheEnabled, + CacheDuration = cacheDuration }; // Move past first line if (firstLineEnd == -1) { + NormalizeCacheDirective(result, typeName); result.NeedsParsing = needsParsing; return result; } @@ -233,6 +249,36 @@ public HttpClientTypes(IApplicationBuilder? builder, RetryStrategy? retryStrateg break; } + // Directives may also appear in the header section (after the request line), not only + // before it. Check for them before treating the line as an HTTP header. A directive's + // separator/value shape means real headers (which carry a 'Name-Word: value') don't match. + if (TryParseTimeoutDirective(line, typeName, out var hdrTimeout)) + { + result.Timeout = hdrTimeout; + pos += lineEnd == -1 ? line.Length : lineEnd; + if (pos < span.Length && span[pos] == '\r') pos++; + if (pos < span.Length && span[pos] == '\n') pos++; + continue; + } + if (TryParseRetryDirective(line, typeName, out var hdrDelays, out var hdrCodes)) + { + result.RetryDelays = hdrDelays; + result.RetryOnStatusCodes = hdrCodes; + pos += lineEnd == -1 ? line.Length : lineEnd; + if (pos < span.Length && span[pos] == '\r') pos++; + if (pos < span.Length && span[pos] == '\n') pos++; + continue; + } + if (TryParseCacheDirective(line, typeName, out var hdrCacheDuration)) + { + result.CacheEnabled = true; + result.CacheDuration = hdrCacheDuration; + pos += lineEnd == -1 ? line.Length : lineEnd; + if (pos < span.Length && span[pos] == '\r') pos++; + if (pos < span.Length && span[pos] == '\n') pos++; + continue; + } + int colonIndex = line.IndexOf(':'); if (colonIndex > 0) { @@ -285,10 +331,33 @@ public HttpClientTypes(IApplicationBuilder? builder, RetryStrategy? retryStrateg } } + NormalizeCacheDirective(result, typeName); result.NeedsParsing = needsParsing; return result; } + // Caching is only safe for GET (idempotent reads). A @cache directive on any other method is + // almost always a mistake; warn and ignore it rather than caching a mutating call. Applied after + // the whole comment is parsed so it sees @cache whether it appeared before the request line or + // among the headers, and after the method is known. + private static void NormalizeCacheDirective(HttpTypeDefinition result, string? typeName) + { + if (!result.CacheEnabled) + { + return; + } + if (!string.Equals(result.Method, "GET", StringComparison.Ordinal)) + { + Logger?.LogWarning("Type '{TypeName}': @cache is only supported for GET requests; ignoring it for '{Method}'", typeName, result.Method); + result.CacheEnabled = false; + result.CacheDuration = null; + } + else if (result.CacheDuration is null) + { + Logger?.LogWarning("Type '{TypeName}': @cache has no expiration interval; responses will be cached until the process restarts", typeName); + } + } + private static ReadOnlySpan TrimSpan(ReadOnlySpan span) { int start = 0; @@ -548,4 +617,59 @@ private static bool TryParseRetryDirective( return true; } + + private static bool TryParseCacheDirective(ReadOnlySpan line, string? typeName, out TimeSpan? duration) + { + duration = null; + var trimmed = TrimSpan(line); + + // Remove leading # if present + if (!trimmed.IsEmpty && trimmed[0] == '#') + { + trimmed = TrimSpan(trimmed[1..]); + } + + // Remove leading @ if present + if (!trimmed.IsEmpty && trimmed[0] == '@') + { + trimmed = TrimSpan(trimmed[1..]); + } + + // Check for "cache" keyword (case-insensitive). Must be the whole token, so "cache_profile" + // or other future "cache*" directives are not swallowed here. + if (!trimmed.StartsWith("cache", StringComparison.OrdinalIgnoreCase)) + { + return false; + } + + var afterKeyword = trimmed[5..]; // Skip "cache" + + // Bare "@cache" (no value) → enabled with no expiration. + if (afterKeyword.IsEmpty) + { + return true; + } + + // The next char must be a separator (space, '=', ':'); otherwise this is a different keyword. + char sep = afterKeyword[0]; + if (sep != ' ' && sep != '\t' && sep != '=' && sep != ':') + { + return false; + } + + if (sep == '=' || sep == ':') + { + afterKeyword = afterKeyword[1..]; + } + afterKeyword = TrimSpan(afterKeyword); + + // "@cache" followed only by separator → enabled with no expiration. + if (afterKeyword.IsEmpty) + { + return true; + } + + duration = ParseTimeoutValue(afterKeyword, typeName); + return true; + } } \ No newline at end of file diff --git a/NpgsqlRest/HttpClientType/HttpResponseCache.cs b/NpgsqlRest/HttpClientType/HttpResponseCache.cs new file mode 100644 index 00000000..df1d5099 --- /dev/null +++ b/NpgsqlRest/HttpClientType/HttpResponseCache.cs @@ -0,0 +1,160 @@ +using System.Collections.Concurrent; + +namespace NpgsqlRest.HttpClientType; + +/// +/// Immutable snapshot of an outbound HTTP type response, suitable for caching and reuse. +/// +public sealed record CachedHttpResponse( + int StatusCode, + string? Body, + string? ResponseHeaders, + string? ContentType, + bool IsSuccess, + string? ErrorMessage); + +/// +/// In-memory cache of HTTP type responses with stampede protection. A burst of concurrent requests +/// for the same key coalesce into a single outbound call; the rest await the in-flight result. +/// Only successful responses are stored, so a transient upstream failure is never pinned for the TTL. +/// +public static class HttpResponseCache +{ + private sealed class CacheEntry + { + public required CachedHttpResponse Value { get; init; } + public DateTime? ExpirationTime { get; init; } + public bool IsExpired => ExpirationTime.HasValue && DateTime.UtcNow > ExpirationTime.Value; + } + + private static readonly ConcurrentDictionary _cache = new(StringComparer.Ordinal); + // In-flight factory invocations keyed by cache key. The value is a Lazy so that even if + // ConcurrentDictionary.GetOrAdd runs its value-factory more than once under contention, only the + // stored Lazy's task is ever started — guaranteeing a single outbound call per key. + private static readonly ConcurrentDictionary>> _inflight = new(StringComparer.Ordinal); + + private static Timer? _cleanupTimer; + private static int _maxEntries = 10_000; + + public static void Start(NpgsqlRestOptions options) + { + _maxEntries = options.HttpClientOptions.MaxCacheEntries; + var interval = TimeSpan.FromSeconds(options.HttpClientOptions.CachePruneIntervalSeconds); + _cleanupTimer?.Dispose(); + _cleanupTimer = new Timer(_ => CleanupExpiredEntries(), null, interval, interval); + } + + public static void Shutdown() + { + _cleanupTimer?.Dispose(); + _cleanupTimer = null; + _cache.Clear(); + _inflight.Clear(); + } + + private static void CleanupExpiredEntries() + { + foreach (var kvp in _cache) + { + if (kvp.Value.IsExpired) + { + _cache.TryRemove(kvp.Key, out _); + } + } + } + + private static bool TryGet(string key, out CachedHttpResponse value) + { + if (_cache.TryGetValue(key, out var entry)) + { + if (entry.IsExpired) + { + _cache.TryRemove(key, out _); + value = null!; + return false; + } + value = entry.Value; + return true; + } + value = null!; + return false; + } + + private static void Store(string key, CachedHttpResponse value, TimeSpan? ttl) + { + // Bound memory: once full, don't admit new keys (existing entries still serve and expire). + // Updates to an already-cached key are always allowed. + if (_cache.Count >= _maxEntries && !_cache.ContainsKey(key)) + { + return; + } + + _cache[key] = new CacheEntry + { + Value = value, + ExpirationTime = ttl.HasValue ? DateTime.UtcNow + ttl.Value : null + }; + } + + /// + /// Returns the cached response for if present and unexpired; otherwise runs + /// (the outbound call), stores the result when it is successful, and + /// returns it. Concurrent callers for the same key coalesce into one factory invocation. + /// + public static async Task GetOrCreateAsync( + string key, + TimeSpan? ttl, + Func> factory, + CancellationToken cancellationToken) + { + while (true) + { + cancellationToken.ThrowIfCancellationRequested(); + + if (TryGet(key, out var cached)) + { + return cached; + } + + var lazy = _inflight.GetOrAdd( + key, + _ => new Lazy>(() => RunFactoryAsync(key, ttl, factory, cancellationToken))); + + try + { + return await lazy.Value.WaitAsync(cancellationToken); + } + catch (OperationCanceledException) + { + if (cancellationToken.IsCancellationRequested) + { + throw; + } + // The lead caller (whose token drove the shared run) was cancelled before this waiter. + // Loop and retry — the value may now be cached, or this caller becomes the new lead. + } + } + } + + private static async Task RunFactoryAsync( + string key, + TimeSpan? ttl, + Func> factory, + CancellationToken cancellationToken) + { + try + { + var result = await factory(cancellationToken).ConfigureAwait(false); + // Success-only: never pin a transient upstream failure for the whole TTL. + if (result.IsSuccess) + { + Store(key, result, ttl); + } + return result; + } + finally + { + _inflight.TryRemove(key, out _); + } + } +} diff --git a/NpgsqlRest/HttpClientType/HttpTypeDefinition.cs b/NpgsqlRest/HttpClientType/HttpTypeDefinition.cs index 6cb1ffb8..045a3960 100644 --- a/NpgsqlRest/HttpClientType/HttpTypeDefinition.cs +++ b/NpgsqlRest/HttpClientType/HttpTypeDefinition.cs @@ -11,4 +11,17 @@ public class HttpTypeDefinition public TimeSpan[]? RetryDelays { get; set; } public HashSet? RetryOnStatusCodes { get; set; } public bool NeedsParsing { get; set; } + + /// + /// True when the type comment carries a @cache directive, opting this HTTP type into + /// response caching. Successful responses are cached and reused for matching requests + /// (same method + resolved URL + headers + body) until elapses. + /// + public bool CacheEnabled { get; set; } + + /// + /// Time-to-live for cached responses. null when is true means + /// the response is cached with no expiration (until the process restarts). + /// + public TimeSpan? CacheDuration { get; set; } } \ No newline at end of file diff --git a/NpgsqlRest/NpgsqlRestBuilder.cs b/NpgsqlRest/NpgsqlRestBuilder.cs index a7b87d30..42b5fb15 100644 --- a/NpgsqlRest/NpgsqlRestBuilder.cs +++ b/NpgsqlRest/NpgsqlRestBuilder.cs @@ -58,6 +58,15 @@ public static IApplicationBuilder UseNpgsqlRest(this WebApplication builder, Npg if (Options.HttpClientOptions.Enabled is true) { new HttpClientTypes(builder, defaultStrategy); + + if (Options.HttpClientOptions.CacheEnabled) + { + HttpClientType.HttpResponseCache.Start(Options); + if (builder is WebApplication httpCacheApp) + { + httpCacheApp.Lifetime.ApplicationStopping.Register(HttpClientType.HttpResponseCache.Shutdown); + } + } } var ( diff --git a/NpgsqlRest/Options/HttpClientOptions.cs b/NpgsqlRest/Options/HttpClientOptions.cs index 6cbfd0fe..a72ebd18 100644 --- a/NpgsqlRest/Options/HttpClientOptions.cs +++ b/NpgsqlRest/Options/HttpClientOptions.cs @@ -44,4 +44,24 @@ public class HttpClientOptions /// Example: "http://localhost:5000" /// public string? SelfBaseUrl { get; set; } + + /// + /// Global kill switch for HTTP type response caching. When false, the @cache directive on + /// individual types is ignored and every request fires a fresh outbound call. Default is true, + /// so caching is opt-in per type via @cache and globally disable-able here. + /// + public bool CacheEnabled { get; set; } = true; + + /// + /// Maximum number of distinct cached HTTP responses held in memory. Once the cache is full, new + /// responses are not cached (existing entries are still served and expire normally). Bounds memory + /// for types whose URL/body/headers contain per-request placeholders. Default is 10000. + /// + public int MaxCacheEntries { get; set; } = 10_000; + + /// + /// Interval in seconds at which expired cached HTTP responses are pruned from memory. + /// Default is 60 seconds. + /// + public int CachePruneIntervalSeconds { get; set; } = 60; } diff --git a/NpgsqlRestClient/Builder.cs b/NpgsqlRestClient/Builder.cs index 8fb0b747..d38119aa 100644 --- a/NpgsqlRestClient/Builder.cs +++ b/NpgsqlRestClient/Builder.cs @@ -2806,7 +2806,10 @@ public HttpClientOptions BuildHttpClientOptions() ResponseHeadersField = _config.GetConfigStr("ResponseHeadersField", cfg) ?? "headers", ResponseContentTypeField = _config.GetConfigStr("ResponseContentTypeField", cfg) ?? "content_type", ResponseSuccessField = _config.GetConfigStr("ResponseSuccessField", cfg) ?? "success", - ResponseErrorMessageField = _config.GetConfigStr("ResponseErrorMessageField", cfg) ?? "error_message" + ResponseErrorMessageField = _config.GetConfigStr("ResponseErrorMessageField", cfg) ?? "error_message", + CacheEnabled = _config.GetConfigBool("CacheEnabled", cfg, true), + MaxCacheEntries = _config.GetConfigInt("MaxCacheEntries", cfg) ?? 10_000, + CachePruneIntervalSeconds = _config.GetConfigInt("CachePruneIntervalSeconds", cfg) ?? 60 }; if (options.Enabled) diff --git a/NpgsqlRestClient/ConfigDefaults.cs b/NpgsqlRestClient/ConfigDefaults.cs index 4c4f30c9..cbdeff67 100644 --- a/NpgsqlRestClient/ConfigDefaults.cs +++ b/NpgsqlRestClient/ConfigDefaults.cs @@ -1044,7 +1044,10 @@ private static JsonObject GetHttpClientOptionsDefaults() ["ResponseHeadersField"] = "headers", ["ResponseContentTypeField"] = "content_type", ["ResponseSuccessField"] = "success", - ["ResponseErrorMessageField"] = "error_message" + ["ResponseErrorMessageField"] = "error_message", + ["CacheEnabled"] = true, + ["MaxCacheEntries"] = 10000, + ["CachePruneIntervalSeconds"] = 60 }; } diff --git a/NpgsqlRestClient/ConfigSchemaGenerator.cs b/NpgsqlRestClient/ConfigSchemaGenerator.cs index d58b0882..114082cb 100644 --- a/NpgsqlRestClient/ConfigSchemaGenerator.cs +++ b/NpgsqlRestClient/ConfigSchemaGenerator.cs @@ -493,6 +493,9 @@ public static partial class ConfigSchemaGenerator ["NpgsqlRest:HttpClientOptions:ResponseContentTypeField"] = "Default name for the response content type field within annotated types.", ["NpgsqlRest:HttpClientOptions:ResponseSuccessField"] = "Default name for the response success field within annotated types.", ["NpgsqlRest:HttpClientOptions:ResponseErrorMessageField"] = "Default name for the response error message field within annotated types.", + ["NpgsqlRest:HttpClientOptions:CacheEnabled"] = "Global kill switch for HTTP type response caching. When false, the '@cache' directive on individual types is ignored and every request fires a fresh outbound call. Caching is opt-in per type via the '@cache ' type-comment directive.", + ["NpgsqlRest:HttpClientOptions:MaxCacheEntries"] = "Maximum number of distinct cached HTTP responses held in memory. Once full, new responses are not cached (existing entries are still served and expire normally).", + ["NpgsqlRest:HttpClientOptions:CachePruneIntervalSeconds"] = "Interval in seconds at which expired cached HTTP responses are pruned from memory.", ["NpgsqlRest:ProxyOptions"] = "Reverse proxy functionality for NpgsqlRest endpoints.\nWhen an endpoint is marked with 'proxy' annotation, incoming requests are forwarded to another URL.", ["NpgsqlRest:ProxyOptions:Enabled"] = "Enable proxy functionality for annotated endpoints.", ["NpgsqlRest:ProxyOptions:Host"] = "Base URL (host) for proxy requests (e.g., \"https://api.example.com\").\nWhen set, proxy endpoints will forward requests to this host + the original path.", diff --git a/NpgsqlRestClient/ConfigTemplate.cs b/NpgsqlRestClient/ConfigTemplate.cs index aaeafd15..df4b51c6 100644 --- a/NpgsqlRestClient/ConfigTemplate.cs +++ b/NpgsqlRestClient/ConfigTemplate.cs @@ -2803,7 +2803,22 @@ public static partial class ConfigSchemaGenerator // // Default name for the response error message field within annotated types. // - "ResponseErrorMessageField": "error_message" + "ResponseErrorMessageField": "error_message", + // + // Global kill switch for HTTP type response caching. When false, the '@cache' directive on + // individual types is ignored and every request fires a fresh outbound call. Caching is opt-in + // per type via the '@cache ' type-comment directive. + // + "CacheEnabled": true, + // + // Maximum number of distinct cached HTTP responses held in memory. Once full, new responses are + // not cached (existing entries are still served and expire normally). + // + "MaxCacheEntries": 10000, + // + // Interval in seconds at which expired cached HTTP responses are pruned from memory. + // + "CachePruneIntervalSeconds": 60 }, // diff --git a/NpgsqlRestClient/appsettings.json b/NpgsqlRestClient/appsettings.json index 14ba99a4..9b19fb5d 100644 --- a/NpgsqlRestClient/appsettings.json +++ b/NpgsqlRestClient/appsettings.json @@ -2794,7 +2794,22 @@ // // Default name for the response error message field within annotated types. // - "ResponseErrorMessageField": "error_message" + "ResponseErrorMessageField": "error_message", + // + // Global kill switch for HTTP type response caching. When false, the '@cache' directive on + // individual types is ignored and every request fires a fresh outbound call. Caching is opt-in + // per type via the '@cache ' type-comment directive. + // + "CacheEnabled": true, + // + // Maximum number of distinct cached HTTP responses held in memory. Once full, new responses are + // not cached (existing entries are still served and expire normally). + // + "MaxCacheEntries": 10000, + // + // Interval in seconds at which expired cached HTTP responses are pruned from memory. + // + "CachePruneIntervalSeconds": 60 }, // diff --git a/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeCacheTests.cs b/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeCacheTests.cs new file mode 100644 index 00000000..d204e052 --- /dev/null +++ b/NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeCacheTests.cs @@ -0,0 +1,289 @@ +using WireMock.Server; +using WireMock.RequestBuilders; +using WireMock.ResponseBuilders; +using WireMock; +using WireMock.Types; +using WireMock.Util; + +namespace NpgsqlRestTests; + +public static partial class Database +{ + public static void HttpClientTypeCacheTests() + { + script.Append($@" + -- Cache test 1: basic GET with @cache, body response + create type cache_basic_api as ( + body text + ); + comment on type cache_basic_api is '@cache 60s +GET http://localhost:{WireMockFixture.Port}/api/cache-basic'; + + create function get_cache_basic( + req cache_basic_api + ) + returns text + language plpgsql + as + $$ + begin + return (req).body; + end; + $$; + + -- Cache test 2: multi-field (6) type with @cache - dedup + cache combined + create type cache_sixfield_api as ( + body text, + status_code int, + content_type text, + headers json, + success boolean, + error_message text + ); + comment on type cache_sixfield_api is '@cache 60s +GET http://localhost:{WireMockFixture.Port}/api/cache-sixfield'; + + create function get_cache_sixfield( + req cache_sixfield_api + ) + returns text + language plpgsql + as + $$ + begin + return (req).body; + end; + $$; + + -- Cache test 3: error responses must NOT be cached + create type cache_error_api as ( + body text, + status_code int, + success boolean + ); + comment on type cache_error_api is '@cache 60s +GET http://localhost:{WireMockFixture.Port}/api/cache-error'; + + create function get_cache_error( + req cache_error_api + ) + returns json + language plpgsql + as + $$ + begin + return json_build_object( + 'body', (req).body, + 'status_code', (req).status_code, + 'success', (req).success + ); + end; + $$; + + -- Cache test 4: @cache on POST is ignored (only GET is cacheable) + create type cache_post_api as ( + body text, + status_code int + ); + comment on type cache_post_api is '@cache 60s +POST http://localhost:{WireMockFixture.Port}/api/cache-post +Content-Type: application/json + +{{""ping"": true}}'; + + create function get_cache_post( + req cache_post_api + ) + returns text + language plpgsql + as + $$ + begin + return (req).body; + end; + $$; + + -- Cache test 5: short TTL for expiry verification + create type cache_ttl_api as ( + body text + ); + comment on type cache_ttl_api is '@cache 1s +GET http://localhost:{WireMockFixture.Port}/api/cache-ttl'; + + create function get_cache_ttl( + req cache_ttl_api + ) + returns text + language plpgsql + as + $$ + begin + return (req).body; + end; + $$; +"); + } +} + +[Collection("TestFixture")] +public class HttpClientTypeCacheTests : IClassFixture +{ + private readonly TestFixture _test; + private readonly WireMockServer _server; + + public HttpClientTypeCacheTests(TestFixture test, WireMockFixture wireMock) + { + _test = test; + _server = wireMock.Server; + _server.Reset(); + } + + // A cached GET fires the outbound call once; the second request is served from the cache. The + // callback embeds its invocation count in the body, so a cache hit returns the SAME body the + // first call produced - proving the second response is the cached one, not a fresh fetch. + [Fact] + public async Task Test_cached_get_fires_outbound_call_once() + { + int calls = 0; + _server + .Given(Request.Create().WithPath("/api/cache-basic").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + int n = Interlocked.Increment(ref calls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = $"cache-basic-call-{n}", DetectedBodyType = BodyType.String } + }; + })); + + using var first = await _test.Client.GetAsync("/api/get-cache-basic/"); + var firstContent = await first.Content.ReadAsStringAsync(); + using var second = await _test.Client.GetAsync("/api/get-cache-basic/"); + var secondContent = await second.Content.ReadAsStringAsync(); + + first.StatusCode.Should().Be(HttpStatusCode.OK); + second.StatusCode.Should().Be(HttpStatusCode.OK); + firstContent.Should().Be("cache-basic-call-1"); + secondContent.Should().Be("cache-basic-call-1"); // served from cache, not "call-2" + calls.Should().Be(1); + } + + // A 6-field composite type combines the dedup fix (one call per request, not 6) with caching + // (one call across requests). Two requests against a 6-field cached type → exactly one call. + [Fact] + public async Task Test_cached_multi_field_type_fires_one_call_across_requests() + { + int calls = 0; + _server + .Given(Request.Create().WithPath("/api/cache-sixfield").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + Interlocked.Increment(ref calls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "sixfield-body", DetectedBodyType = BodyType.String } + }; + })); + + using var first = await _test.Client.GetAsync("/api/get-cache-sixfield/"); + var firstContent = await first.Content.ReadAsStringAsync(); + using var second = await _test.Client.GetAsync("/api/get-cache-sixfield/"); + + first.StatusCode.Should().Be(HttpStatusCode.OK); + second.StatusCode.Should().Be(HttpStatusCode.OK); + firstContent.Should().Be("sixfield-body"); + calls.Should().Be(1); + } + + // A failed (non-2xx) response must not be cached, so a transient upstream error is not pinned for + // the whole TTL. First call returns 500, second returns 200 - the second must reach upstream and + // observe the 200, and both calls must hit the server. + [Fact] + public async Task Test_error_response_is_not_cached() + { + int calls = 0; + _server + .Given(Request.Create().WithPath("/api/cache-error").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + int n = Interlocked.Increment(ref calls); + return n == 1 + ? new ResponseMessage + { + StatusCode = 500, + BodyData = new BodyData { BodyAsString = "boom", DetectedBodyType = BodyType.String } + } + : new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "recovered", DetectedBodyType = BodyType.String } + }; + })); + + using var first = await _test.Client.GetAsync("/api/get-cache-error/"); + var firstContent = await first.Content.ReadAsStringAsync(); + using var second = await _test.Client.GetAsync("/api/get-cache-error/"); + var secondContent = await second.Content.ReadAsStringAsync(); + + firstContent.Should().Contain("\"status_code\" : 500"); + firstContent.Should().Contain("\"success\" : false"); + secondContent.Should().Contain("\"status_code\" : 200"); + secondContent.Should().Contain("\"success\" : true"); + calls.Should().Be(2); // the 500 was not cached + } + + // @cache on a non-GET method is ignored (warned at startup), so POST fires every request. + [Fact] + public async Task Test_cache_directive_ignored_for_post() + { + int calls = 0; + _server + .Given(Request.Create().WithPath("/api/cache-post").UsingPost()) + .RespondWith(Response.Create().WithCallback(req => + { + Interlocked.Increment(ref calls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "posted", DetectedBodyType = BodyType.String } + }; + })); + + using var first = await _test.Client.GetAsync("/api/get-cache-post/"); + using var second = await _test.Client.GetAsync("/api/get-cache-post/"); + + first.StatusCode.Should().Be(HttpStatusCode.OK); + second.StatusCode.Should().Be(HttpStatusCode.OK); + calls.Should().Be(2); // POST is never cached + } + + // After the TTL elapses, the entry expires and the next request re-fetches. + [Fact] + public async Task Test_cached_response_expires_after_ttl() + { + int calls = 0; + _server + .Given(Request.Create().WithPath("/api/cache-ttl").UsingGet()) + .RespondWith(Response.Create().WithCallback(req => + { + Interlocked.Increment(ref calls); + return new ResponseMessage + { + StatusCode = 200, + BodyData = new BodyData { BodyAsString = "ttl-body", DetectedBodyType = BodyType.String } + }; + })); + + using var first = await _test.Client.GetAsync("/api/get-cache-ttl/"); // miss -> 1 call + using var second = await _test.Client.GetAsync("/api/get-cache-ttl/"); // hit -> still 1 + calls.Should().Be(1); + + await Task.Delay(TimeSpan.FromSeconds(2)); // TTL is 1s; generous margin + + using var third = await _test.Client.GetAsync("/api/get-cache-ttl/"); // expired -> 2 calls + third.StatusCode.Should().Be(HttpStatusCode.OK); + calls.Should().Be(2); + } +} diff --git a/NpgsqlRestTests/HttpClientTypeTests/ParseHttpTypeDefinitionTests.cs b/NpgsqlRestTests/HttpClientTypeTests/ParseHttpTypeDefinitionTests.cs index 9a05221b..5cb76b3d 100644 --- a/NpgsqlRestTests/HttpClientTypeTests/ParseHttpTypeDefinitionTests.cs +++ b/NpgsqlRestTests/HttpClientTypeTests/ParseHttpTypeDefinitionTests.cs @@ -1016,4 +1016,148 @@ public void NeedsParsing_handles_nested_json_with_placeholder() result.Should().NotBeNull(); result!.NeedsParsing.Should().BeTrue(); } + + // No @cache directive: caching is off and duration is null. + [Fact] + public void Cache_disabled_by_default() + { + var result = _parser.ParseHttpTypeDefinition("GET https://api.example.com/data"); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeFalse(); + result.CacheDuration.Should().BeNull(); + } + + [Theory] + [InlineData("cache 60s")] + [InlineData("cache=60s")] + [InlineData("cache: 60s")] + [InlineData("@cache 60s")] + public void Parses_cache_directive_with_interval(string directive) + { + var input = $"{directive}\nGET https://api.example.com/data"; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeTrue(); + result.CacheDuration.Should().Be(TimeSpan.FromSeconds(60)); + } + + [Fact] + public void Parses_cache_directive_with_timespan_format() + { + var input = """ + @cache 00:05:00 + GET https://api.example.com/data + """; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeTrue(); + result.CacheDuration.Should().Be(TimeSpan.FromMinutes(5)); + } + + // Bare @cache (no interval): enabled, no expiration. + [Fact] + public void Parses_bare_cache_directive_as_enabled_no_expiry() + { + var input = """ + @cache + GET https://api.example.com/data + """; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeTrue(); + result.CacheDuration.Should().BeNull(); + } + + // @cache on a non-GET method is ignored (only GET is cacheable). + [Theory] + [InlineData("POST")] + [InlineData("PUT")] + [InlineData("PATCH")] + [InlineData("DELETE")] + public void Cache_directive_ignored_for_non_get(string method) + { + var input = $"@cache 60s\n{method} https://api.example.com/data"; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeFalse(); + result.CacheDuration.Should().BeNull(); + } + + // Directives may appear AFTER the request line and headers, not only before it. The docs show + // them in this position; the parser recognizes them in the header section too. + [Fact] + public void Parses_timeout_directive_after_headers() + { + var input = """ + GET https://api.example.com/data + Accept: application/json + @timeout 30s + """; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.Timeout.Should().Be(TimeSpan.FromSeconds(30)); + result.Headers.Should().ContainKey("Accept"); + } + + [Fact] + public void Parses_retry_directive_after_headers() + { + var input = """ + GET https://api.example.com/data + Accept: application/json + @retry_delay 1s, 2s on 429, 503 + """; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.RetryDelays.Should().Equal(TimeSpan.FromSeconds(1), TimeSpan.FromSeconds(2)); + result.RetryOnStatusCodes.Should().BeEquivalentTo(new[] { 429, 503 }); + result.Headers.Should().ContainKey("Accept"); + } + + [Fact] + public void Parses_cache_directive_after_headers() + { + var input = """ + GET https://api.example.com/data + Accept: application/json + @cache 5m + """; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeTrue(); + result.CacheDuration.Should().Be(TimeSpan.FromMinutes(5)); + result.Headers.Should().ContainKey("Accept"); + } + + // A real header whose name merely starts with a directive keyword but isn't one (e.g. + // "Cache-Control") is still treated as a header, not a directive. + [Fact] + public void Cache_control_header_is_not_treated_as_cache_directive() + { + var input = """ + GET https://api.example.com/data + Cache-Control: no-cache + """; + + var result = _parser.ParseHttpTypeDefinition(input); + + result.Should().NotBeNull(); + result!.CacheEnabled.Should().BeFalse(); + result.Headers.Should().ContainKey("Cache-Control"); + } } diff --git a/changelog/v3.17.1.md b/changelog/v3.17.1.md deleted file mode 100644 index 3d659752..00000000 --- a/changelog/v3.17.1.md +++ /dev/null @@ -1,45 +0,0 @@ -# Changelog v3.17.1 - -## Version [3.17.1](https://github.com/NpgsqlRest/NpgsqlRest/tree/3.17.1) - -[Full Changelog](https://github.com/NpgsqlRest/NpgsqlRest/compare/3.17.0...3.17.1) - -Patch release that fixes **duplicate outbound HTTP calls for HTTP Custom Types on database-function endpoints**. - -## The bug - -An endpoint backed by a **database function/procedure** whose parameter is an HTTP Custom Type fired **one outbound HTTP call per field of the type** on every inbound request. A 4-field type made 4 identical calls; a 6-field type made 6 — multiplying latency and load on the target service. SQL-file endpoints were not affected. - -## Cause - -A composite function parameter is expanded into one parameter **per field**, and each of those expanded parameters carries the same `TypeDescriptor.CustomType` (the HTTP type name). The type name was therefore collected once per field, so the per-request list of HTTP types to invoke contained the same name N times. The firing loop in `HttpClientTypeHandler.InvokeAllAsync` then called `InvokeAsync` once per entry, with no dedup — while the fill loop immediately below resolves handlers by **distinct** type name out of a dictionary. The design already assumes exactly one call per distinct type; the firing loop just failed to match, so the extra calls were made and their handlers discarded. - -SQL-file endpoints keep the HTTP type as a single composite parameter (`CustomTypeName` null), so the name was collected once → exactly one call, which is why they were unaffected. - -## What changed - -A one-line guard in `HttpClientTypeHandler.InvokeAllAsync` makes the firing loop request each distinct HTTP type once: - -```csharp -foreach (var typeName in typeNames) -{ - if (handlers.ContainsKey(typeName)) continue; // fire once per distinct type - if (HttpClientTypes.Definitions.TryGetValue(typeName, out var definition)) - { - var handler = new HttpClientTypeHandler(definition, replacements); - handlers[typeName] = handler; - tasks.Add((typeName, handler, handler.InvokeAsync(cancellationToken))); - } -} -``` - -This is the single choke point for firing outbound calls and reuses the same dictionary the fill loop already keys on, so it is robust regardless of how the parameter list is built. The established contract is preserved: one call per **distinct** HTTP type, shared from one response — two parameters referencing the same type still share a single call, and two different types remain two separate calls. - -## Tests - -Two regression tests added (`NpgsqlRestTests/HttpClientTypeTests/HttpClientTypeTests.cs`) that count **actual** outbound calls via a WireMock response callback (the prior suite asserted response content but never call counts): - -- A 6-field HTTP type fires **exactly one** outbound call (was 6 before the fix). -- Two distinct HTTP types in one function each fire **exactly one** call. - -Verified against the pre-fix code: the multi-field test failed with `Expected calls to be 1, but found 6`, reproducing the reported behavior. All 46 HTTP-client-type tests pass with the fix. diff --git a/changelog/v3.18.0.md b/changelog/v3.18.0.md new file mode 100644 index 00000000..33042a0a --- /dev/null +++ b/changelog/v3.18.0.md @@ -0,0 +1,53 @@ +# Changelog v3.18.0 + +## Version [3.18.0](https://github.com/NpgsqlRest/NpgsqlRest/tree/3.18.0) + +[Full Changelog](https://github.com/NpgsqlRest/NpgsqlRest/compare/3.17.0...3.18.0) + +The headline of this release is **HTTP Custom Type response caching** — outbound HTTP calls made by HTTP Custom Types can now be cached and reused, eliminating repeated calls to the same upstream within a configurable time window. The release also fixes a duplicate-outbound-call bug for HTTP types on database-function endpoints. + +## New Features + +### HTTP Custom Type response caching — `@cache` directive + +An HTTP Custom Type can now opt into response caching with a `@cache` directive in its type comment, alongside the existing `@timeout` and `@retry_delay` directives. Directives appear **before** the request line: + +```sql +comment on type books_api is '@cache 5m +GET https://books.toscrape.com/'; +``` + +A cached type fires **one outbound call** for a given request shape; subsequent matching requests are served from the in-memory cache until the TTL elapses. For a type with no per-request placeholders (a constant URL/headers/body), that means a single shared upstream call per TTL window across the whole application — instead of one call per inbound request. + +**Behavior and safety rules:** + +- **Opt-in, GET-only.** Caching is enabled per type by `@cache`. A `@cache` directive on any non-GET method is ignored with a startup warning — caching a mutating call is almost always a mistake. +- **TTL.** `@cache ` accepts the same formats as `@timeout` (`5m`, `30s`, `1h`, `00:05:00`, or a bare number of seconds). A bare `@cache` (no interval) caches with no expiration (until the process restarts) and warns. +- **Success-only.** Only successful (2xx) responses are cached, so a transient upstream failure is never pinned for the whole TTL — the next request re-fetches. +- **Stampede protection.** A burst of concurrent requests for the same cache key coalesces into a **single** outbound call; the rest await the in-flight result (same `Lazy` coalescing model as the routine cache). +- **Cache key** = HTTP method + resolved URL + resolved content-type + resolved headers + resolved body. Placeholders are resolved first, so per-request values vary the key naturally. + +**Configuration** (`HttpClientOptions`): + +- `CacheEnabled` (default `true`) — global kill switch. When `false`, `@cache` directives are ignored and every request fires a fresh call. +- `MaxCacheEntries` (default `10000`) — bounds memory; once full, new responses are not cached (existing entries still serve and expire normally). +- `CachePruneIntervalSeconds` (default `60`) — how often expired entries are pruned. + +## Fixes + +### HTTP Custom Type request fired once per composite field on database-function endpoints + +An endpoint backed by a **database function/procedure** whose parameter is an HTTP Custom Type fired **one outbound HTTP call per field of the type** on every inbound request (a 4-field type → 4 identical calls; a 6-field type → 6), multiplying latency and load on the target. SQL-file endpoints were not affected. + +**Cause.** A composite function parameter is expanded into one parameter per field, each carrying the same `TypeDescriptor.CustomType` (the HTTP type name). The per-request list of HTTP types therefore held the same name N times, and the firing loop in `HttpClientTypeHandler.InvokeAllAsync` called `InvokeAsync` once per entry — while the fill loop immediately below resolves handlers by **distinct** type name. The design already assumes one call per distinct type; the firing loop just failed to match. + +**Fix.** A guard in the firing loop requests each distinct HTTP type once, reusing the dictionary the fill loop already keys on. The established contract is preserved: one call per **distinct** HTTP type, shared from one response — two parameters referencing the same type still share a single call, and two different types remain two separate calls. + +### HTTP type directives after the headers were silently ignored + +The `@timeout`, `@retry_delay`, and `@cache` directives are now parsed both **before** the request line and **after** the headers. Previously only the leading position (before the request line) was recognized, so a directive placed after the headers — as the documentation and examples showed — was silently dropped (e.g. a `@timeout` that never applied). Both placements are now equivalent. Real HTTP headers are unaffected: a header whose name merely starts with a directive keyword (e.g. `Cache-Control`) is still treated as a header. + +## Tests + +- Regression tests count **actual** outbound calls via WireMock response callbacks (the prior suite asserted content but never call counts): a 6-field type fires exactly one call (was 6), and two distinct types fire one call each. +- Caching tests cover: cache hit reduces to one call, 6-field dedup + caching combined, error responses not cached, `@cache` ignored on POST, and TTL expiry. Parse-level tests cover the `@cache` directive forms and GET-only enforcement. diff --git a/npm/package.json b/npm/package.json index 859024b3..621ecdbe 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "npgsqlrest", - "version": "3.17.1", + "version": "3.18.0", "description": "Automatic REST API for PostgreSQL Databases Client Build", "scripts": { "postinstall": "node postinstall.js", diff --git a/version.txt b/version.txt index 0caba260..ae561550 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -3.17.1 \ No newline at end of file +3.18.0 \ No newline at end of file From e036f3d322e900e7a36c5d65ae96f4f48b32602e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vedran=20Bilopavlovi=C4=87?= Date: Tue, 23 Jun 2026 12:36:52 +0200 Subject: [PATCH 5/5] upgrade references --- NpgsqlRestClient/NpgsqlRestClient.csproj | 4 ++-- NpgsqlRestTests/NpgsqlRestTests.csproj | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/NpgsqlRestClient/NpgsqlRestClient.csproj b/NpgsqlRestClient/NpgsqlRestClient.csproj index 03e0a02b..6beaecf6 100644 --- a/NpgsqlRestClient/NpgsqlRestClient.csproj +++ b/NpgsqlRestClient/NpgsqlRestClient.csproj @@ -15,14 +15,14 @@ - + - + diff --git a/NpgsqlRestTests/NpgsqlRestTests.csproj b/NpgsqlRestTests/NpgsqlRestTests.csproj index fcd52b6b..a2a10468 100644 --- a/NpgsqlRestTests/NpgsqlRestTests.csproj +++ b/NpgsqlRestTests/NpgsqlRestTests.csproj @@ -16,9 +16,9 @@ - + - + runtime; build; native; contentfiles; analyzers; buildtransitive