feat(m9/T28a): strict expression-trigger analysis kind (advisory default, strict escalates)

This commit is contained in:
Joseph Doherty
2026-06-18 10:13:12 -04:00
parent 939aea159b
commit 0f04afbdf1
2 changed files with 165 additions and 3 deletions
@@ -320,12 +320,19 @@ public class ValidationService
List<ValidationEntry> warnings) List<ValidationEntry> warnings)
{ {
var expression = ExtractExpressionFromTriggerConfig(triggerConfigJson); var expression = ExtractExpressionFromTriggerConfig(triggerConfigJson);
var strict = IsStrictAnalysis(triggerConfigJson);
if (string.IsNullOrWhiteSpace(expression)) if (string.IsNullOrWhiteSpace(expression))
{ {
warnings.Add(ValidationEntry.Warning(category, // M9-T28a: the blank-expression finding is the only advisory one. Under the
$"The {entityLabel} '{entityName}' has an expression trigger with no expression; it will never fire.", // default Advisory kind it stays a non-blocking warning (it merely never fires);
entityName)); // 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; return;
} }
@@ -374,6 +381,35 @@ public class ValidationService
return null; return null;
} }
/// <summary>
/// Reads the optional <c>"analysisKind"</c> discriminator from a trigger
/// configuration (M9-T28a). <c>true</c> only when the value is the case-insensitive
/// literal <c>"Strict"</c>; everything else — a missing key, the literal
/// <c>"Advisory"</c>, any other value, or malformed JSON — defaults to Advisory
/// (<c>false</c>), preserving today's behavior exactly.
/// </summary>
/// <param name="triggerConfigJson">The trigger configuration JSON to parse.</param>
/// <returns><c>true</c> when the configured analysis kind is Strict; otherwise <c>false</c> (Advisory).</returns>
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;
}
/// <summary> /// <summary>
/// Authoritative syntax/trust check for a trigger expression. Delegates to the /// Authoritative syntax/trust check for a trigger expression. Delegates to the
/// shared <see cref="ZB.MOM.WW.ScadaBridge.ScriptAnalysis"/> analyzer (same gate /// shared <see cref="ZB.MOM.WW.ScadaBridge.ScriptAnalysis"/> analyzer (same gate
@@ -336,4 +336,130 @@ public class ValidationServiceTests
var result = _sut.Validate(config); var result = _sut.Validate(config);
Assert.Contains(result.Warnings, w => w.Category == ValidationCategory.FlatteningFailure); 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);
}
} }