diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Services/IExternalDefinitionCacheInvalidator.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Services/IExternalDefinitionCacheInvalidator.cs new file mode 100644 index 00000000..84ff6d69 --- /dev/null +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Services/IExternalDefinitionCacheInvalidator.cs @@ -0,0 +1,15 @@ +namespace ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; + +/// +/// Drops the node-local cache of external-system / method / database-connection definitions +/// (ExternalSystemGateway.ExternalDefinitionLookupCache) 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. +/// +public interface IExternalDefinitionCacheInvalidator +{ + /// Drops every cached definition on this node. + void InvalidateAll(); +} diff --git a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalDefinitionLookupCache.cs b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalDefinitionLookupCache.cs index 8b8467cd..d3ae1746 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ExternalDefinitionLookupCache.cs +++ b/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. /// /// -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); /// Drops every cached definition. + /// + /// Called post-commit by the ManagementActor edit handlers (Gitea #65) and by the Transport + /// bundle importer through (Gitea #83). + /// public void InvalidateAll() => _entries.Clear(); private async Task GetAsync(string key, Func> fetch) where T : class diff --git a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ServiceCollectionExtensions.cs b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ServiceCollectionExtensions.cs index e63f04e7..30bd274e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ServiceCollectionExtensions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway/ServiceCollectionExtensions.cs @@ -44,6 +44,10 @@ public static class ServiceCollectionExtensions sp.GetRequiredService>())); services.TryAddSingleton(); + // 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( + sp => sp.GetRequiredService()); services.AddScoped(); services.AddScoped(sp => sp.GetRequiredService()); services.AddScoped(); diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 5531bd49..3ecf8159 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/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? _logger; /// @@ -121,6 +124,7 @@ public sealed class BundleImporter : IBundleImporter /// 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). /// Optional logger. /// 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. + /// 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. public BundleImporter( BundleSerializer bundleSerializer, ManifestValidator manifestValidator, @@ -142,7 +146,8 @@ public sealed class BundleImporter : IBundleImporter IStaleInstanceProbe? staleInstanceProbe = null, IScriptArtifactChangeBus? scriptArtifactChangeBus = null, ILogger? 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). /// + /// + /// Gitea #83: the importer writes external systems, their methods and database connections + /// through the raw ScadaBridgeDbContext, bypassing the ManagementActor handlers that + /// drop ExternalDefinitionLookupCache (Gitea #65), so without this the importing node + /// served the pre-import definition for up to DefinitionCacheTtl. 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). + /// + 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 resolutions) { if (_scriptArtifactChangeBus is null) diff --git a/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ServiceWiringTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ServiceWiringTests.cs index 76e1acfc..0b4439f7 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ExternalSystemGateway.Tests/ServiceWiringTests.cs +++ b/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; /// public class ServiceWiringTests { + /// + /// 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. + /// + [Fact] + public void DefinitionCacheInvalidator_IsTheLookupCacheSingleton() + { + var services = new ServiceCollection(); + services.AddLogging(); + services.AddSingleton(new ConfigurationBuilder().Build()); + services.AddSingleton(Substitute.For()); + services.AddExternalSystemGateway(); + + using var provider = services.BuildServiceProvider(); + + Assert.Same( + provider.GetRequiredService(), + provider.GetRequiredService()); + } + /// Verifies that the per-system connection cap is applied to the primary handler of the named HTTP client. [Fact] public void MaxConcurrentConnectionsPerSystem_IsAppliedToTheNamedHttpClientPrimaryHandler() diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs index 13ff037d..1880a8e5 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs +++ b/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(); /// Initializes a new instance of the BundleImporterApplyTests class. 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(_artifactBus); + // Gitea #83: observable post-commit drop of the external-definition lookup cache. + services.AddSingleton(_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 } + /// + /// 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. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ApplyAsync_committed_import_invalidates_the_external_definition_cache() + { + await using (var scope = _provider.CreateAsyncScope()) + { + var ctx = scope.ServiceProvider.GetRequiredService(); + 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(); + ctx.ExternalSystemDefinitions.RemoveRange(ctx.ExternalSystemDefinitions); + await ctx.SaveChangesAsync(); + } + Assert.Equal(0, _definitionCache.Calls); + + await using (var scope = _provider.CreateAsyncScope()) + { + var importer = scope.ServiceProvider.GetRequiredService(); + await importer.ApplyAsync(sessionId, + new List { new("ExternalSystem", "Mes", ResolutionAction.Add, null) }, + user: "bob"); + } + + Assert.Equal(1, _definitionCache.Calls); + } + /// Verifies that a failed apply publishes no notifications. /// A task that represents the asynchronous operation. [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); + } + + /// Counts post-commit external-definition cache invalidations (Gitea #83). + private sealed class CountingDefinitionCacheInvalidator : IExternalDefinitionCacheInvalidator + { + private int _calls; + + /// Gets the number of invalidations received. + public int Calls => Volatile.Read(ref _calls); + + /// + public void InvalidateAll() => Interlocked.Increment(ref _calls); } /// Records every published notification for assertion.