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

src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Services/IExternalDefinitionCacheInvalidator.cs

@@ -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();
+}

src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalDefinitionLookupCache.cs

@@ -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

src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ServiceCollectionExtensions.cs

@@ -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>();

src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs

@@ -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)

tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ServiceWiringTests.cs

@@ -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()

tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs

@@ -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>