diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs index c8d71116..1fb37f1d 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/IDeploymentManagerRepository.cs @@ -1,4 +1,5 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; @@ -206,6 +207,16 @@ public interface IDeploymentManagerRepository /// The number of rows deleted; fewer than means the sweep is complete. Task PurgeSystemArtifactDeploymentRecordsAsync(DateTimeOffset cutoffUtc, int batchSize, CancellationToken cancellationToken = default); + /// + /// Reads every external-system method in ONE untracked query, for the artifact-deploy + /// global snapshot (which serializes each system's methods into its artifact). Replaces a + /// per-system method query that ran on every artifact deploy and every pending-updates + /// evaluation. + /// + /// A cancellation token that can be used to cancel the operation. + /// All external-system methods; group by ExternalSystemDefinitionId. + Task> GetAllExternalSystemMethodsAsync(CancellationToken cancellationToken = default); + // DeployedConfigSnapshot /// /// Gets the deployed config snapshot for an instance. diff --git a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs index ef0d07be..5215de91 100644 --- a/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs +++ b/src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/DeploymentManagerRepository.cs @@ -1,5 +1,6 @@ using Microsoft.EntityFrameworkCore; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; using ZB.MOM.WW.ScadaBridge.Commons.Entities.Instances; using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories; using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment; @@ -308,6 +309,17 @@ public class DeploymentManagerRepository : IDeploymentManagerRepository return stale.Count; } + /// + public async Task> GetAllExternalSystemMethodsAsync( + CancellationToken cancellationToken = default) + { + return await _dbContext.Set() + .AsNoTracking() + .OrderBy(m => m.ExternalSystemDefinitionId) + .ThenBy(m => m.Id) + .ToListAsync(cancellationToken); + } + // --- DeployedConfigSnapshot --- /// diff --git a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs index 9c580cbe..99926aae 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DeploymentManager/ArtifactDeploymentService.cs @@ -301,7 +301,8 @@ public class ArtifactDeploymentService /// /// The per-site artifact build path previously re-issued /// every one of these queries per site (≈ N + M·N round trips for N sites - /// and M external systems). Hoisting them here drops that to a single fetch. + /// and M external systems). Hoisting them here drops that to a single fetch, and the + /// methods of every external system are read in one query rather than one per system. /// /// Notification lists and SMTP configurations are NOT /// fetched here. Notification delivery is central-only, so they are never @@ -313,6 +314,10 @@ public class ArtifactDeploymentService var externalSystems = await _externalSystemRepo.GetAllExternalSystemsAsync(cancellationToken); var dbConnections = await _externalSystemRepo.GetAllDatabaseConnectionsAsync(cancellationToken); + // Every system's methods in ONE query, grouped here — not one query per system. + var methodsBySystem = (await _deploymentRepo.GetAllExternalSystemMethodsAsync(cancellationToken)) + .ToLookup(m => m.ExternalSystemDefinitionId); + // Map shared scripts var scriptArtifacts = sharedScripts.Select(s => new SharedScriptArtifact(s.Name, s.Code, s.ParameterDefinitions, s.ReturnDefinition)).ToList(); @@ -321,7 +326,7 @@ public class ArtifactDeploymentService var externalSystemArtifacts = new List(); foreach (var es in externalSystems) { - var methods = await _externalSystemRepo.GetMethodsByExternalSystemIdAsync(es.Id, cancellationToken); + var methods = methodsBySystem[es.Id].ToList(); // The property NAMES emitted here are part of a contract: // ArtifactRevisionHashService.CanonicalizeMethodDefinitions re-orders this array by // each element's "Name" property (case-insensitively) so repository row order cannot diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SystemArtifactDeploymentPurgeTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/DeploymentManagerArtifactRepositoryTests.cs similarity index 56% rename from tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SystemArtifactDeploymentPurgeTests.cs rename to tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/DeploymentManagerArtifactRepositoryTests.cs index 7f91a83f..94b04b48 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/SystemArtifactDeploymentPurgeTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/DeploymentManagerArtifactRepositoryTests.cs @@ -1,20 +1,22 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment; +using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems; using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories; namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests; /// -/// Retention purge for rows (review 2026-10-21 +/// Artifact-deploy queries on DeploymentManagerRepository. Retention purge for +/// rows (review 2026-10-21 /// 08/G2): one row is written per Deploy Artifacts run and nothing reads them back, so they /// must share the terminal deployment-record retention window. /// -public sealed class SystemArtifactDeploymentPurgeTests : IDisposable +public sealed class DeploymentManagerArtifactRepositoryTests : IDisposable { private readonly ScadaBridgeDbContext _context = SqliteTestHelper.CreateInMemoryContext(); private readonly DeploymentManagerRepository _repository; /// Initializes the repository over a fresh in-memory SQLite context. - public SystemArtifactDeploymentPurgeTests() + public DeploymentManagerArtifactRepositoryTests() { _repository = new DeploymentManagerRepository(_context); } @@ -49,4 +51,30 @@ public sealed class SystemArtifactDeploymentPurgeTests : IDisposable var survivor = Assert.Single(remaining); Assert.True(survivor.DeployedAt > cutoff); } + + /// + /// review 2026-10-21 04/P7: the artifact-deploy method read returns every system's methods + /// in one untracked query. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task GetAllExternalSystemMethods_ReturnsEverySystemsMethods_Untracked() + { + var a = new ExternalSystemDefinition("A", "https://a.test", "ApiKey"); + var b = new ExternalSystemDefinition("B", "https://b.test", "ApiKey"); + _context.AddRange(a, b); + await _context.SaveChangesAsync(); + _context.AddRange( + new ExternalSystemMethod("A1", "GET", "/a1") { ExternalSystemDefinitionId = a.Id }, + new ExternalSystemMethod("B1", "GET", "/b1") { ExternalSystemDefinitionId = b.Id }, + new ExternalSystemMethod("B2", "POST", "/b2") { ExternalSystemDefinitionId = b.Id }); + await _context.SaveChangesAsync(); + _context.ChangeTracker.Clear(); + + var methods = await _repository.GetAllExternalSystemMethodsAsync(); + + Assert.Equal(3, methods.Count); + Assert.Equal(2, methods.Count(m => m.ExternalSystemDefinitionId == b.Id)); + Assert.Empty(_context.ChangeTracker.Entries()); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs index a6bab7e8..829ba6d4 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/ArtifactDeploymentServiceTests.cs @@ -255,6 +255,47 @@ public class ArtifactDeploymentServiceTests : TestKit Assert.Equal(TimeSpan.FromSeconds(120), artifact.RetryDelay); } + /// + /// review 2026-10-21 04/P7: every external system's methods come from ONE query, grouped + /// per system — not one method query per external system — and each system's artifact + /// still carries exactly its own methods. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task ExternalSystemMethods_AreReadInOneQuery_AndGroupedPerSystem() + { + _externalSystemRepo.GetAllExternalSystemsAsync(Arg.Any()).Returns( + [ + new ExternalSystemDefinition("Mes", "https://mes", "ApiKey") { Id = 1 }, + new ExternalSystemDefinition("Erp", "https://erp", "ApiKey") { Id = 2 }, + new ExternalSystemDefinition("Idle", "https://idle", "ApiKey") { Id = 3 }, + ]); + _deploymentRepo.GetAllExternalSystemMethodsAsync(Arg.Any()).Returns( + [ + new ExternalSystemMethod("MoveOut", "POST", "/move") { Id = 10, ExternalSystemDefinitionId = 1 }, + new ExternalSystemMethod("GetOrder", "GET", "/order") { Id = 11, ExternalSystemDefinitionId = 2 }, + new ExternalSystemMethod("PostOrder", "POST", "/order") { Id = 12, ExternalSystemDefinitionId = 2 }, + ]); + + var recorder = new ArtifactProbeRecorder(); + var probe = Sys.ActorOf(Props.Create(() => new ArtifactProbeActor(recorder))); + var service = CreateServiceWithCommActor(probe); + + var result = await service.RetryForSiteAsync(1, "retry-site", "admin"); + + Assert.True(result.IsSuccess); + await _deploymentRepo.Received(1).GetAllExternalSystemMethodsAsync(Arg.Any()); + await _externalSystemRepo.DidNotReceive().GetMethodsByExternalSystemIdAsync( + Arg.Any(), Arg.Any()); + + var artifacts = Assert.Single(recorder.Received).ExternalSystems!.ToDictionary(a => a.Name); + Assert.Contains("MoveOut", artifacts["Mes"].MethodDefinitionsJson); + Assert.DoesNotContain("Order", artifacts["Mes"].MethodDefinitionsJson); + Assert.Contains("GetOrder", artifacts["Erp"].MethodDefinitionsJson); + Assert.Contains("PostOrder", artifacts["Erp"].MethodDefinitionsJson); + Assert.Null(artifacts["Idle"].MethodDefinitionsJson); + } + /// Verifies that retrying a single site still runs the global artifact queries exactly once. /// A task that represents the asynchronous operation. [Fact]