perf(template-engine): cache Expression-trigger compile verdicts under the trigger-surface key — unchanged expressions compile once per process (plan R2-05 T3)
This commit is contained in:
@@ -530,19 +530,27 @@ public class ValidationService
|
||||
/// <returns>A human-readable error message if the expression is invalid; <c>null</c> if well-formed.</returns>
|
||||
internal static string? CheckExpressionSyntax(string expression)
|
||||
{
|
||||
// Authoritative forbidden-API verdict first. Report ALL violations, not just
|
||||
// the first, so an operator fixing one forbidden API isn't surprised by a
|
||||
// second on the next deploy attempt (mirrors ScriptCompiler.TryCompile).
|
||||
var violations = ScriptTrustValidator.FindViolations(expression);
|
||||
if (violations.Count > 0)
|
||||
return $"uses forbidden API: {string.Join("; ", violations)}";
|
||||
// Memoise the verdict under the TRIGGER surface key (see ScriptCompileVerdictCache):
|
||||
// the verdict is a pure function of expression + policy + TriggerCompileSurface, and
|
||||
// it is the exact hot-path cost N1 flagged — every staleness sweep / import probe of
|
||||
// an Expression-triggered config re-ran a full trust compilation + script compile.
|
||||
var (_, error) = ScriptCompileVerdictCache.GetOrAdd(nameof(TriggerCompileSurface), expression, () =>
|
||||
{
|
||||
// Authoritative forbidden-API verdict first. Report ALL violations, not
|
||||
// just the first, so an operator fixing one forbidden API isn't surprised
|
||||
// by a second on the next deploy attempt (mirrors ScriptCompiler.TryCompile).
|
||||
var violations = ScriptTrustValidator.FindViolations(expression);
|
||||
if (violations.Count > 0)
|
||||
return (false, $"uses forbidden API: {string.Join("; ", violations)}");
|
||||
|
||||
// Real compile of the bare boolean expression against the trigger globals.
|
||||
var errors = RoslynScriptCompiler.Compile(expression, typeof(TriggerCompileSurface));
|
||||
if (errors.Count > 0)
|
||||
return $"is not a valid expression: {string.Join("; ", errors)}";
|
||||
// Real compile of the bare boolean expression against the trigger globals.
|
||||
var errors = RoslynScriptCompiler.Compile(expression, typeof(TriggerCompileSurface));
|
||||
if (errors.Count > 0)
|
||||
return (false, $"is not a valid expression: {string.Join("; ", errors)}");
|
||||
|
||||
return null;
|
||||
return (true, (string?)null);
|
||||
});
|
||||
return error;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -149,6 +149,29 @@ public class ScriptCompilerTests
|
||||
Assert.Contains("forbidden", error, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CheckExpressionSyntax_SameExpressionTwice_SecondCallIsCacheHit()
|
||||
{
|
||||
ScriptCompileVerdictCache.Clear();
|
||||
ValidationService.CheckExpressionSyntax("Attributes[\"Temp\"] != null");
|
||||
var hitsBefore = ScriptCompileVerdictCache.Hits;
|
||||
var error = ValidationService.CheckExpressionSyntax("Attributes[\"Temp\"] != null");
|
||||
Assert.Null(error);
|
||||
Assert.Equal(hitsBefore + 1, ScriptCompileVerdictCache.Hits); // FAILS today: no cache use
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CheckExpressionSyntax_ScriptSurfaceVerdict_NotReusedForTriggerSurface()
|
||||
{
|
||||
// "Notify != null" resolves on ScriptCompileSurface (Notify is a script global,
|
||||
// ScriptCompileSurface.cs:53) but NOT on TriggerCompileSurface — a shared
|
||||
// code-only cache entry would wrongly report the trigger expression clean.
|
||||
ScriptCompileVerdictCache.Clear();
|
||||
Assert.True(new ScriptCompiler().TryCompile("Notify != null", "S").IsSuccess); // warms the SCRIPT-surface entry
|
||||
var error = ValidationService.CheckExpressionSyntax("Notify != null");
|
||||
Assert.NotNull(error); // green today (no cache), and MUST STAY green after T3 — the T2 key makes it structural
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void CheckExpressionSyntax_MultipleForbiddenApis_ReportsAll()
|
||||
{
|
||||
|
||||
Reference in New Issue
Block a user