From 21b801b71f00aa778c8b8d83844071e1f6f064cf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Tue, 16 Jun 2026 05:34:56 -0400 Subject: [PATCH] =?UTF-8?q?test(template):=20M2.8=20review=20nits=20?= =?UTF-8?q?=E2=80=94=20stale-binding=20comment=20+=20stale-ID=20&=20inert-?= =?UTF-8?q?check=20tests=20(#23)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add code comments in ValidateConnectionBindingCompleteness explaining that the unbound-attribute branch also covers the silently-dropped stale-binding case (cross-reference FlatteningService.ApplyConnectionBindings), and that the `continue` skips the exists-at-site check for unbound attrs. Add two new tests: - FlatteningPipelineConnectionBindingTests: stale DataConnectionId (999) not present in site connections → flattener drops it silently → validator reports ConnectionBinding Error, IsValid false. - ValidationServiceTests: enforce:true + siteConnectionNames:null on a properly-bound attribute → no ConnectionBinding error (exists-at-site check stays inert when site set is not supplied). --- .../Validation/ValidationService.cs | 12 ++++++++ ...latteningPipelineConnectionBindingTests.cs | 23 ++++++++++++++ .../Validation/ValidationServiceTests.cs | 30 +++++++++++++++++++ 3 files changed, 65 insertions(+) diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs index 548e5d60..2dd21b01 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs @@ -580,6 +580,17 @@ public class ValidationService // Unbound data-sourced attribute. At deploy time this gates the // deployment; at design time the binding is set later, so it is // only advisory. + // + // NOTE: this branch fires for TWO distinct cases that are + // indistinguishable post-flattening: + // 1. The user genuinely never set a binding. + // 2. The user set a binding, but FlatteningService.ApplyConnectionBindings + // silently dropped it because the stored DataConnectionId no longer + // resolves to any loaded site DataConnection (i.e. the connection was + // deleted after the binding was created). In that case the flattener + // leaves BoundDataConnectionId == null, and the attribute falls into + // this same "unbound → Error" path. + // The error message covers both cases; no behavioral change is needed. if (enforce) { errors.Add(ValidationEntry.Error(ValidationCategory.ConnectionBinding, @@ -592,6 +603,7 @@ public class ValidationService $"Attribute '{attr.CanonicalName}' has a data source reference but no connection binding.", attr.CanonicalName)); } + // Skip the "exists at site" check below — it only applies to bound attributes. continue; } diff --git a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/FlatteningPipelineConnectionBindingTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/FlatteningPipelineConnectionBindingTests.cs index a6c60fa9..49e7e897 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/FlatteningPipelineConnectionBindingTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DeploymentManager.Tests/FlatteningPipelineConnectionBindingTests.cs @@ -96,4 +96,27 @@ public class FlatteningPipelineConnectionBindingTests Assert.DoesNotContain(result.Value.Validation.Errors, e => e.Category == ValidationCategory.ConnectionBinding); } + + [Fact] + public async Task FlattenAndValidate_BindingToStaleDeletedConnection_ReportsBindingError() + { + // M2.8 (#23): FlatteningService.ApplyConnectionBindings silently drops a + // binding whose DataConnectionId doesn't resolve to any loaded site + // DataConnection (stale / deleted connection). The flattener leaves + // BoundDataConnectionId == null, so the validator treats the attribute as + // unbound and gates the deployment with a ConnectionBinding Error. + // + // Arrange: the instance binding points at id 999, but the site only has + // the connection with id=ConnectionId (7). The flattener can't resolve 999 + // and drops the binding silently; the validator then flags it. + const int StaleConnectionId = 999; + Arrange(boundConnectionId: StaleConnectionId); + + var result = await _sut.FlattenAndValidateAsync(InstanceId); + + Assert.True(result.IsSuccess); + Assert.False(result.Value.Validation.IsValid); + Assert.Contains(result.Value.Validation.Errors, + e => e.Category == ValidationCategory.ConnectionBinding); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs index d5f1b5e8..1bba9d50 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs @@ -298,6 +298,36 @@ public class ValidationServiceTests Assert.False(result.IsValid); } + [Fact] + public void Validate_BoundAttributeWithNoSiteSet_DeployTime_ExistsAtSiteCheckIsInert() + { + // M2.8 (#23): when siteConnectionNames is null the "exists at site" half of the + // binding check stays inert — a properly-bound data-sourced attribute must NOT + // produce a ConnectionBinding error, even under deploy-time enforcement. + // This pins the contract: passing enforce:true + siteConnectionNames:null is safe + // (e.g. when the caller doesn't have a site connection set available yet). + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Attributes = + [ + new ResolvedAttribute + { + CanonicalName = "Temp", + DataType = "Double", + DataSourceReference = "ns=2;s=Temp", + BoundDataConnectionId = 7, + BoundDataConnectionName = "PlantBus" + } + ] + }; + + var result = _sut.Validate(config, enforceConnectionBindings: true, siteConnectionNames: null); + + Assert.DoesNotContain(result.Errors, e => e.Category == ValidationCategory.ConnectionBinding); + Assert.True(result.IsValid); + } + [Fact] public void Validate_EmptyConfig_ReturnsWarning() {