dohertj2/ScadaBridge
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

fix(m9/T24a): scope move-guard native-alarm scan to source-site templates (Ordinal); purpose-built include; add guard-4 + repo tests

Browse Source
This commit is contained in:
Joseph Doherty
2026-06-18 11:38:31 -04:00
parent fbe4ddaf58
commit dbe51e5f25
5 changed files with 290 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Repositories/ISiteRepository.cs
+12
View File
@@ -90,6 +90,18 @@ public interface ISiteRepository
/// <returns>A task that resolves to the distinct referencing <see cref="Instance"/> entities (empty when none).</returns>
Task<IReadOnlyList<Instance>> GetInstancesReferencingDataConnectionAsync(int dataConnectionId, CancellationToken cancellationToken = default);
/// <summary>
/// Retrieves a site's instances with their
/// <see cref="Instance.NativeAlarmSourceOverrides"/> eagerly loaded. Purpose-built
/// for the move-data-connection guard, which must scan source-site instance
/// overrides for name-based connection references without paying the eager-load
/// cost on the hot-path <see cref="GetInstancesBySiteIdAsync"/>.
/// </summary>
/// <param name="siteId">The site primary key to filter by.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task that resolves to the site's <see cref="Instance"/> entities with overrides loaded (empty when none).</returns>
Task<IReadOnlyList<Instance>> GetInstancesWithNativeAlarmOverridesBySiteIdAsync(int siteId, CancellationToken cancellationToken = default);
/// <summary>Saves all pending changes to the database.</summary>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task that resolves to the number of state entries written to the database.</returns>
src/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase/Repositories/SiteRepository.cs
+8 -2
View File
@@ -132,9 +132,15 @@ public class SiteRepository : ISiteRepository
{
return await _dbContext.Instances
.Where(i => i.SiteId == siteId)
.Include(i => i.ConnectionBindings)
.ToListAsync(cancellationToken);
}
/// <inheritdoc />
public async Task<IReadOnlyList<Instance>> GetInstancesWithNativeAlarmOverridesBySiteIdAsync(int siteId, CancellationToken cancellationToken = default)
{
return await _dbContext.Instances
.Where(i => i.SiteId == siteId)
.Include(i => i.NativeAlarmSourceOverrides)
.AsSplitQuery()
.ToListAsync(cancellationToken);
}
src/ZB.MOM.WW.ScadaBridge.ManagementService/ManagementActor.cs
+29 -7
View File
@@ -1426,17 +1426,40 @@ public class ManagementActor : ReceiveActor
// instances may override that name
// (InstanceNativeAlarmSourceOverride.ConnectionNameOverride). Moving a
// connection changes which physical connection that name resolves to on
// the source site, so any such reference to the moving name is a blocker.
// Detect and report only — never auto-rewrite.
// the SOURCE site, so any such reference *on the source site* to the
// moving name is a blocker. Detect and report only — never auto-rewrite.
//
// Scoping: templates are site-agnostic and ConnectionName resolves
// against the DEPLOYING site's connection pool at flatten time, so the
// template scan is restricted to templates actually instantiated on the
// SOURCE site. A template referencing the same connection NAME but only
// instantiated on another site is NOT affected by this move (that site
// keeps its own connection of the same name), so a global template scan
// would be a false positive that over-blocks legitimate moves whenever
// connection names are reused across sites (the common case).
//
// Comparison is StringComparison.Ordinal to match the
// flattening/deployment pipeline (FlatteningService / FlatteningPipeline
// resolve connection names case-sensitively) — block exactly the
// references the runtime would actually fail to resolve.
var referenceBlockers = new List<string>();
// Source-site instances with their native-alarm-source overrides eagerly
// loaded (purpose-built load so the hot-path GetInstancesBySiteIdAsync
// stays lean). Reused below for both the override scan and to derive the
// set of templates to scan.
var sourceInstances = await repo.GetInstancesWithNativeAlarmOverridesBySiteIdAsync(sourceSiteId);
// Only templates instantiated on the SOURCE site can be broken by the move.
var templateRepo = sp.GetRequiredService<ITemplateEngineRepository>();
var templates = await templateRepo.GetAllTemplatesAsync();
foreach (var template in templates)
var sourceTemplateIds = sourceInstances.Select(i => i.TemplateId).Distinct();
foreach (var templateId in sourceTemplateIds)
{
var template = await templateRepo.GetTemplateByIdAsync(templateId);
if (template is null) continue;
foreach (var source in template.NativeAlarmSources)
{
if (string.Equals(source.ConnectionName, conn.Name, StringComparison.OrdinalIgnoreCase))
if (string.Equals(source.ConnectionName, conn.Name, StringComparison.Ordinal))
{
referenceBlockers.Add(
$"template '{template.Name}' native-alarm-source '{source.Name}'");
@@ -1446,12 +1469,11 @@ public class ManagementActor : ReceiveActor
// Instance-level overrides on the SOURCE site that name this connection
// would orphan once the connection leaves the site.
var sourceInstances = await repo.GetInstancesBySiteIdAsync(sourceSiteId);
foreach (var instance in sourceInstances)
{
foreach (var ovr in instance.NativeAlarmSourceOverrides)
{
if (string.Equals(ovr.ConnectionNameOverride, conn.Name, StringComparison.OrdinalIgnoreCase))
if (string.Equals(ovr.ConnectionNameOverride, conn.Name, StringComparison.Ordinal))
{
referenceBlockers.Add(
$"instance '{instance.UniqueName}' (ID {instance.Id}) native-alarm-source override '{ovr.SourceCanonicalName}'");
tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/RepositoryCoverageTests.cs
+70
View File
@@ -776,6 +776,76 @@ public class SiteRepositoryTests : IDisposable
Assert.Single(instances);
}
[Fact]
public async Task GetInstancesReferencingDataConnection_NoBindings_ReturnsEmpty()
{
var site = new Site("Site1", "S-001");
var template = new Template("T1");
_context.Sites.Add(site);
_context.Templates.Add(template);
await _context.SaveChangesAsync();
var conn = new DataConnection("Conn1", "OpcUa", site.Id);
_context.DataConnections.Add(conn);
_context.Instances.Add(new Instance("I1") { SiteId = site.Id, TemplateId = template.Id });
await _context.SaveChangesAsync();
var result = await _repository.GetInstancesReferencingDataConnectionAsync(conn.Id);
Assert.Empty(result);
}
[Fact]
public async Task GetInstancesReferencingDataConnection_OneBinding_ReturnsThatInstance()
{
var site = new Site("Site1", "S-001");
var template = new Template("T1");
_context.Sites.Add(site);
_context.Templates.Add(template);
await _context.SaveChangesAsync();
var conn = new DataConnection("Conn1", "OpcUa", site.Id);
_context.DataConnections.Add(conn);
var instance = new Instance("I1") { SiteId = site.Id, TemplateId = template.Id };
_context.Instances.Add(instance);
await _context.SaveChangesAsync();
_context.InstanceConnectionBindings.Add(
new InstanceConnectionBinding("Attr1") { InstanceId = instance.Id, DataConnectionId = conn.Id });
await _context.SaveChangesAsync();
var result = await _repository.GetInstancesReferencingDataConnectionAsync(conn.Id);
Assert.Single(result);
Assert.Equal(instance.Id, result[0].Id);
}
[Fact]
public async Task GetInstancesReferencingDataConnection_TwoBindingsSameInstance_ReturnsOne()
{
var site = new Site("Site1", "S-001");
var template = new Template("T1");
_context.Sites.Add(site);
_context.Templates.Add(template);
await _context.SaveChangesAsync();
var conn = new DataConnection("Conn1", "OpcUa", site.Id);
_context.DataConnections.Add(conn);
var instance = new Instance("I1") { SiteId = site.Id, TemplateId = template.Id };
_context.Instances.Add(instance);
await _context.SaveChangesAsync();
// Two distinct bindings (different attributes) on the SAME instance both
// reference the connection -> the result must be deduped to one instance.
_context.InstanceConnectionBindings.Add(
new InstanceConnectionBinding("Attr1") { InstanceId = instance.Id, DataConnectionId = conn.Id });
_context.InstanceConnectionBindings.Add(
new InstanceConnectionBinding("Attr2") { InstanceId = instance.Id, DataConnectionId = conn.Id });
await _context.SaveChangesAsync();
var result = await _repository.GetInstancesReferencingDataConnectionAsync(conn.Id);
Assert.Single(result);
Assert.Equal(instance.Id, result[0].Id);
}
[Fact]
public void Constructor_NullContext_Throws()
{
tests/ZB.MOM.WW.ScadaBridge.ManagementService.Tests/ManagementActorTests.cs
+171 -6
View File
@@ -2104,18 +2104,14 @@ public class ManagementActorTests : TestKit, IDisposable
// No bindings reference the connection -> move allowed.
siteRepo.GetInstancesReferencingDataConnectionAsync(5, Arg.Any<CancellationToken>())
.Returns(new List<Instance>());
// No native-alarm-source overrides on the source site.
siteRepo.GetInstancesBySiteIdAsync(1, Arg.Any<CancellationToken>())
// No native-alarm-source overrides on the source site (purpose-built load).
siteRepo.GetInstancesWithNativeAlarmOverridesBySiteIdAsync(1, Arg.Any<CancellationToken>())
.Returns(new List<Instance>());
siteRepo.UpdateDataConnectionAsync(Arg.Any<Commons.Entities.Sites.DataConnection>(), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
siteRepo.SaveChangesAsync(Arg.Any<CancellationToken>()).Returns(1);
_services.AddScoped(_ => siteRepo);
// No templates reference the connection by name.
_templateRepo.GetAllTemplatesAsync(Arg.Any<CancellationToken>())
.Returns(new List<Template>());
var actor = CreateActor();
var envelope = Envelope(new MoveDataConnectionCommand(5, 2), "Designer");
@@ -2213,4 +2209,173 @@ public class ManagementActorTests : TestKit, IDisposable
siteRepo.DidNotReceive().UpdateDataConnectionAsync(
Arg.Any<Commons.Entities.Sites.DataConnection>(), Arg.Any<CancellationToken>());
}
[Fact]
public void MoveDataConnection_SameSite_ReturnsErrorNoWrite()
{
// Moving a connection to the site it already belongs to is a no-op and
// must be rejected before any write — and before the existence/collision
// guards even run.
var conn = new Commons.Entities.Sites.DataConnection("Opc1", "OpcUa", 1) { Id = 5 };
var siteRepo = Substitute.For<ISiteRepository>();
siteRepo.GetDataConnectionByIdAsync(5, Arg.Any<CancellationToken>()).Returns(conn);
_services.AddScoped(_ => siteRepo);
var actor = CreateActor();
var envelope = Envelope(new MoveDataConnectionCommand(5, 1), "Designer");
actor.Tell(envelope);
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
Assert.Contains("already belongs", response.Error);
Assert.Equal(1, conn.SiteId);
siteRepo.DidNotReceive().UpdateDataConnectionAsync(
Arg.Any<Commons.Entities.Sites.DataConnection>(), Arg.Any<CancellationToken>());
siteRepo.DidNotReceive().SaveChangesAsync(Arg.Any<CancellationToken>());
}
[Fact]
public void MoveDataConnection_BlockedByTemplateNativeAlarmSourceOnSourceSite_ReturnsError()
{
// A template instantiated on the SOURCE site has a native-alarm-source
// whose ConnectionName matches the moving connection -> blocked, because
// the deploying-site flatten would no longer resolve that name.
var conn = new Commons.Entities.Sites.DataConnection("Opc1", "OpcUa", 1) { Id = 5 };
var targetSite = new Commons.Entities.Sites.Site("Target", "SITE-B") { Id = 2 };
// Source-site instance referencing template 42.
var sourceInstance = new Instance("Tank01") { Id = 7, SiteId = 1, TemplateId = 42 };
var template = new Template("PumpTemplate") { Id = 42 };
template.NativeAlarmSources.Add(
new TemplateNativeAlarmSource("PumpFaults") { Id = 1, TemplateId = 42, ConnectionName = "Opc1" });
var siteRepo = Substitute.For<ISiteRepository>();
siteRepo.GetDataConnectionByIdAsync(5, Arg.Any<CancellationToken>()).Returns(conn);
siteRepo.GetSiteByIdAsync(2, Arg.Any<CancellationToken>()).Returns(targetSite);
siteRepo.GetDataConnectionsBySiteIdAsync(2, Arg.Any<CancellationToken>())
.Returns(new List<Commons.Entities.Sites.DataConnection>());
siteRepo.GetInstancesReferencingDataConnectionAsync(5, Arg.Any<CancellationToken>())
.Returns(new List<Instance>());
// Source-site instances (purpose-built load with overrides eagerly loaded).
siteRepo.GetInstancesWithNativeAlarmOverridesBySiteIdAsync(1, Arg.Any<CancellationToken>())
.Returns(new List<Instance> { sourceInstance });
_services.AddScoped(_ => siteRepo);
// Only template 42 (instantiated on the source site) is scanned.
_templateRepo.GetTemplateByIdAsync(42, Arg.Any<CancellationToken>()).Returns(template);
var actor = CreateActor();
var envelope = Envelope(new MoveDataConnectionCommand(5, 2), "Designer");
actor.Tell(envelope);
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
// Error names the template and its native-alarm-source.
Assert.Contains("PumpTemplate", response.Error);
Assert.Contains("PumpFaults", response.Error);
Assert.Equal(1, conn.SiteId);
siteRepo.DidNotReceive().UpdateDataConnectionAsync(
Arg.Any<Commons.Entities.Sites.DataConnection>(), Arg.Any<CancellationToken>());
}
[Fact]
public void MoveDataConnection_BlockedByInstanceOverrideOnSourceSite_ReturnsError()
{
// A source-site instance has a native-alarm-source override whose
// ConnectionNameOverride matches the moving connection -> blocked.
var conn = new Commons.Entities.Sites.DataConnection("Opc1", "OpcUa", 1) { Id = 5 };
var targetSite = new Commons.Entities.Sites.Site("Target", "SITE-B") { Id = 2 };
var sourceInstance = new Instance("Tank01") { Id = 7, SiteId = 1, TemplateId = 42 };
sourceInstance.NativeAlarmSourceOverrides.Add(
new InstanceNativeAlarmSourceOverride("PumpFaults")
{ Id = 1, InstanceId = 7, ConnectionNameOverride = "Opc1" });
var siteRepo = Substitute.For<ISiteRepository>();
siteRepo.GetDataConnectionByIdAsync(5, Arg.Any<CancellationToken>()).Returns(conn);
siteRepo.GetSiteByIdAsync(2, Arg.Any<CancellationToken>()).Returns(targetSite);
siteRepo.GetDataConnectionsBySiteIdAsync(2, Arg.Any<CancellationToken>())
.Returns(new List<Commons.Entities.Sites.DataConnection>());
siteRepo.GetInstancesReferencingDataConnectionAsync(5, Arg.Any<CancellationToken>())
.Returns(new List<Instance>());
siteRepo.GetInstancesWithNativeAlarmOverridesBySiteIdAsync(1, Arg.Any<CancellationToken>())
.Returns(new List<Instance> { sourceInstance });
_services.AddScoped(_ => siteRepo);
// Template 42 has no matching native-alarm-source; the override is the blocker.
_templateRepo.GetTemplateByIdAsync(42, Arg.Any<CancellationToken>())
.Returns(new Template("PumpTemplate") { Id = 42 });
var actor = CreateActor();
var envelope = Envelope(new MoveDataConnectionCommand(5, 2), "Designer");
actor.Tell(envelope);
var response = ExpectMsg<ManagementError>(TimeSpan.FromSeconds(5));
// Error names the instance and its override.
Assert.Contains("Tank01", response.Error);
Assert.Contains("PumpFaults", response.Error);
Assert.Equal(1, conn.SiteId);
siteRepo.DidNotReceive().UpdateDataConnectionAsync(
Arg.Any<Commons.Entities.Sites.DataConnection>(), Arg.Any<CancellationToken>());
}
[Fact]
public void MoveDataConnection_TemplateReferencedOnlyByOtherSite_DoesNotBlock()
{
// Regression for the over-blocking false positive: a template references
// the connection NAME ("Opc1") but is instantiated ONLY on another site.
// Templates are site-agnostic and ConnectionName resolves against the
// DEPLOYING site's pool, so moving site 1's "Opc1" does not break the
// other site's deployment. The move must succeed.
var conn = new Commons.Entities.Sites.DataConnection("Opc1", "OpcUa", 1) { Id = 5 };
var targetSite = new Commons.Entities.Sites.Site("Target", "SITE-B") { Id = 2 };
// The SOURCE site (1) has an instance using template 99, which does NOT
// reference "Opc1". A different template (42) referencing "Opc1" exists
// but is instantiated only on the target/other site, so it is never loaded.
var sourceInstance = new Instance("Tank01") { Id = 7, SiteId = 1, TemplateId = 99 };
var siteRepo = Substitute.For<ISiteRepository>();
siteRepo.GetDataConnectionByIdAsync(5, Arg.Any<CancellationToken>()).Returns(conn);
siteRepo.GetSiteByIdAsync(2, Arg.Any<CancellationToken>()).Returns(targetSite);
siteRepo.GetDataConnectionsBySiteIdAsync(2, Arg.Any<CancellationToken>())
.Returns(new List<Commons.Entities.Sites.DataConnection>());
siteRepo.GetInstancesReferencingDataConnectionAsync(5, Arg.Any<CancellationToken>())
.Returns(new List<Instance>());
siteRepo.GetInstancesWithNativeAlarmOverridesBySiteIdAsync(1, Arg.Any<CancellationToken>())
.Returns(new List<Instance> { sourceInstance });
siteRepo.UpdateDataConnectionAsync(Arg.Any<Commons.Entities.Sites.DataConnection>(), Arg.Any<CancellationToken>())
.Returns(Task.CompletedTask);
siteRepo.SaveChangesAsync(Arg.Any<CancellationToken>()).Returns(1);
_services.AddScoped(_ => siteRepo);
// Template 99 (the one actually on the source site) does not reference "Opc1".
_templateRepo.GetTemplateByIdAsync(99, Arg.Any<CancellationToken>())
.Returns(new Template("OtherTemplate") { Id = 99 });
// Template 42 references "Opc1" but lives only on another site: if the
// handler scoped its scan correctly it must NEVER load template 42. We
// still stub it to prove the cross-site reference is irrelevant.
var crossSiteTemplate = new Template("PumpTemplate") { Id = 42 };
crossSiteTemplate.NativeAlarmSources.Add(
new TemplateNativeAlarmSource("PumpFaults") { Id = 1, TemplateId = 42, ConnectionName = "Opc1" });
_templateRepo.GetTemplateByIdAsync(42, Arg.Any<CancellationToken>()).Returns(crossSiteTemplate);
var actor = CreateActor();
var envelope = Envelope(new MoveDataConnectionCommand(5, 2), "Designer");
actor.Tell(envelope);
// Move SUCCEEDS: the cross-site template reference must not block.
var response = ExpectMsg<ManagementSuccess>(TimeSpan.FromSeconds(5));
Assert.Equal(envelope.CorrelationId, response.CorrelationId);
Assert.Equal(2, conn.SiteId);
siteRepo.Received(1).UpdateDataConnectionAsync(
Arg.Is<Commons.Entities.Sites.DataConnection>(c => c.Id == 5 && c.SiteId == 2),
Arg.Any<CancellationToken>());
// The off-site template (42) must never be scanned.
_templateRepo.DidNotReceive().GetTemplateByIdAsync(42, Arg.Any<CancellationToken>());
}
}