From 0f04afbdf1772dc6da116dcfaf483a2ddd42f892 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 18 Jun 2026 10:13:12 -0400 Subject: [PATCH] feat(m9/T28a): strict expression-trigger analysis kind (advisory default, strict escalates) --- .../Validation/ValidationService.cs | 42 +++++- .../Validation/ValidationServiceTests.cs | 126 ++++++++++++++++++ 2 files changed, 165 insertions(+), 3 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs index c2a56b3b..59e60bc3 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs @@ -320,12 +320,19 @@ public class ValidationService List warnings) { var expression = ExtractExpressionFromTriggerConfig(triggerConfigJson); + var strict = IsStrictAnalysis(triggerConfigJson); if (string.IsNullOrWhiteSpace(expression)) { - warnings.Add(ValidationEntry.Warning(category, - $"The {entityLabel} '{entityName}' has an expression trigger with no expression; it will never fire.", - entityName)); + // M9-T28a: the blank-expression finding is the only advisory one. Under the + // default Advisory kind it stays a non-blocking warning (it merely never fires); + // under Strict it is promoted to a deploy-blocking error. Pure escalation — + // the message and category are unchanged. + var message = $"The {entityLabel} '{entityName}' has an expression trigger with no expression; it will never fire."; + if (strict) + errors.Add(ValidationEntry.Error(category, message, entityName)); + else + warnings.Add(ValidationEntry.Warning(category, message, entityName)); return; } @@ -374,6 +381,35 @@ public class ValidationService return null; } + /// + /// Reads the optional "analysisKind" discriminator from a trigger + /// configuration (M9-T28a). true only when the value is the case-insensitive + /// literal "Strict"; everything else — a missing key, the literal + /// "Advisory", any other value, or malformed JSON — defaults to Advisory + /// (false), preserving today's behavior exactly. + /// + /// The trigger configuration JSON to parse. + /// true when the configured analysis kind is Strict; otherwise false (Advisory). + internal static bool IsStrictAnalysis(string? triggerConfigJson) + { + if (string.IsNullOrWhiteSpace(triggerConfigJson)) + return false; + try + { + using var doc = JsonDocument.Parse(triggerConfigJson); + if (doc.RootElement.TryGetProperty("analysisKind", out var prop) + && prop.ValueKind == JsonValueKind.String) + { + return string.Equals(prop.GetString(), "Strict", StringComparison.OrdinalIgnoreCase); + } + } + catch (JsonException) + { + // Not valid JSON — Advisory (no escalation), matching the blank-expression default. + } + return false; + } + /// /// Authoritative syntax/trust check for a trigger expression. Delegates to the /// shared analyzer (same gate 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 1bba9d50..4ce13f9b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs @@ -336,4 +336,130 @@ public class ValidationServiceTests var result = _sut.Validate(config); Assert.Contains(result.Warnings, w => w.Category == ValidationCategory.FlatteningFailure); } + + // --- M9-T28a: per-trigger AnalysisKind (Advisory default | Strict escalates) --- + // The only currently-advisory finding in CheckExpressionTrigger is the blank/empty + // expression (which "will never fire"). Advisory keeps it a non-blocking warning; + // Strict promotes it to a deploy-blocking error. The kind rides the existing + // trigger-config JSON ({"expression":"...","analysisKind":"Strict"}) — no migration. + + [Fact] + public void Validate_ExpressionTrigger_BlankExpression_AdvisoryDefault_WarnsButPasses() + { + // No analysisKind in the config → Advisory (today's behavior): a blank expression + // is a non-blocking warning, validation still passes. + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Alarms = + [ + new ResolvedAlarm + { + CanonicalName = "BlankAlarm", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"\"}" + } + ] + }; + + var result = _sut.Validate(config); + Assert.Contains(result.Warnings, w => w.Category == ValidationCategory.AlarmTriggerReference); + Assert.DoesNotContain(result.Errors, e => e.Category == ValidationCategory.AlarmTriggerReference); + Assert.True(result.IsValid); + } + + [Fact] + public void Validate_ExpressionTrigger_BlankExpression_StrictKind_FailsWithError() + { + // analysisKind:"Strict" promotes the blank-expression advisory to a deploy-blocking error. + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Alarms = + [ + new ResolvedAlarm + { + CanonicalName = "BlankAlarm", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"\",\"analysisKind\":\"Strict\"}" + } + ] + }; + + var result = _sut.Validate(config); + Assert.Contains(result.Errors, e => e.Category == ValidationCategory.AlarmTriggerReference); + Assert.DoesNotContain(result.Warnings, w => w.Category == ValidationCategory.AlarmTriggerReference); + Assert.False(result.IsValid); + } + + [Fact] + public void Validate_ScriptExpressionTrigger_BlankExpression_StrictKind_FailsWithError() + { + // Strict escalation also applies to script expression triggers (mirrors the alarm path). + var config = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Scripts = + [ + new ResolvedScript + { + CanonicalName = "BlankScript", + Code = "var x = 1;", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\" \",\"analysisKind\":\"Strict\"}" + } + ] + }; + + var result = _sut.Validate(config); + Assert.Contains(result.Errors, e => e.Category == ValidationCategory.ScriptTriggerReference); + Assert.False(result.IsValid); + } + + [Fact] + public void Validate_ExpressionTrigger_ValidExpression_PassesUnderBothKinds() + { + // A genuinely valid expression must pass clean under Advisory AND Strict — + // Strict only escalates the currently-advisory findings, it does not invent new ones. + var attributes = new[] + { + new ResolvedAttribute { CanonicalName = "Temp", Value = "25", DataType = "Double" } + }; + + var advisory = new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Attributes = attributes, + Alarms = + [ + new ResolvedAlarm + { + CanonicalName = "HighTemp", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"(double)Attributes[\\\"Temp\\\"] > 50\"}" + } + ] + }; + + var strict = advisory with + { + Alarms = + [ + new ResolvedAlarm + { + CanonicalName = "HighTemp", + TriggerType = "Expression", + TriggerConfiguration = "{\"expression\":\"(double)Attributes[\\\"Temp\\\"] > 50\",\"analysisKind\":\"Strict\"}" + } + ] + }; + + var advisoryResult = _sut.Validate(advisory); + var strictResult = _sut.Validate(strict); + + Assert.DoesNotContain(advisoryResult.Errors, e => e.Category == ValidationCategory.AlarmTriggerReference); + Assert.DoesNotContain(advisoryResult.Warnings, w => w.Category == ValidationCategory.AlarmTriggerReference); + Assert.DoesNotContain(strictResult.Errors, e => e.Category == ValidationCategory.AlarmTriggerReference); + Assert.DoesNotContain(strictResult.Warnings, w => w.Category == ValidationCategory.AlarmTriggerReference); + } }