fix(deployment): read every external system's methods in one query for the artifact snapshot (review 2026-10-21 04/P7)
This commit is contained in:
+11
@@ -1,4 +1,5 @@
|
|||||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
|
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.Entities.Instances;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment;
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
||||||
@@ -206,6 +207,16 @@ public interface IDeploymentManagerRepository
|
|||||||
/// <returns>The number of rows deleted; fewer than <paramref name="batchSize"/> means the sweep is complete.</returns>
|
/// <returns>The number of rows deleted; fewer than <paramref name="batchSize"/> means the sweep is complete.</returns>
|
||||||
Task<int> PurgeSystemArtifactDeploymentRecordsAsync(DateTimeOffset cutoffUtc, int batchSize, CancellationToken cancellationToken = default);
|
Task<int> PurgeSystemArtifactDeploymentRecordsAsync(DateTimeOffset cutoffUtc, int batchSize, CancellationToken cancellationToken = default);
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="cancellationToken">A cancellation token that can be used to cancel the operation.</param>
|
||||||
|
/// <returns>All external-system methods; group by <c>ExternalSystemDefinitionId</c>.</returns>
|
||||||
|
Task<IReadOnlyList<ExternalSystemMethod>> GetAllExternalSystemMethodsAsync(CancellationToken cancellationToken = default);
|
||||||
|
|
||||||
// DeployedConfigSnapshot
|
// DeployedConfigSnapshot
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Gets the deployed config snapshot for an instance.
|
/// Gets the deployed config snapshot for an instance.
|
||||||
|
|||||||
+12
@@ -1,5 +1,6 @@
|
|||||||
using Microsoft.EntityFrameworkCore;
|
using Microsoft.EntityFrameworkCore;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
|
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.Entities.Instances;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment;
|
using ZB.MOM.WW.ScadaBridge.Commons.Types.Deployment;
|
||||||
@@ -308,6 +309,17 @@ public class DeploymentManagerRepository : IDeploymentManagerRepository
|
|||||||
return stale.Count;
|
return stale.Count;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <inheritdoc />
|
||||||
|
public async Task<IReadOnlyList<ExternalSystemMethod>> GetAllExternalSystemMethodsAsync(
|
||||||
|
CancellationToken cancellationToken = default)
|
||||||
|
{
|
||||||
|
return await _dbContext.Set<ExternalSystemMethod>()
|
||||||
|
.AsNoTracking()
|
||||||
|
.OrderBy(m => m.ExternalSystemDefinitionId)
|
||||||
|
.ThenBy(m => m.Id)
|
||||||
|
.ToListAsync(cancellationToken);
|
||||||
|
}
|
||||||
|
|
||||||
// --- DeployedConfigSnapshot ---
|
// --- DeployedConfigSnapshot ---
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
|
|||||||
@@ -301,7 +301,8 @@ public class ArtifactDeploymentService
|
|||||||
/// <remarks>
|
/// <remarks>
|
||||||
/// The per-site artifact build path previously re-issued
|
/// The per-site artifact build path previously re-issued
|
||||||
/// every one of these queries per site (≈ N + M·N round trips for N sites
|
/// 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
|
/// Notification lists and SMTP configurations are NOT
|
||||||
/// fetched here. Notification delivery is central-only, so they are never
|
/// 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 externalSystems = await _externalSystemRepo.GetAllExternalSystemsAsync(cancellationToken);
|
||||||
var dbConnections = await _externalSystemRepo.GetAllDatabaseConnectionsAsync(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
|
// Map shared scripts
|
||||||
var scriptArtifacts = sharedScripts.Select(s =>
|
var scriptArtifacts = sharedScripts.Select(s =>
|
||||||
new SharedScriptArtifact(s.Name, s.Code, s.ParameterDefinitions, s.ReturnDefinition)).ToList();
|
new SharedScriptArtifact(s.Name, s.Code, s.ParameterDefinitions, s.ReturnDefinition)).ToList();
|
||||||
@@ -321,7 +326,7 @@ public class ArtifactDeploymentService
|
|||||||
var externalSystemArtifacts = new List<ExternalSystemArtifact>();
|
var externalSystemArtifacts = new List<ExternalSystemArtifact>();
|
||||||
foreach (var es in externalSystems)
|
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:
|
// The property NAMES emitted here are part of a contract:
|
||||||
// ArtifactRevisionHashService.CanonicalizeMethodDefinitions re-orders this array by
|
// ArtifactRevisionHashService.CanonicalizeMethodDefinitions re-orders this array by
|
||||||
// each element's "Name" property (case-insensitively) so repository row order cannot
|
// each element's "Name" property (case-insensitively) so repository row order cannot
|
||||||
|
|||||||
+31
-3
@@ -1,20 +1,22 @@
|
|||||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
|
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Deployment;
|
||||||
|
using ZB.MOM.WW.ScadaBridge.Commons.Entities.ExternalSystems;
|
||||||
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories;
|
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories;
|
||||||
|
|
||||||
namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests;
|
namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests;
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Retention purge for <see cref="SystemArtifactDeploymentRecord"/> rows (review 2026-10-21
|
/// Artifact-deploy queries on DeploymentManagerRepository. Retention purge for
|
||||||
|
/// <see cref="SystemArtifactDeploymentRecord"/> rows (review 2026-10-21
|
||||||
/// 08/G2): one row is written per Deploy Artifacts run and nothing reads them back, so they
|
/// 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.
|
/// must share the terminal deployment-record retention window.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public sealed class SystemArtifactDeploymentPurgeTests : IDisposable
|
public sealed class DeploymentManagerArtifactRepositoryTests : IDisposable
|
||||||
{
|
{
|
||||||
private readonly ScadaBridgeDbContext _context = SqliteTestHelper.CreateInMemoryContext();
|
private readonly ScadaBridgeDbContext _context = SqliteTestHelper.CreateInMemoryContext();
|
||||||
private readonly DeploymentManagerRepository _repository;
|
private readonly DeploymentManagerRepository _repository;
|
||||||
|
|
||||||
/// <summary>Initializes the repository over a fresh in-memory SQLite context.</summary>
|
/// <summary>Initializes the repository over a fresh in-memory SQLite context.</summary>
|
||||||
public SystemArtifactDeploymentPurgeTests()
|
public DeploymentManagerArtifactRepositoryTests()
|
||||||
{
|
{
|
||||||
_repository = new DeploymentManagerRepository(_context);
|
_repository = new DeploymentManagerRepository(_context);
|
||||||
}
|
}
|
||||||
@@ -49,4 +51,30 @@ public sealed class SystemArtifactDeploymentPurgeTests : IDisposable
|
|||||||
var survivor = Assert.Single(remaining);
|
var survivor = Assert.Single(remaining);
|
||||||
Assert.True(survivor.DeployedAt > cutoff);
|
Assert.True(survivor.DeployedAt > cutoff);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// review 2026-10-21 04/P7: the artifact-deploy method read returns every system's methods
|
||||||
|
/// in one untracked query.
|
||||||
|
/// </summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[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<ExternalSystemMethod>());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
@@ -255,6 +255,47 @@ public class ArtifactDeploymentServiceTests : TestKit
|
|||||||
Assert.Equal(TimeSpan.FromSeconds(120), artifact.RetryDelay);
|
Assert.Equal(TimeSpan.FromSeconds(120), artifact.RetryDelay);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// 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.
|
||||||
|
/// </summary>
|
||||||
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
|
[Fact]
|
||||||
|
public async Task ExternalSystemMethods_AreReadInOneQuery_AndGroupedPerSystem()
|
||||||
|
{
|
||||||
|
_externalSystemRepo.GetAllExternalSystemsAsync(Arg.Any<CancellationToken>()).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<CancellationToken>()).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<CancellationToken>());
|
||||||
|
await _externalSystemRepo.DidNotReceive().GetMethodsByExternalSystemIdAsync(
|
||||||
|
Arg.Any<int>(), Arg.Any<CancellationToken>());
|
||||||
|
|
||||||
|
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);
|
||||||
|
}
|
||||||
|
|
||||||
/// <summary>Verifies that retrying a single site still runs the global artifact queries exactly once.</summary>
|
/// <summary>Verifies that retrying a single site still runs the global artifact queries exactly once.</summary>
|
||||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user