fix(esg): short-TTL definition cache for central lookups (review 2026-10-21 05/P2)

This commit is contained in:
Joseph Doherty
2026-10-22 08:41:54 -04:00
parent ee8be8001e
commit 12f815a7ea
8 changed files with 193 additions and 4 deletions
@@ -120,6 +120,8 @@ Scripts choose between two call modes per invocation, mirroring the dual-mode da
- Database connections use **standard ADO.NET connection pooling** per named connection. No custom pool management.
- Pool behavior (max pool size, connection lifetime, etc.) can be tuned via connection string parameters in the database connection definition if needed.
- **Definition cache (central only).** `ExternalDefinitionLookupCache` holds resolved external-system, method and database-connection definitions for `ExternalSystemGateway:DefinitionCacheTtl` (default 0 = off in the options; the central host sets 10 s because each lookup there is a SQL Server round-trip). Only found definitions are cached, so a new definition is visible immediately; an edit/delete within one TTL. Sites keep it off — `SiteExternalSystemRepository` already serves external systems from the deploy-invalidated `ExternalSystemDefinitionCache` snapshot.
- GET/DELETE parameters are rendered into the query string with scalars via `ToString()` and collections/objects JSON-serialised (never a CLR type name).
- Synchronous failures on `Database.Connection()` (e.g., unreachable server) return an error to the calling script, consistent with external system permanent failure handling.
## Dependencies
@@ -21,6 +21,7 @@ public class DatabaseGateway : IDatabaseGateway, ISandboxDatabaseConnectionSourc
private readonly IExternalSystemRepository _repository;
private readonly StoreAndForwardService? _storeAndForward;
private readonly ILogger<DatabaseGateway> _logger;
private readonly ExternalDefinitionLookupCache? _definitionCache;
/// <summary>
/// Initializes a new instance of <see cref="DatabaseGateway"/>.
@@ -28,11 +29,14 @@ public class DatabaseGateway : IDatabaseGateway, ISandboxDatabaseConnectionSourc
/// <param name="repository">Repository for resolving database connection definitions.</param>
/// <param name="logger">Logger for diagnostics.</param>
/// <param name="storeAndForward">Optional store-and-forward service for cached writes; null disables buffering.</param>
/// <param name="definitionCache">Optional short-TTL definition cache; null (or a zero TTL) resolves every call from the repository.</param>
public DatabaseGateway(
IExternalSystemRepository repository,
ILogger<DatabaseGateway> logger,
StoreAndForwardService? storeAndForward = null)
StoreAndForwardService? storeAndForward = null,
ExternalDefinitionLookupCache? definitionCache = null)
{
_definitionCache = definitionCache;
_repository = repository;
_logger = logger;
_storeAndForward = storeAndForward;
@@ -447,6 +451,12 @@ public class DatabaseGateway : IDatabaseGateway, ISandboxDatabaseConnectionSourc
// fetch-all-then-filter — connection definitions are resolved on every
// cached write / connection request, so the repository performs an indexed
// query rather than loading every connection into memory.
if (_definitionCache is { Enabled: true })
{
return await _definitionCache.GetConnectionAsync(connectionName,
() => _repository.GetDatabaseConnectionByNameAsync(connectionName, cancellationToken));
}
return await _repository.GetDatabaseConnectionByNameAsync(connectionName, cancellationToken);
}
}
@@ -0,0 +1,98 @@
using System.Collections.Concurrent;
using Microsoft.Extensions.Options;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
namespace ZB.MOM.WW.ScadaBridge.ExternalSystemGateway;
/// <summary>
/// Short-TTL, in-memory cache of the external-system / method / database-connection
/// definitions the gateway resolves on every <c>ExternalSystem.Call</c> and
/// <c>Database.*</c> invocation (review 2026-10-21 05/P2).
/// <para>
/// Enabled only when <see cref="ExternalSystemGatewayOptions.DefinitionCacheTtl"/> is
/// positive — the central host turns it on (each call there is otherwise a SQL Server
/// round-trip inside an inbound request). Site nodes leave it off: their
/// <c>SiteExternalSystemRepository</c> already serves external systems from the shared,
/// deploy-invalidated <c>ExternalSystemDefinitionCache</c> snapshot, and layering a TTL on
/// top would only delay the effect of an artifact deploy. Only found definitions are cached
/// (a miss always re-queries), so a newly created system is visible immediately; an edited or
/// deleted one is visible within one TTL.
/// </para>
/// </summary>
public sealed class ExternalDefinitionLookupCache
{
private sealed record Entry(object? Value, DateTimeOffset ExpiresAtUtc);
private const int PruneThreshold = 512;
private readonly ConcurrentDictionary<string, Entry> _entries = new(StringComparer.Ordinal);
private readonly TimeSpan _ttl;
private readonly TimeProvider _timeProvider;
/// <summary>Initializes the cache from the gateway options.</summary>
/// <param name="options">Gateway options; <see cref="ExternalSystemGatewayOptions.DefinitionCacheTtl"/> sets the TTL.</param>
/// <param name="timeProvider">Optional time source; defaults to <see cref="TimeProvider.System"/>.</param>
public ExternalDefinitionLookupCache(IOptions<ExternalSystemGatewayOptions> options, TimeProvider? timeProvider = null)
: this(options.Value.DefinitionCacheTtl, timeProvider)
{
}
/// <summary>Initializes the cache with an explicit TTL.</summary>
/// <param name="ttl">Entry lifetime; zero or negative disables caching.</param>
/// <param name="timeProvider">Optional time source; defaults to <see cref="TimeProvider.System"/>.</param>
public ExternalDefinitionLookupCache(TimeSpan ttl, TimeProvider? timeProvider = null)
{
_ttl = ttl;
_timeProvider = timeProvider ?? TimeProvider.System;
}
/// <summary>Whether caching is active.</summary>
public bool Enabled => _ttl > TimeSpan.Zero;
/// <summary>Resolves an external system by name, via the cache.</summary>
/// <param name="name">The system name.</param>
/// <param name="fetch">Loads the definition on a miss.</param>
/// <returns>The definition, or null when it does not exist.</returns>
public Task<ExternalSystemDefinition?> GetSystemAsync(string name, Func<Task<ExternalSystemDefinition?>> fetch)
=> GetAsync("s:" + name, fetch);
/// <summary>Resolves a method of an external system by name, via the cache.</summary>
/// <param name="systemId">The owning system's id.</param>
/// <param name="name">The method name.</param>
/// <param name="fetch">Loads the method on a miss.</param>
/// <returns>The method, or null when it does not exist.</returns>
public Task<ExternalSystemMethod?> GetMethodAsync(int systemId, string name, Func<Task<ExternalSystemMethod?>> fetch)
=> GetAsync("m:" + systemId + ":" + name, fetch);
/// <summary>Resolves a database connection definition by name, via the cache.</summary>
/// <param name="name">The connection name.</param>
/// <param name="fetch">Loads the definition on a miss.</param>
/// <returns>The definition, or null when it does not exist.</returns>
public Task<DatabaseConnectionDefinition?> GetConnectionAsync(string name, Func<Task<DatabaseConnectionDefinition?>> fetch)
=> GetAsync("c:" + name, fetch);
/// <summary>Drops every cached definition.</summary>
public void InvalidateAll() => _entries.Clear();
private async Task<T?> GetAsync<T>(string key, Func<Task<T?>> fetch) where T : class
{
if (!Enabled)
return await fetch().ConfigureAwait(false);
var now = _timeProvider.GetUtcNow();
if (_entries.TryGetValue(key, out var hit) && hit.ExpiresAtUtc > now && hit.Value is T cached)
return cached;
var value = await fetch().ConfigureAwait(false);
if (value is not null)
{
if (_entries.Count >= PruneThreshold)
{
foreach (var kv in _entries)
if (kv.Value.ExpiresAtUtc <= now) _entries.TryRemove(kv);
}
_entries[key] = new Entry(value, now + _ttl);
}
return value;
}
}
@@ -26,6 +26,7 @@ public class ExternalSystemClient : IExternalSystemClient
private readonly StoreAndForwardService? _storeAndForward;
private readonly ILogger<ExternalSystemClient> _logger;
private readonly ExternalSystemGatewayOptions _options;
private readonly ExternalDefinitionLookupCache? _definitionCache;
/// <summary>
/// Shared options for the per-call AuthConfiguration JSON parse in
@@ -44,13 +45,16 @@ public class ExternalSystemClient : IExternalSystemClient
/// <param name="logger">Logger instance.</param>
/// <param name="storeAndForward">Store-and-forward service for buffering transient failures, or null if not available.</param>
/// <param name="options">Configuration options, or null for defaults.</param>
/// <param name="definitionCache">Optional short-TTL definition cache; null (or a zero TTL) resolves every call from the repository.</param>
public ExternalSystemClient(
IHttpClientFactory httpClientFactory,
IExternalSystemRepository repository,
ILogger<ExternalSystemClient> logger,
StoreAndForwardService? storeAndForward = null,
IOptions<ExternalSystemGatewayOptions>? options = null)
IOptions<ExternalSystemGatewayOptions>? options = null,
ExternalDefinitionLookupCache? definitionCache = null)
{
_definitionCache = definitionCache;
_httpClientFactory = httpClientFactory;
_repository = repository;
_logger = logger;
@@ -815,11 +819,17 @@ public class ExternalSystemClient : IExternalSystemClient
// fetch-all-then-filter — definitions are resolved on every hot-path call
// (a script's ExternalSystem.Call()), so the repository performs an indexed
// query rather than loading every system / every method into memory.
var system = await _repository.GetExternalSystemByNameAsync(systemName, cancellationToken);
var system = _definitionCache is { Enabled: true }
? await _definitionCache.GetSystemAsync(systemName,
() => _repository.GetExternalSystemByNameAsync(systemName, cancellationToken))
: await _repository.GetExternalSystemByNameAsync(systemName, cancellationToken);
if (system == null)
return (null, null);
var method = await _repository.GetMethodByNameAsync(system.Id, methodName, cancellationToken);
var method = _definitionCache is { Enabled: true }
? await _definitionCache.GetMethodAsync(system.Id, methodName,
() => _repository.GetMethodByNameAsync(system.Id, methodName, cancellationToken))
: await _repository.GetMethodByNameAsync(system.Id, methodName, cancellationToken);
return (system, method);
}
@@ -19,4 +19,13 @@ public class ExternalSystemGatewayOptions
/// Default 10 MiB.
/// </summary>
public long MaxResponseBodyBytes { get; set; } = 10 * 1024 * 1024;
/// <summary>
/// How long a resolved external-system / method / database-connection definition is
/// cached in memory before the repository is consulted again. <see cref="TimeSpan.Zero"/>
/// (the default) disables the cache. The central host defaults it on (10 s) because each
/// lookup there is a SQL Server round-trip; site nodes keep it off since their repository
/// is already a deploy-invalidated snapshot (review 2026-10-21 05/P2).
/// </summary>
public TimeSpan DefinitionCacheTtl { get; set; } = TimeSpan.Zero;
}
@@ -29,5 +29,9 @@ public sealed class ExternalSystemGatewayOptionsValidator : OptionsValidatorBase
builder.RequireThat(options.MaxResponseBodyBytes > 0,
$"ScadaBridge:ExternalSystemGateway:MaxResponseBodyBytes must be greater than zero " +
$"(was {options.MaxResponseBodyBytes}); it caps the outbound response body buffered into memory.");
builder.RequireThat(options.DefinitionCacheTtl >= TimeSpan.Zero,
$"ScadaBridge:ExternalSystemGateway:DefinitionCacheTtl must not be negative " +
$"(was {options.DefinitionCacheTtl}); use 00:00:00 to disable the definition cache.");
}
}
@@ -43,6 +43,7 @@ public static class ServiceCollectionExtensions
new GatewayHttpClientConfigurator(
sp.GetRequiredService<IOptionsMonitor<ExternalSystemGatewayOptions>>()));
services.TryAddSingleton<ExternalDefinitionLookupCache>();
services.AddScoped<ExternalSystemClient>();
services.AddScoped<IExternalSystemClient>(sp => sp.GetRequiredService<ExternalSystemClient>());
services.AddScoped<DatabaseGateway>();
@@ -0,0 +1,55 @@
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
namespace ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests;
/// <summary>Tests for <see cref="ExternalDefinitionLookupCache"/> (review 2026-10-21 05/P2).</summary>
public class ExternalDefinitionLookupCacheTests
{
private static ExternalSystemDefinition Sys() => new("S", "https://x", "none") { Id = 1 };
/// <summary>Verifies hits within the TTL skip the repository and expiry refetches.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Hit_WithinTtl_SkipsFetch_AndExpiryRefetches()
{
var time = new FakeTimeProvider(DateTimeOffset.UtcNow);
var cache = new ExternalDefinitionLookupCache(TimeSpan.FromSeconds(5), time);
var fetches = 0;
Task<ExternalSystemDefinition?> Fetch() { fetches++; return Task.FromResult<ExternalSystemDefinition?>(Sys()); }
await cache.GetSystemAsync("S", Fetch);
await cache.GetSystemAsync("S", Fetch);
Assert.Equal(1, fetches);
time.Advance(TimeSpan.FromSeconds(6));
await cache.GetSystemAsync("S", Fetch);
Assert.Equal(2, fetches);
}
/// <summary>Verifies misses are not cached and a zero TTL disables caching.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task NotFound_IsNotCached_AndZeroTtlDisables()
{
var cache = new ExternalDefinitionLookupCache(TimeSpan.FromSeconds(5));
var fetches = 0;
Task<ExternalSystemDefinition?> Miss() { fetches++; return Task.FromResult<ExternalSystemDefinition?>(null); }
await cache.GetSystemAsync("S", Miss);
await cache.GetSystemAsync("S", Miss);
Assert.Equal(2, fetches);
var off = new ExternalDefinitionLookupCache(TimeSpan.Zero);
fetches = 0;
Task<ExternalSystemDefinition?> Hit() { fetches++; return Task.FromResult<ExternalSystemDefinition?>(Sys()); }
await off.GetSystemAsync("S", Hit);
await off.GetSystemAsync("S", Hit);
Assert.Equal(2, fetches);
}
private sealed class FakeTimeProvider(DateTimeOffset start) : TimeProvider
{
private DateTimeOffset _now = start;
public override DateTimeOffset GetUtcNow() => _now;
public void Advance(TimeSpan by) => _now += by;
}
}