diff --git a/.github/workflows/build-test-publish.yml b/.github/workflows/build-test-publish.yml index 3e5b8e75..e9d337b2 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 diff --git a/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs b/NpgsqlRest/HttpClientType/HttpClientTypeHandler.cs index 41a126c1..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) @@ -284,11 +354,19 @@ 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); 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/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/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/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/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/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 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) 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/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/ diff --git a/npm/package.json b/npm/package.json index 484cd0f8..621ecdbe 100644 --- a/npm/package.json +++ b/npm/package.json @@ -1,6 +1,6 @@ { "name": "npgsqlrest", - "version": "3.17.0", + "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 f85bf6e3..ae561550 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -3.17.0 \ No newline at end of file +3.18.0 \ No newline at end of file