diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs index 7ab95ce3..d21b2e14 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs @@ -8,6 +8,7 @@ using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; +using ZB.MOM.WW.ScadaBridge.ExternalSystemGateway; using ZB.MOM.WW.ScadaBridge.Commons.Entities.InboundApi; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts; @@ -2364,6 +2365,23 @@ public class ManagementActor : ReceiveActor HasAuthConfiguration = !string.IsNullOrEmpty(d.AuthConfiguration), }; + /// + /// Drops this node's once an external-system / + /// external-system-method / database-connection edit has committed, so the next inbound-method + /// call on this node resolves the fresh definition rather than waiting out + /// DefinitionCacheTtl (ScadaBridge#65). Queued post-commit: invalidating before the commit + /// would let a concurrent lookup re-cache the pre-edit row. The cache is per-node, so the other + /// central node still converges within DefinitionCacheTtl. A no-op when the cache is not + /// registered (site roles, plain test hosts). + /// + private static Task InvalidateExternalDefinitionCacheAfterCommit(IServiceProvider sp) + { + var cache = sp.GetService(); + return cache is null + ? Task.CompletedTask + : AfterConfigurationCommit(sp, () => { cache.InvalidateAll(); return Task.CompletedTask; }); + } + private static async Task HandleListExternalSystems(IServiceProvider sp) { var repo = sp.GetRequiredService(); @@ -2392,6 +2410,7 @@ public class ManagementActor : ReceiveActor await repo.AddExternalSystemAsync(def); await repo.SaveChangesAsync(); var publicShape = ExternalSystemPublicShape(def); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Create", "ExternalSystem", def.Id.ToString(), def.Name, publicShape); return publicShape; } @@ -2414,6 +2433,7 @@ public class ManagementActor : ReceiveActor await repo.UpdateExternalSystemAsync(def); await repo.SaveChangesAsync(); var publicShape = ExternalSystemPublicShape(def); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Update", "ExternalSystem", def.Id.ToString(), def.Name, publicShape); return publicShape; } @@ -2423,6 +2443,7 @@ public class ManagementActor : ReceiveActor var repo = sp.GetRequiredService(); await repo.DeleteExternalSystemAsync(cmd.ExternalSystemId); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Delete", "ExternalSystem", cmd.ExternalSystemId.ToString(), cmd.ExternalSystemId.ToString(), null); return true; } @@ -2450,6 +2471,7 @@ public class ManagementActor : ReceiveActor }; await repo.AddExternalSystemMethodAsync(method); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Create", "ExternalSystemMethod", method.Id.ToString(), method.Name, method); return method; } @@ -2466,6 +2488,7 @@ public class ManagementActor : ReceiveActor if (cmd.ReturnDefinition != null) method.ReturnDefinition = cmd.ReturnDefinition; await repo.UpdateExternalSystemMethodAsync(method); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Update", "ExternalSystemMethod", method.Id.ToString(), method.Name, method); return method; } @@ -2475,6 +2498,7 @@ public class ManagementActor : ReceiveActor var repo = sp.GetRequiredService(); await repo.DeleteExternalSystemMethodAsync(cmd.MethodId); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Delete", "ExternalSystemMethod", cmd.MethodId.ToString(), cmd.MethodId.ToString(), null); return true; } @@ -3681,6 +3705,7 @@ public class ManagementActor : ReceiveActor if (cmd.RetryDelay is { } retryDelay) def.RetryDelay = retryDelay; await repo.AddDatabaseConnectionAsync(def); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Create", "DatabaseConnection", def.Id.ToString(), def.Name, new { def.Id, def.Name }); return DatabaseConnectionPublicShape(def); } @@ -3699,6 +3724,7 @@ public class ManagementActor : ReceiveActor if (cmd.RetryDelay is { } retryDelay) def.RetryDelay = retryDelay; await repo.UpdateDatabaseConnectionAsync(def); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Update", "DatabaseConnection", def.Id.ToString(), def.Name, new { def.Id, def.Name }); return DatabaseConnectionPublicShape(def); } @@ -3708,6 +3734,7 @@ public class ManagementActor : ReceiveActor var repo = sp.GetRequiredService(); await repo.DeleteDatabaseConnectionAsync(cmd.DatabaseConnectionId); await repo.SaveChangesAsync(); + await InvalidateExternalDefinitionCacheAfterCommit(sp); await AuditAsync(sp, user, "Delete", "DatabaseConnection", cmd.DatabaseConnectionId.ToString(), cmd.DatabaseConnectionId.ToString(), null); return true; } diff --git a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ZB.MOM.WW.ScadaBridge.ManagementService.csproj b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ZB.MOM.WW.ScadaBridge.ManagementService.csproj index e86a51d5..67db2fc2 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ManagementService/ZB.MOM.WW.ScadaBridge.ManagementService.csproj +++ b/src/ZB.MOM.WW.ScadaBridge.ManagementService/ZB.MOM.WW.ScadaBridge.ManagementService.csproj @@ -24,5 +24,6 @@ + diff --git a/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ExternalDefinitionCacheInvalidationTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ExternalDefinitionCacheInvalidationTests.cs new file mode 100644 index 00000000..e72605bd --- /dev/null +++ b/tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ExternalDefinitionCacheInvalidationTests.cs @@ -0,0 +1,106 @@ +using Akka.Actor; +using Akka.TestKit.Xunit2; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using NSubstitute; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; +using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services; +using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; +using ZB.MOM.WW.ScadaBridge.ExternalSystemGateway; +using ZB.MOM.WW.ScadaBridge.Security; + +namespace ZB.MOM.WW.ScadaBridge.ManagementService.Tests; + +/// +/// ScadaBridge#65: the central used to be dropped only +/// by its TTL, so an external-system / method / database-connection edit made through the management +/// path was invisible to inbound-method calls on the same node for up to DefinitionCacheTtl. +/// Every such mutation now invalidates it once the change is committed. +/// +public class ExternalDefinitionCacheInvalidationTests : TestKit +{ + private readonly IExternalSystemRepository _repo = Substitute.For(); + // A TTL far longer than the test: only an explicit InvalidateAll can make the cache re-fetch. + private readonly ExternalDefinitionLookupCache _cache = new(TimeSpan.FromHours(1)); + private readonly ServiceCollection _services = new ServiceCollection().AddTestConfigurationTransaction(); + + /// Initializes the harness with one external system, one method and one DB connection. + public ExternalDefinitionCacheInvalidationTests() + { + _repo.GetExternalSystemByIdAsync(1, Arg.Any()) + .Returns(_ => new ExternalSystemDefinition("Erp", "http://erp", "None") { Id = 1 }); + _repo.GetExternalSystemMethodByIdAsync(2, Arg.Any()) + .Returns(_ => new ExternalSystemMethod("Get", "GET", "/x") { Id = 2, ExternalSystemDefinitionId = 1 }); + _repo.GetDatabaseConnectionByIdAsync(3, Arg.Any()) + .Returns(_ => new DatabaseConnectionDefinition("Db", "Server=x") { Id = 3 }); + _services.AddScoped(_ => _repo); + _services.AddScoped(_ => Substitute.For()); + _services.AddSingleton(_cache); + } + + private IActorRef CreateActor() => + Sys.ActorOf(Props.Create(() => new ManagementActor( + _services.BuildServiceProvider(), NullLogger.Instance))); + + private static ManagementEnvelope Admin(object command) => + new(new AuthenticatedUser("admin", "Admin", new[] { "Designer" }, Array.Empty()), + command, Guid.NewGuid().ToString("N")); + + private async Task PrimeAndCountFetches() + { + var fetches = 0; + Task Fetch() + { + fetches++; + return Task.FromResult(new ExternalSystemDefinition("Erp", "http://erp", "None")); + } + + await _cache.GetSystemAsync("Erp", Fetch); + await _cache.GetSystemAsync("Erp", Fetch); + Assert.Equal(1, fetches); // second read was served from the cache + return fetches; + } + + private async Task IsStillCached() + { + var refetched = false; + await _cache.GetSystemAsync("Erp", () => + { + refetched = true; + return Task.FromResult(new ExternalSystemDefinition("Erp", "http://erp", "None")); + }); + return !refetched; + } + + /// Every external-system / method / database-connection mutation drops the cached definitions. + /// A task that represents the asynchronous operation. + [Fact] + public async Task EveryDefinitionMutation_InvalidatesTheCache() + { + object[] commands = + [ + new CreateExternalSystemCommand("New", "http://n", "None", null), + new UpdateExternalSystemCommand(1, "Erp", "http://erp2", "None", null), + new DeleteExternalSystemCommand(1), + new CreateExternalSystemMethodCommand(1, "M", "GET", "/m"), + new UpdateExternalSystemMethodCommand(2, Path: "/y"), + new DeleteExternalSystemMethodCommand(2), + new CreateDatabaseConnectionDefCommand("Db2", "Server=y"), + new UpdateDatabaseConnectionDefCommand(3, "Db", "Server=z"), + new DeleteDatabaseConnectionDefCommand(3), + ]; + var actor = CreateActor(); + foreach (var command in commands) + { + _cache.InvalidateAll(); + await PrimeAndCountFetches(); + Assert.True(await IsStillCached()); + + actor.Tell(Admin(command)); + ExpectMsg(TimeSpan.FromSeconds(5)); + + Assert.False(await IsStillCached()); // dropped by the edit, so the lookup re-fetched + } + } +}