diff --git a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs index 8b8db824..1f1d5502 100644 --- a/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.TemplateEngine/Validation/ValidationService.cs @@ -97,11 +97,16 @@ public class ValidationService /// /// /// true (default) runs the authoritative Roslyn - /// stage — the single most expensive check (a non-collectible assembly load per script). - /// false skips ONLY that stage; every other structural/semantic check still runs. - /// Read-only staleness/comparison callers (DeploymentManager's FlatteningPipeline on its - /// comparison/probe paths) pass false because they need only the flattened config and - /// revision hash, not a compile. The deploy gate keeps the default (true). + /// stage — the single most expensive check (a non-collectible assembly load per script) — + /// AND the Expression-trigger syntax/compile check + /// (, a forbidden-API verdict plus a real compile of every + /// Expression-triggered script/alarm body against ). + /// false skips ONLY those two compile stages; every other structural/semantic check + /// still runs — including, for Expression triggers, the blank-expression check and the + /// cheap attribute-reference scan. Read-only staleness/comparison callers (DeploymentManager's + /// FlatteningPipeline on its comparison/probe paths) pass false because they need only + /// the flattened config and revision hash, not a compile. The deploy gate keeps the default + /// (true) — it is the authoritative check for both stages. /// /// A merged aggregating all pipeline stage outcomes. public ValidationResult Validate( @@ -128,7 +133,7 @@ public class ValidationService validateScriptCompilation ? ValidateScriptCompilation(configuration) : ValidationResult.Success(), ValidateAlarmTriggerReferences(configuration), ValidateScriptTriggerReferences(configuration), - ValidateExpressionTriggers(configuration), + ValidateExpressionTriggers(configuration, validateScriptCompilation), ValidateConnectionBindingCompleteness(configuration, enforceConnectionBindings, siteConnectionNames), ValidateSchemaReferences(configuration, sharedScripts, resolveSchemaRef), _semanticValidator.Validate(configuration, sharedScripts, alarmCapableConnectionNames) @@ -366,8 +371,16 @@ public class ValidationService /// /// /// The flattened configuration to validate. + /// + /// true (default) runs the authoritative + /// forbidden-API + compile stage for each Expression trigger. false skips ONLY + /// that stage — the blank-expression check and the attribute-reference scan still run — + /// so read-only staleness/comparison callers do not pay the per-expression compile. + /// /// A with errors and warnings from all expression trigger checks. - public static ValidationResult ValidateExpressionTriggers(FlattenedConfiguration configuration) + public static ValidationResult ValidateExpressionTriggers( + FlattenedConfiguration configuration, + bool validateExpressionSyntax = true) { var errors = new List(); var warnings = new List(); @@ -382,7 +395,7 @@ public class ValidationService CheckExpressionTrigger( ValidationCategory.ScriptTriggerReference, "script", script.CanonicalName, script.TriggerConfiguration, - attributeNames, errors, warnings); + attributeNames, errors, warnings, validateExpressionSyntax); } foreach (var alarm in configuration.Alarms) @@ -393,7 +406,7 @@ public class ValidationService CheckExpressionTrigger( ValidationCategory.AlarmTriggerReference, "alarm", alarm.CanonicalName, alarm.TriggerConfiguration, - attributeNames, errors, warnings); + attributeNames, errors, warnings, validateExpressionSyntax); } return new ValidationResult { Errors = errors, Warnings = warnings }; @@ -424,7 +437,8 @@ public class ValidationService string? triggerConfigJson, HashSet attributeNames, List errors, - List warnings) + List warnings, + bool validateExpressionSyntax = true) { var expression = ExtractExpressionFromTriggerConfig(triggerConfigJson); var strict = IsStrictAnalysis(triggerConfigJson); @@ -443,12 +457,20 @@ public class ValidationService return; } - var syntaxError = CheckExpressionSyntax(expression); - if (syntaxError != null) + // The syntax/compile stage is the expensive one (a forbidden-API verdict + a + // real Roslyn compile of the expression). Read-only staleness/comparison + // callers pass validateExpressionSyntax: false to skip ONLY this stage; the + // blank-expression check above and the attribute-reference scan below still run. + // The deploy gate keeps the default (true) — the authoritative check. + if (validateExpressionSyntax) { - errors.Add(ValidationEntry.Error(category, - $"The {entityLabel} '{entityName}' expression trigger failed validation: {syntaxError}", - entityName)); + var syntaxError = CheckExpressionSyntax(expression); + if (syntaxError != null) + { + errors.Add(ValidationEntry.Error(category, + $"The {entityLabel} '{entityName}' expression trigger failed validation: {syntaxError}", + entityName)); + } } foreach (var attrName in ExtractAttributeReferences(expression)) 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 4ce13f9b..bdd4719c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests/Validation/ValidationServiceTests.cs @@ -462,4 +462,43 @@ public class ValidationServiceTests Assert.DoesNotContain(strictResult.Errors, e => e.Category == ValidationCategory.AlarmTriggerReference); Assert.DoesNotContain(strictResult.Warnings, w => w.Category == ValidationCategory.AlarmTriggerReference); } + + [Fact] + public void Validate_SkipCompilation_SkipsExpressionTriggerSyntaxCheck() + { + var config = ConfigWithExpressionTriggerScript("this is not C# (("); + var gated = new ValidationService().Validate(config, validateScriptCompilation: false); + Assert.DoesNotContain(gated.Errors, e => e.Message.Contains("failed validation")); // FAILS today + var full = new ValidationService().Validate(config); + Assert.Contains(full.Errors, e => e.Message.Contains("failed validation")); // deploy gate unchanged + } + + [Fact] + public void Validate_SkipCompilation_StillChecksExpressionAttributeReferences() + { + // expression "Attributes[\"Ghost\"] != null" referencing a missing attribute: + // the reference scan is cheap string work and MUST survive the gate. + var config = ConfigWithExpressionTriggerScript("Attributes[\"Ghost\"] != null"); + var gated = new ValidationService().Validate(config, validateScriptCompilation: false); + Assert.Contains(gated.Errors, e => e.Message.Contains("Ghost")); + } + + private static FlattenedConfiguration ConfigWithExpressionTriggerScript(string expression) + { + var json = System.Text.Json.JsonSerializer.Serialize(new { expression }); + return new FlattenedConfiguration + { + InstanceUniqueName = "Instance1", + Attributes = [new ResolvedAttribute { CanonicalName = "Temp", Value = "25", DataType = "Double" }], + Scripts = + [ + new ResolvedScript + { + CanonicalName = "OnExpr", + TriggerType = "Expression", + TriggerConfiguration = json + } + ] + }; + } }