fix(templateengine): audit projected template/script summaries instead of the tracked graph and script bodies (review 2026-10-21 08/G1)
This commit is contained in:
@@ -332,6 +332,26 @@ The same validation logic is available to Design users in the Central UI without
|
||||
|
||||
Gating against the instance surface is a design decision worth knowing: a shared script executes inline inside the calling script's context, so it is type-checked as if it were an instance script. A shared script that references an identifier the instance surface does not expose fails at save, and one that uses an instance accessor (`Attributes`, `Instance`, …) compiles — whether that accessor is meaningful at run time depends on the caller.
|
||||
|
||||
## Configuration Audit Shape
|
||||
|
||||
`IAuditService.LogAsync` serialises the after-state it is handed into the
|
||||
indefinitely-retained `AuditLogEntries.AfterStateJson`. Template Engine services
|
||||
therefore hand it a **projected summary** for aggregates, never a tracked entity
|
||||
graph (review 2026-10-21 08/G1, `TemplateAuditShapes`):
|
||||
|
||||
- **Template** create / update / move — identity and scalar fields (`Id`, `Name`,
|
||||
`Description`, `ParentTemplateId`, `FolderId`, `IsDerived`, `OwnerCompositionId`),
|
||||
per-collection member counts, and `ChangedFields` (e.g. `Name`, `Description`,
|
||||
`FolderId`). No member rows. A rename of a 40-script template used to write all 40
|
||||
script bodies.
|
||||
- **Template script** and **shared script** create / update — every scalar field
|
||||
except the body, which is recorded as `CodeLength` + `CodeSha256`, so an auditor can
|
||||
tell whether (and to what) a body changed without the table storing every revision
|
||||
of every body.
|
||||
|
||||
Small leaf entities (attributes, alarms, compositions, folders, areas, instance
|
||||
overrides) are still audited as-is.
|
||||
|
||||
## Dependencies
|
||||
|
||||
- **Configuration Database (MS SQL)**: Stores all templates, instances, areas, and their relationships.
|
||||
|
||||
@@ -78,7 +78,7 @@ public class SharedScriptService
|
||||
// SaveChangesAsync ran.
|
||||
await _repository.AddSharedScriptAsync(script, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
await _auditService.LogAsync(user, "Create", "SharedScript", script.Id.ToString(), name, script, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Create", "SharedScript", script.Id.ToString(), name, TemplateAuditShapes.SharedScript(script), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
return Result<SharedScript>.Success(script);
|
||||
@@ -123,7 +123,7 @@ public class SharedScriptService
|
||||
script.ReturnDefinition = returnDefinition;
|
||||
|
||||
await _repository.UpdateSharedScriptAsync(script, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Update", "SharedScript", scriptId.ToString(), script.Name, script, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Update", "SharedScript", scriptId.ToString(), script.Name, TemplateAuditShapes.SharedScript(script), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
return Result<SharedScript>.Success(script);
|
||||
|
||||
@@ -0,0 +1,78 @@
|
||||
using System.Security.Cryptography;
|
||||
using System.Text;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.TemplateEngine;
|
||||
|
||||
/// <summary>
|
||||
/// Compact after-state shapes for the configuration audit log (review 2026-10-21
|
||||
/// 08/G1). <c>IAuditService.LogAsync</c> serialises whatever it is handed into
|
||||
/// <c>AuditLogEntries.AfterStateJson</c> — an indefinitely-retained table — so
|
||||
/// handing it a tracked <see cref="Template"/> wrote the whole loaded graph,
|
||||
/// every attribute, alarm and script body included, on each create / rename / move.
|
||||
/// These shapes keep the identity, the scalar fields, per-collection member counts
|
||||
/// and the fields that changed. Script bodies are never written: a body is
|
||||
/// represented by its length and SHA-256, which still lets an auditor tell whether
|
||||
/// (and to what) a body changed without storing it again on every edit.
|
||||
/// </summary>
|
||||
internal static class TemplateAuditShapes
|
||||
{
|
||||
/// <summary>Summary of a template: identity, scalars and member counts — no member rows.</summary>
|
||||
/// <param name="template">The template.</param>
|
||||
/// <param name="changedFields">Names of the fields this operation changed, if known.</param>
|
||||
/// <returns>The audit after-state object.</returns>
|
||||
internal static object Template(Template template, IReadOnlyList<string>? changedFields = null) => new
|
||||
{
|
||||
template.Id,
|
||||
template.Name,
|
||||
template.Description,
|
||||
template.ParentTemplateId,
|
||||
template.FolderId,
|
||||
template.IsDerived,
|
||||
template.OwnerCompositionId,
|
||||
AttributeCount = template.Attributes.Count,
|
||||
AlarmCount = template.Alarms.Count,
|
||||
ScriptCount = template.Scripts.Count,
|
||||
CompositionCount = template.Compositions.Count,
|
||||
NativeAlarmSourceCount = template.NativeAlarmSources.Count,
|
||||
ChangedFields = changedFields,
|
||||
};
|
||||
|
||||
/// <summary>A template script without its body (length + SHA-256 instead).</summary>
|
||||
/// <param name="script">The script.</param>
|
||||
/// <returns>The audit after-state object.</returns>
|
||||
internal static object Script(TemplateScript script) => new
|
||||
{
|
||||
script.Id,
|
||||
script.TemplateId,
|
||||
script.Name,
|
||||
script.IsLocked,
|
||||
script.LockedInDerived,
|
||||
script.IsInherited,
|
||||
script.TriggerType,
|
||||
script.TriggerConfiguration,
|
||||
script.ParameterDefinitions,
|
||||
script.ReturnDefinition,
|
||||
script.MinTimeBetweenRuns,
|
||||
script.ExecutionTimeoutSeconds,
|
||||
CodeLength = script.Code?.Length ?? 0,
|
||||
CodeSha256 = HashCode(script.Code),
|
||||
};
|
||||
|
||||
/// <summary>A shared script without its body (length + SHA-256 instead).</summary>
|
||||
/// <param name="script">The shared script.</param>
|
||||
/// <returns>The audit after-state object.</returns>
|
||||
internal static object SharedScript(SharedScript script) => new
|
||||
{
|
||||
script.Id,
|
||||
script.Name,
|
||||
script.ParameterDefinitions,
|
||||
script.ReturnDefinition,
|
||||
CodeLength = script.Code?.Length ?? 0,
|
||||
CodeSha256 = HashCode(script.Code),
|
||||
};
|
||||
|
||||
private static string? HashCode(string? code) =>
|
||||
code == null ? null : Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(code)));
|
||||
}
|
||||
@@ -84,7 +84,7 @@ public class TemplateService
|
||||
// to the row it described.
|
||||
await _repository.AddTemplateAsync(template, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
await _auditService.LogAsync(user, "Create", "Template", template.Id.ToString(), name, template, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Create", "Template", template.Id.ToString(), name, TemplateAuditShapes.Template(template), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
return Result<Template>.Success(template);
|
||||
@@ -130,6 +130,12 @@ public class TemplateService
|
||||
"Parent template cannot be changed after creation.");
|
||||
}
|
||||
|
||||
var changedFields = new List<string>();
|
||||
if (!string.Equals(template.Name, name, StringComparison.Ordinal))
|
||||
changedFields.Add(nameof(Template.Name));
|
||||
if (description != null && !string.Equals(template.Description, description, StringComparison.Ordinal))
|
||||
changedFields.Add(nameof(Template.Description));
|
||||
|
||||
template.Name = name;
|
||||
// Leave-unchanged semantics: only overwrite the description when a value is
|
||||
// supplied. null = not provided (keep existing); "" = explicit clear.
|
||||
@@ -142,7 +148,10 @@ public class TemplateService
|
||||
return Result<Template>.Failure(collisionResult);
|
||||
|
||||
await _repository.UpdateTemplateAsync(template, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Update", "Template", templateId.ToString(), name, template, cancellationToken);
|
||||
// Audit a projected summary, never the tracked graph with every script body
|
||||
// (review 2026-10-21 08/G1).
|
||||
await _auditService.LogAsync(user, "Update", "Template", templateId.ToString(), name,
|
||||
TemplateAuditShapes.Template(template, changedFields), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
return Result<Template>.Success(template);
|
||||
@@ -234,7 +243,8 @@ public class TemplateService
|
||||
|
||||
template.FolderId = newFolderId;
|
||||
await _repository.UpdateTemplateAsync(template, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Move", "Template", template.Id.ToString(), template.Name, template, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Move", "Template", template.Id.ToString(), template.Name,
|
||||
TemplateAuditShapes.Template(template, [nameof(Template.FolderId)]), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
return Result<Template>.Success(template);
|
||||
@@ -723,7 +733,7 @@ public class TemplateService
|
||||
// real script Id rather than a literal "0".
|
||||
await _repository.AddTemplateScriptAsync(script, cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
await _auditService.LogAsync(user, "Create", "TemplateScript", script.Id.ToString(), script.Name, script, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Create", "TemplateScript", script.Id.ToString(), script.Name, TemplateAuditShapes.Script(script), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
// Propagate the new member to derived descendants.
|
||||
@@ -809,7 +819,7 @@ public class TemplateService
|
||||
// Name is NOT updated (fixed)
|
||||
|
||||
await _repository.UpdateTemplateScriptAsync(existing, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Update", "TemplateScript", scriptId.ToString(), existing.Name, existing, cancellationToken);
|
||||
await _auditService.LogAsync(user, "Update", "TemplateScript", scriptId.ToString(), existing.Name, TemplateAuditShapes.Script(existing), cancellationToken);
|
||||
await _repository.SaveChangesAsync(cancellationToken);
|
||||
|
||||
// Propagate the changed effective value to derived descendants.
|
||||
|
||||
@@ -0,0 +1,117 @@
|
||||
using System.Text.Json;
|
||||
using Moq;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Review 2026-10-21 08/G1: the configuration audit (indefinitely retained) must
|
||||
/// record a projected summary of a template — ids, names, member counts, changed
|
||||
/// fields — never the whole tracked graph with every script body.
|
||||
/// </summary>
|
||||
public class TemplateAuditShapeTests
|
||||
{
|
||||
private const string Body = "return \"BODY_SENTINEL_7f3a\";";
|
||||
|
||||
private readonly Mock<ITemplateEngineRepository> _repoMock = new();
|
||||
private readonly Mock<IAuditService> _auditMock = new();
|
||||
private readonly List<object?> _afterStates = [];
|
||||
|
||||
/// <summary>Captures every audit after-state the service writes.</summary>
|
||||
public TemplateAuditShapeTests()
|
||||
{
|
||||
_repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>())).ReturnsAsync(1);
|
||||
_auditMock
|
||||
.Setup(a => a.LogAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(),
|
||||
It.IsAny<string>(), It.IsAny<string>(), It.IsAny<object?>(), It.IsAny<CancellationToken>()))
|
||||
.Callback<string, string, string, string, string, object?, CancellationToken>(
|
||||
(_, _, _, _, _, after, _) => _afterStates.Add(after))
|
||||
.Returns(Task.CompletedTask);
|
||||
}
|
||||
|
||||
private Template SeedTemplateWithScripts()
|
||||
{
|
||||
var template = new Template("Pump") { Id = 1 };
|
||||
template.Scripts.Add(new TemplateScript("Calc", Body) { Id = 10, TemplateId = 1 });
|
||||
template.Scripts.Add(new TemplateScript("Calc2", Body + "//2") { Id = 11, TemplateId = 1 });
|
||||
template.Attributes.Add(new TemplateAttribute("Temp") { Id = 20, TemplateId = 1 });
|
||||
_repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { template });
|
||||
return template;
|
||||
}
|
||||
|
||||
private string SingleAfterStateJson()
|
||||
{
|
||||
var after = Assert.Single(_afterStates);
|
||||
return JsonSerializer.Serialize(after);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a template rename audits a summary without script bodies.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task UpdateTemplate_AuditsSummary_WithoutScriptBodies()
|
||||
{
|
||||
SeedTemplateWithScripts();
|
||||
var sut = new TemplateService(_repoMock.Object, _auditMock.Object);
|
||||
|
||||
var result = await sut.UpdateTemplateAsync(1, "Pump2", null, null, "admin");
|
||||
|
||||
Assert.True(result.IsSuccess);
|
||||
var json = SingleAfterStateJson();
|
||||
Assert.DoesNotContain("BODY_SENTINEL", json);
|
||||
using var doc = JsonDocument.Parse(json);
|
||||
Assert.Equal("Pump2", doc.RootElement.GetProperty("Name").GetString());
|
||||
Assert.Equal(2, doc.RootElement.GetProperty("ScriptCount").GetInt32());
|
||||
Assert.Equal(1, doc.RootElement.GetProperty("AttributeCount").GetInt32());
|
||||
Assert.Contains("Name", doc.RootElement.GetProperty("ChangedFields").EnumerateArray().Select(e => e.GetString()));
|
||||
Assert.False(doc.RootElement.TryGetProperty("Scripts", out _));
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a template move audits a summary without script bodies.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task MoveTemplate_AuditsSummary_WithoutScriptBodies()
|
||||
{
|
||||
SeedTemplateWithScripts();
|
||||
var sut = new TemplateService(_repoMock.Object, _auditMock.Object);
|
||||
|
||||
var result = await sut.MoveTemplateAsync(1, null, "admin");
|
||||
|
||||
Assert.True(result.IsSuccess);
|
||||
Assert.DoesNotContain("BODY_SENTINEL", SingleAfterStateJson());
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a script add audits the script's metadata and body hash, not its body.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task AddScript_AuditsBodyHash_NotBody()
|
||||
{
|
||||
SeedTemplateWithScripts();
|
||||
var sut = new TemplateService(_repoMock.Object, _auditMock.Object);
|
||||
|
||||
var result = await sut.AddScriptAsync(1, new TemplateScript("Calc3", Body + "//3"), "admin");
|
||||
|
||||
Assert.True(result.IsSuccess);
|
||||
var json = SingleAfterStateJson();
|
||||
Assert.DoesNotContain("BODY_SENTINEL", json);
|
||||
using var doc = JsonDocument.Parse(json);
|
||||
Assert.Equal("Calc3", doc.RootElement.GetProperty("Name").GetString());
|
||||
Assert.Equal(64, doc.RootElement.GetProperty("CodeSha256").GetString()!.Length);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a shared-script create audits its body hash, not its body.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
public async Task CreateSharedScript_AuditsBodyHash_NotBody()
|
||||
{
|
||||
var sut = new SharedScriptService(_repoMock.Object, _auditMock.Object);
|
||||
|
||||
var result = await sut.CreateSharedScriptAsync("Helpers", Body, null, null, "admin");
|
||||
|
||||
Assert.True(result.IsSuccess, result.IsFailure ? result.Error : null);
|
||||
Assert.DoesNotContain("BODY_SENTINEL", SingleAfterStateJson());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user