fix(transport): drop the external-definition lookup cache after a committed bundle import (ScadaBridge#83)
The bundle importer writes external systems, methods and database connections through the raw DbContext, bypassing the ManagementActor handlers that invalidate ExternalDefinitionLookupCache (#65), so the importing node served pre-import definitions for up to DefinitionCacheTtl. New Commons seam IExternalDefinitionCacheInvalidator, implemented by the cache and registered as the same singleton; BundleImporter takes it optionally and invalidates once after every committed import (never on rollback; never throws). Peer-central-node invalidation (the other half of #83) is NOT done here: the issue leaves open whether the 10 s TTL bound is acceptable for the peer. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
+15
@@ -0,0 +1,15 @@
|
||||
namespace ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
|
||||
/// <summary>
|
||||
/// Drops the node-local cache of external-system / method / database-connection definitions
|
||||
/// (<c>ExternalSystemGateway.ExternalDefinitionLookupCache</c>) after a committed write to
|
||||
/// those definitions, so the editing node serves the new definition immediately instead of
|
||||
/// after the cache TTL. Lives in Commons so a writer that cannot reference the gateway (the
|
||||
/// Transport bundle importer, Gitea #83) can still signal it. Scope is THIS node only — a
|
||||
/// peer central node converges within the TTL.
|
||||
/// </summary>
|
||||
public interface IExternalDefinitionCacheInvalidator
|
||||
{
|
||||
/// <summary>Drops every cached definition on this node.</summary>
|
||||
void InvalidateAll();
|
||||
}
|
||||
@@ -1,6 +1,7 @@
|
||||
using System.Collections.Concurrent;
|
||||
using Microsoft.Extensions.Options;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.ExternalSystemGateway;
|
||||
|
||||
@@ -19,7 +20,7 @@ namespace ZB.MOM.WW.ScadaBridge.ExternalSystemGateway;
|
||||
/// deleted one is visible within one TTL.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public sealed class ExternalDefinitionLookupCache
|
||||
public sealed class ExternalDefinitionLookupCache : IExternalDefinitionCacheInvalidator
|
||||
{
|
||||
private sealed record Entry(object? Value, DateTimeOffset ExpiresAtUtc);
|
||||
|
||||
@@ -72,6 +73,10 @@ public sealed class ExternalDefinitionLookupCache
|
||||
=> GetAsync("c:" + name, fetch);
|
||||
|
||||
/// <summary>Drops every cached definition.</summary>
|
||||
/// <remarks>
|
||||
/// Called post-commit by the ManagementActor edit handlers (Gitea #65) and by the Transport
|
||||
/// bundle importer through <see cref="IExternalDefinitionCacheInvalidator"/> (Gitea #83).
|
||||
/// </remarks>
|
||||
public void InvalidateAll() => _entries.Clear();
|
||||
|
||||
private async Task<T?> GetAsync<T>(string key, Func<Task<T?>> fetch) where T : class
|
||||
|
||||
@@ -44,6 +44,10 @@ public static class ServiceCollectionExtensions
|
||||
sp.GetRequiredService<IOptionsMonitor<ExternalSystemGatewayOptions>>()));
|
||||
|
||||
services.TryAddSingleton<ExternalDefinitionLookupCache>();
|
||||
// Gitea #83: the same singleton, exposed through the Commons seam so the Transport bundle
|
||||
// importer can drop it after a committed import without referencing this assembly.
|
||||
services.TryAddSingleton<IExternalDefinitionCacheInvalidator>(
|
||||
sp => sp.GetRequiredService<ExternalDefinitionLookupCache>());
|
||||
services.AddScoped<ExternalSystemClient>();
|
||||
services.AddScoped<IExternalSystemClient>(sp => sp.GetRequiredService<ExternalSystemClient>());
|
||||
services.AddScoped<DatabaseGateway>();
|
||||
|
||||
@@ -95,6 +95,9 @@ public sealed class BundleImporter : IBundleImporter
|
||||
// derives watermark bumps from the change tracker. Null in hosts that don't
|
||||
// register it; the caches then simply never get this invalidation signal.
|
||||
private readonly ITemplateGraphWatermark? _watermark;
|
||||
// Optional (Gitea #83): drops this node's external-definition lookup cache after a
|
||||
// committed import. Null on hosts without the External System Gateway.
|
||||
private readonly IExternalDefinitionCacheInvalidator? _definitionCacheInvalidator;
|
||||
private readonly ILogger<BundleImporter>? _logger;
|
||||
|
||||
/// <summary>
|
||||
@@ -121,6 +124,7 @@ public sealed class BundleImporter : IBundleImporter
|
||||
/// <param name="scriptArtifactChangeBus">Optional: node-local bus notified AFTER a successful commit that script-bearing artifacts (ApiMethod / SharedScript / Template) were overwritten or renamed, so compiled-handler caches can invalidate. Null on hosts that don't register it (staleness/invalidation then relies on the consumer's own self-heal).</param>
|
||||
/// <param name="logger">Optional logger.</param>
|
||||
/// <param name="watermark">Optional: template-graph version watermark, bumped once per apply attempt so the flatten-session caches and the process-wide staleness memo cannot serve pre-import (or rolled-back) readings. Null on hosts that don't register it.</param>
|
||||
/// <param name="definitionCacheInvalidator">Optional (Gitea #83): drops this node's external-system / database-connection definition cache after a committed import, so imported definitions take effect immediately rather than after the cache TTL. Null on hosts without the External System Gateway.</param>
|
||||
public BundleImporter(
|
||||
BundleSerializer bundleSerializer,
|
||||
ManifestValidator manifestValidator,
|
||||
@@ -142,7 +146,8 @@ public sealed class BundleImporter : IBundleImporter
|
||||
IStaleInstanceProbe? staleInstanceProbe = null,
|
||||
IScriptArtifactChangeBus? scriptArtifactChangeBus = null,
|
||||
ILogger<BundleImporter>? logger = null,
|
||||
ITemplateGraphWatermark? watermark = null)
|
||||
ITemplateGraphWatermark? watermark = null,
|
||||
IExternalDefinitionCacheInvalidator? definitionCacheInvalidator = null)
|
||||
{
|
||||
_bundleSerializer = bundleSerializer ?? throw new ArgumentNullException(nameof(bundleSerializer));
|
||||
_manifestValidator = manifestValidator ?? throw new ArgumentNullException(nameof(manifestValidator));
|
||||
@@ -164,6 +169,7 @@ public sealed class BundleImporter : IBundleImporter
|
||||
_staleInstanceProbe = staleInstanceProbe;
|
||||
_scriptArtifactChangeBus = scriptArtifactChangeBus;
|
||||
_watermark = watermark;
|
||||
_definitionCacheInvalidator = definitionCacheInvalidator;
|
||||
_logger = logger;
|
||||
}
|
||||
|
||||
@@ -1697,6 +1703,7 @@ public sealed class BundleImporter : IBundleImporter
|
||||
// Post-commit side effects — run ONCE, only after a durable commit, so a
|
||||
// retried attempt neither publishes twice nor releases the session early.
|
||||
PublishScriptArtifactChanges(resolutions);
|
||||
InvalidateExternalDefinitionCache();
|
||||
|
||||
// T-007: zero out the decrypted plaintext BEFORE remove so any
|
||||
// caller-held reference (e.g. the Razor page that built the
|
||||
@@ -2442,6 +2449,32 @@ public sealed class BundleImporter : IBundleImporter
|
||||
/// (nothing was written). Fully guarded: never throws (it runs after the transaction
|
||||
/// has committed).
|
||||
/// </summary>
|
||||
/// <summary>
|
||||
/// Gitea #83: the importer writes external systems, their methods and database connections
|
||||
/// through the raw <c>ScadaBridgeDbContext</c>, bypassing the ManagementActor handlers that
|
||||
/// drop <c>ExternalDefinitionLookupCache</c> (Gitea #65), so without this the importing node
|
||||
/// served the pre-import definition for up to <c>DefinitionCacheTtl</c>. Invalidates on every
|
||||
/// committed import rather than only when an external entity was resolved: dependencies can
|
||||
/// pull definitions in implicitly, imports are rare, and a cleared cache only costs one
|
||||
/// re-query per name. Never throws (the transaction has already committed).
|
||||
/// </summary>
|
||||
private void InvalidateExternalDefinitionCache()
|
||||
{
|
||||
if (_definitionCacheInvalidator is null)
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
_definitionCacheInvalidator.InvalidateAll();
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger?.LogError(ex, "Invalidating the external-definition cache after a committed bundle import failed; ignored (the cache TTL still bounds staleness).");
|
||||
}
|
||||
}
|
||||
|
||||
private void PublishScriptArtifactChanges(IReadOnlyList<ImportResolution> resolutions)
|
||||
{
|
||||
if (_scriptArtifactChangeBus is null)
|
||||
|
||||
@@ -3,6 +3,7 @@ using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Http;
|
||||
using NSubstitute;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests;
|
||||
|
||||
@@ -12,6 +13,26 @@ namespace ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests;
|
||||
/// </summary>
|
||||
public class ServiceWiringTests
|
||||
{
|
||||
/// <summary>
|
||||
/// Gitea #83: the Commons invalidation seam resolves to the SAME lookup-cache singleton the
|
||||
/// gateway reads through, so a bundle import that invalidates it actually clears the cache.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void DefinitionCacheInvalidator_IsTheLookupCacheSingleton()
|
||||
{
|
||||
var services = new ServiceCollection();
|
||||
services.AddLogging();
|
||||
services.AddSingleton<IConfiguration>(new ConfigurationBuilder().Build());
|
||||
services.AddSingleton(Substitute.For<IExternalSystemRepository>());
|
||||
services.AddExternalSystemGateway();
|
||||
|
||||
using var provider = services.BuildServiceProvider();
|
||||
|
||||
Assert.Same(
|
||||
provider.GetRequiredService<ExternalDefinitionLookupCache>(),
|
||||
provider.GetRequiredService<IExternalDefinitionCacheInvalidator>());
|
||||
}
|
||||
|
||||
/// <summary>Verifies that the per-system connection cap is applied to the primary handler of the named HTTP client.</summary>
|
||||
[Fact]
|
||||
public void MaxConcurrentConnectionsPerSystem_IsAppliedToTheNamedHttpClientPrimaryHandler()
|
||||
|
||||
+52
@@ -47,6 +47,7 @@ public sealed class BundleImporterApplyTests : IDisposable
|
||||
{
|
||||
private readonly ServiceProvider _provider;
|
||||
private readonly RecordingScriptArtifactChangeBus _artifactBus = new();
|
||||
private readonly CountingDefinitionCacheInvalidator _definitionCache = new();
|
||||
|
||||
/// <summary>Initializes a new instance of the <c>BundleImporterApplyTests</c> class.</summary>
|
||||
public BundleImporterApplyTests()
|
||||
@@ -100,6 +101,8 @@ public sealed class BundleImporterApplyTests : IDisposable
|
||||
// publish is observable. BundleImporter takes IScriptArtifactChangeBus?
|
||||
// optionally, so this registration flows in via DI activation.
|
||||
services.AddSingleton<IScriptArtifactChangeBus>(_artifactBus);
|
||||
// Gitea #83: observable post-commit drop of the external-definition lookup cache.
|
||||
services.AddSingleton<IExternalDefinitionCacheInvalidator>(_definitionCache);
|
||||
// The stale-instance probe is implemented in DeploymentManager
|
||||
// and the flatten/hash primitives in TemplateEngine — register both so
|
||||
// BundleImporter resolves a real IStaleInstanceProbe and ApplyAsync can
|
||||
@@ -1910,6 +1913,41 @@ public sealed class BundleImporterApplyTests : IDisposable
|
||||
Assert.Contains("DelmiaRecipeDownload", notification.Names); // FAILS today: Adds are filtered out
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gitea #83: a committed import drops the node's external-definition lookup cache once,
|
||||
/// because the importer writes external systems / database connections without going
|
||||
/// through the ManagementActor handlers that otherwise invalidate it.
|
||||
/// </summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task ApplyAsync_committed_import_invalidates_the_external_definition_cache()
|
||||
{
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
ctx.ExternalSystemDefinitions.Add(new ExternalSystemDefinition("Mes", "https://mes.example/api", "None"));
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
var sessionId = await ExportAndLoadAsync();
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var ctx = scope.ServiceProvider.GetRequiredService<ScadaBridgeDbContext>();
|
||||
ctx.ExternalSystemDefinitions.RemoveRange(ctx.ExternalSystemDefinitions);
|
||||
await ctx.SaveChangesAsync();
|
||||
}
|
||||
Assert.Equal(0, _definitionCache.Calls);
|
||||
|
||||
await using (var scope = _provider.CreateAsyncScope())
|
||||
{
|
||||
var importer = scope.ServiceProvider.GetRequiredService<IBundleImporter>();
|
||||
await importer.ApplyAsync(sessionId,
|
||||
new List<ImportResolution> { new("ExternalSystem", "Mes", ResolutionAction.Add, null) },
|
||||
user: "bob");
|
||||
}
|
||||
|
||||
Assert.Equal(1, _definitionCache.Calls);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a failed apply publishes no notifications.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
@@ -1958,6 +1996,20 @@ public sealed class BundleImporterApplyTests : IDisposable
|
||||
}
|
||||
|
||||
Assert.Empty(_artifactBus.Received);
|
||||
// Gitea #83: nothing committed, so the definition cache is left alone.
|
||||
Assert.Equal(0, _definitionCache.Calls);
|
||||
}
|
||||
|
||||
/// <summary>Counts post-commit external-definition cache invalidations (Gitea #83).</summary>
|
||||
private sealed class CountingDefinitionCacheInvalidator : IExternalDefinitionCacheInvalidator
|
||||
{
|
||||
private int _calls;
|
||||
|
||||
/// <summary>Gets the number of invalidations received.</summary>
|
||||
public int Calls => Volatile.Read(ref _calls);
|
||||
|
||||
/// <inheritdoc />
|
||||
public void InvalidateAll() => Interlocked.Increment(ref _calls);
|
||||
}
|
||||
|
||||
/// <summary>Records every published notification for assertion.</summary>
|
||||
|
||||
Reference in New Issue
Block a user