perf(templates): member add/update loads only the chain + composition closure (ScadaBridge#66)

Attribute/alarm/script add (collision check) and update (parent-lock lookup)
used GetAllTemplatesForAnalysisAsync, materialising every template, attribute,
alarm and script in the library per edit. They now call the new
ITemplateEngineRepository.GetTemplateAnalysisClosureAsync: the same untracked,
body-free analysis projection, restricted to the template's inheritance chain
plus the composition closure of every chain member - exactly what
TemplateResolver and CollisionDetector walk (a composed module's own parent
chain is never climbed, so it is not loaded). Adds use the edited template's
closure, updates the parent's.

The walk (Commons TemplateAnalysisClosure) loads one batch per closure depth,
requests each id once, terminates on cycles and skips dangling edges.

Evidence:
- TemplateAnalysisClosureTests: for every template of a graph with chains,
  nested + diamond compositions, a derived module, cycles, dangling edges and
  colliding unrelated templates, ResolveAllMembers / DetectCollisions /
  FindMemberByCanonicalName answer identically from the closure and the whole
  graph (mutation-checked).
- TemplateAnalysisClosureRepositoryTests (SQLite, recorded SQL): 6 templates /
  12 attributes loaded instead of 47 / 94, in 4 queries (one per depth),
  untracked, script bodies never selected.
- TemplateServiceTests: two new tests with the whole-graph load wired to throw
  (fail on the old service); every existing add/update test now runs against
  the closure via AnalysisClosureMock.

ReconcileDescendantsAsync still loads the tracked full graph; not in scope.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
2026-11-09 12:52:29 -05:00
parent d36825633b
commit 792f16c2d5
9 changed files with 581 additions and 10 deletions
@@ -56,6 +56,19 @@ public interface ITemplateEngineRepository
/// <returns>A task that resolves to a read-only list of all templates without script bodies.</returns> /// <returns>A task that resolves to a read-only list of all templates without script bodies.</returns>
Task<IReadOnlyList<Template>> GetAllTemplatesForAnalysisAsync(CancellationToken cancellationToken = default); Task<IReadOnlyList<Template>> GetAllTemplatesForAnalysisAsync(CancellationToken cancellationToken = default);
/// <summary> /// <summary>
/// Returns the same untracked, script-body-free projection as
/// <see cref="GetAllTemplatesForAnalysisAsync"/>, but only for the templates an analysis walk
/// rooted at <paramref name="templateId"/> can reach: its inheritance chain plus the
/// composition closure of every chain member (see
/// <see cref="ZB.MOM.WW.ScadaBridge.Commons.Types.Templates.TemplateAnalysisClosure"/>).
/// Backs member add/update validation (Gitea #66), which previously loaded every template.
/// Unknown ids yield an empty list.
/// </summary>
/// <param name="templateId">The template whose chain and composition closure to load.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task that resolves to the closure templates without script bodies.</returns>
Task<IReadOnlyList<Template>> GetTemplateAnalysisClosureAsync(int templateId, CancellationToken cancellationToken = default);
/// <summary>
/// Returns an untracked identity/edge projection of every template (no children), ordered by name. /// Returns an untracked identity/edge projection of every template (no children), ordered by name.
/// </summary> /// </summary>
/// <param name="cancellationToken">Cancellation token.</param> /// <param name="cancellationToken">Cancellation token.</param>
@@ -0,0 +1,113 @@
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
namespace ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;
/// <summary>
/// Computes the set of templates a member add/update analysis walk can reach from one template
/// (Gitea #66): the template's <b>inheritance chain</b> (itself, its parent, grandparent, …) plus
/// the <b>composition closure</b> of every chain member (each composed module, the modules they
/// compose, and so on).
///
/// <para>
/// This mirrors exactly what the analysis walkers read. <c>TemplateResolver.ResolveAllMembers</c>
/// and <c>CollisionDetector.DetectCollisions</c> climb the parent chain of the template being
/// analysed and descend composition edges from every chain member, but never climb a composed
/// module's own parent chain (a derived module carries its inherited members as placeholder rows).
/// So a composed module's ancestors are not loaded — only chain members expand their parent edge.
/// Every other template in the library is irrelevant to the result, which is why member edits no
/// longer need the whole analysis graph.
/// </para>
///
/// <para>
/// The walk is breadth-first by batch: each round asks <c>loadBatch</c> for every id not yet
/// loaded, so a store-backed loader issues one query per closure depth rather than one per
/// template. Missing ids (a dangling edge) are simply absent from the result, exactly as the
/// walkers skip them. Cycles terminate: an id is expanded at most twice (once as a module, once
/// more if it is later reached as a chain member).
/// </para>
/// </summary>
public static class TemplateAnalysisClosure
{
/// <summary>
/// Collects the inheritance chain and composition closure of <paramref name="templateId"/>.
/// </summary>
/// <param name="templateId">The template whose analysis closure is wanted.</param>
/// <param name="loadBatch">Loads the templates with the given ids (unknown ids are omitted).</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>The closure templates, each once, in load order (the root first when it exists).</returns>
public static async Task<IReadOnlyList<Template>> CollectAsync(
int templateId,
Func<IReadOnlyCollection<int>, CancellationToken, Task<IReadOnlyList<Template>>> loadBatch,
CancellationToken cancellationToken = default)
{
ArgumentNullException.ThrowIfNull(loadBatch);
var loaded = new Dictionary<int, Template>();
var result = new List<Template>();
var requested = new HashSet<int>();
// id -> true when expanded as a chain member (parent + compositions),
// false when expanded as a module only (compositions).
var expanded = new Dictionary<int, bool>();
// Ids reached but not loaded yet, with "reached as chain member" OR'd in.
var wanted = new Dictionary<int, bool> { [templateId] = true };
while (wanted.Count > 0)
{
cancellationToken.ThrowIfCancellationRequested();
var toLoad = wanted.Keys.Where(requested.Add).ToArray();
if (toLoad.Length > 0)
{
var batch = await loadBatch(toLoad, cancellationToken).ConfigureAwait(false);
foreach (var t in batch)
{
if (loaded.TryAdd(t.Id, t))
result.Add(t);
}
}
var round = wanted.ToArray();
wanted.Clear();
foreach (var (id, asChain) in round)
Expand(id, asChain, loaded, expanded, wanted, requested);
}
return result;
}
private static void Expand(
int rootId,
bool rootAsChain,
Dictionary<int, Template> loaded,
Dictionary<int, bool> expanded,
Dictionary<int, bool> wanted,
HashSet<int> requested)
{
var stack = new Stack<(int Id, bool AsChain)>();
stack.Push((rootId, rootAsChain));
while (stack.Count > 0)
{
var (id, asChain) = stack.Pop();
if (!loaded.TryGetValue(id, out var template))
{
// Already asked the loader and it does not exist: a dangling edge, skip it.
if (requested.Contains(id))
continue;
wanted[id] = asChain || (wanted.TryGetValue(id, out var w) && w);
continue;
}
if (expanded.TryGetValue(id, out var wasChain) && (wasChain || !asChain))
continue;
expanded[id] = asChain;
if (asChain && template.ParentTemplateId is int parentId)
stack.Push((parentId, true));
foreach (var composition in template.Compositions)
stack.Push((composition.ComposedTemplateId, false));
}
}
}
@@ -122,8 +122,33 @@ public class TemplateEngineRepository : ITemplateEngineRepository
// <see cref="GetAllTemplatesAsync"/> / <see cref="GetTemplateWithChildrenAsync"/>. // <see cref="GetAllTemplatesAsync"/> / <see cref="GetTemplateWithChildrenAsync"/>.
public async Task<IReadOnlyList<Template>> GetAllTemplatesForAnalysisAsync(CancellationToken cancellationToken = default) public async Task<IReadOnlyList<Template>> GetAllTemplatesForAnalysisAsync(CancellationToken cancellationToken = default)
{ {
return await _context.Templates return await AnalysisProjection(_context.Templates.AsNoTracking())
.AsNoTracking() .ToListAsync(cancellationToken);
}
/// <inheritdoc />
// One projection query per closure depth (TemplateAnalysisClosure batches each frontier),
// each restricted to the frontier's ids — so a member edit reads its own chain and modules
// instead of every template, attribute, alarm and script in the library.
public Task<IReadOnlyList<Template>> GetTemplateAnalysisClosureAsync(int templateId, CancellationToken cancellationToken = default)
{
return TemplateAnalysisClosure.CollectAsync(
templateId,
async (ids, ct) =>
{
var idList = ids.ToList();
return await AnalysisProjection(_context.Templates.AsNoTracking().Where(t => idList.Contains(t.Id)))
.ToListAsync(ct);
},
cancellationToken);
}
// The analysis projection shared by GetAllTemplatesForAnalysisAsync and
// GetTemplateAnalysisClosureAsync (see the note on the former for why it is untracked and
// drops TemplateScript.Code).
private static IQueryable<Template> AnalysisProjection(IQueryable<Template> source)
{
return source
.Select(t => new Template(t.Name) .Select(t => new Template(t.Name)
{ {
Id = t.Id, Id = t.Id,
@@ -152,8 +177,7 @@ public class TemplateEngineRepository : ITemplateEngineRepository
LockedInDerived = s.LockedInDerived LockedInDerived = s.LockedInDerived
}) })
.ToList() .ToList()
}) });
.ToListAsync(cancellationToken);
} }
/// <inheritdoc /> /// <inheritdoc />
@@ -311,7 +311,8 @@ public class TemplateService
attribute.TemplateId = templateId; attribute.TemplateId = templateId;
// If inheriting, validate not trying to add a member that would collide // If inheriting, validate not trying to add a member that would collide
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken); // Only the template's inheritance chain + composition closure can collide (#66).
var allTemplates = await _repository.GetTemplateAnalysisClosureAsync(templateId, cancellationToken);
var testTemplate = CloneTemplateWithNewAttribute(template, attribute); var testTemplate = CloneTemplateWithNewAttribute(template, attribute);
var collisions = CollisionDetector.DetectCollisions(testTemplate, allTemplates); var collisions = CollisionDetector.DetectCollisions(testTemplate, allTemplates);
if (collisions.Count > 0) if (collisions.Count > 0)
@@ -363,7 +364,8 @@ public class TemplateService
var template = await _repository.GetTemplateByIdAsync(existing.TemplateId, cancellationToken); var template = await _repository.GetTemplateByIdAsync(existing.TemplateId, cancellationToken);
if (template?.ParentTemplateId != null) if (template?.ParentTemplateId != null)
{ {
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken); // The parent's members resolve from its chain + composition closure alone (#66).
var allTemplates = await _repository.GetTemplateAnalysisClosureAsync(template.ParentTemplateId.Value, cancellationToken);
var parentMember = TemplateResolver.FindMemberByCanonicalName( var parentMember = TemplateResolver.FindMemberByCanonicalName(
existing.Name, template.ParentTemplateId.Value, allTemplates); existing.Name, template.ParentTemplateId.Value, allTemplates);
if (parentMember != null && parentMember.IsLocked) if (parentMember != null && parentMember.IsLocked)
@@ -548,7 +550,8 @@ public class TemplateService
alarm.TemplateId = templateId; alarm.TemplateId = templateId;
// Check collisions // Check collisions
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken); // Only the template's inheritance chain + composition closure can collide (#66).
var allTemplates = await _repository.GetTemplateAnalysisClosureAsync(templateId, cancellationToken);
var testTemplate = CloneTemplateWithNewAlarm(template, alarm); var testTemplate = CloneTemplateWithNewAlarm(template, alarm);
var collisions = CollisionDetector.DetectCollisions(testTemplate, allTemplates); var collisions = CollisionDetector.DetectCollisions(testTemplate, allTemplates);
if (collisions.Count > 0) if (collisions.Count > 0)
@@ -598,7 +601,8 @@ public class TemplateService
var template = await _repository.GetTemplateByIdAsync(existing.TemplateId, cancellationToken); var template = await _repository.GetTemplateByIdAsync(existing.TemplateId, cancellationToken);
if (template?.ParentTemplateId != null) if (template?.ParentTemplateId != null)
{ {
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken); // The parent's members resolve from its chain + composition closure alone (#66).
var allTemplates = await _repository.GetTemplateAnalysisClosureAsync(template.ParentTemplateId.Value, cancellationToken);
var parentMember = TemplateResolver.FindMemberByCanonicalName( var parentMember = TemplateResolver.FindMemberByCanonicalName(
existing.Name, template.ParentTemplateId.Value, allTemplates); existing.Name, template.ParentTemplateId.Value, allTemplates);
if (parentMember != null && parentMember.IsLocked) if (parentMember != null && parentMember.IsLocked)
@@ -723,7 +727,8 @@ public class TemplateService
script.TemplateId = templateId; script.TemplateId = templateId;
// Check collisions // Check collisions
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken); // Only the template's inheritance chain + composition closure can collide (#66).
var allTemplates = await _repository.GetTemplateAnalysisClosureAsync(templateId, cancellationToken);
var testTemplate = CloneTemplateWithNewScript(template, script); var testTemplate = CloneTemplateWithNewScript(template, script);
var collisions = CollisionDetector.DetectCollisions(testTemplate, allTemplates); var collisions = CollisionDetector.DetectCollisions(testTemplate, allTemplates);
if (collisions.Count > 0) if (collisions.Count > 0)
@@ -770,7 +775,8 @@ public class TemplateService
var template = await _repository.GetTemplateByIdAsync(existing.TemplateId, cancellationToken); var template = await _repository.GetTemplateByIdAsync(existing.TemplateId, cancellationToken);
if (template?.ParentTemplateId != null) if (template?.ParentTemplateId != null)
{ {
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken); // The parent's members resolve from its chain + composition closure alone (#66).
var allTemplates = await _repository.GetTemplateAnalysisClosureAsync(template.ParentTemplateId.Value, cancellationToken);
var parentMember = TemplateResolver.FindMemberByCanonicalName( var parentMember = TemplateResolver.FindMemberByCanonicalName(
existing.Name, template.ParentTemplateId.Value, allTemplates); existing.Name, template.ParentTemplateId.Value, allTemplates);
if (parentMember != null && parentMember.IsLocked) if (parentMember != null && parentMember.IsLocked)
@@ -0,0 +1,126 @@
using Microsoft.EntityFrameworkCore;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;
using ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Repositories;
namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests;
/// <summary>
/// Gitea #66: <see cref="TemplateEngineRepository.GetTemplateAnalysisClosureAsync"/> reads only a
/// template's inheritance chain + composition closure — one projection query per closure depth —
/// instead of the whole library that <see cref="TemplateEngineRepository.GetAllTemplatesForAnalysisAsync"/>
/// materialises. Asserted against real SQL on SQLite with the reader commands recorded.
/// </summary>
public class TemplateAnalysisClosureRepositoryTests : IDisposable
{
private const int UnrelatedTemplates = 40;
private const string BigBody = "/* script body that must never be read by analysis */";
private readonly SqlReaderCommandRecorder _recorder = new();
private readonly ScadaBridgeDbContext _context;
private readonly TemplateEngineRepository _repository;
/// <summary>Initializes the test class with an in-memory context and repository.</summary>
public TemplateAnalysisClosureRepositoryTests()
{
_context = SqliteTestHelper.CreateInMemoryContext(_recorder);
_repository = new TemplateEngineRepository(_context, new TemplateGraphWatermark());
}
/// <summary>Releases the resources held by this test class.</summary>
public void Dispose()
{
_context.Database.CloseConnection();
_context.Dispose();
}
private async Task<Template> AddAsync(string name, int? parentId = null)
{
var t = new Template(name) { ParentTemplateId = parentId };
t.Attributes.Add(new TemplateAttribute($"{name}Attr1"));
t.Attributes.Add(new TemplateAttribute($"{name}Attr2"));
t.Scripts.Add(new TemplateScript($"{name}Script", BigBody));
_context.Templates.Add(t);
await _context.SaveChangesAsync();
return t;
}
private async Task ComposeAsync(Template owner, Template module, string slot)
{
_context.Set<TemplateComposition>().Add(
new TemplateComposition(slot) { TemplateId = owner.Id, ComposedTemplateId = module.Id });
await _context.SaveChangesAsync();
}
private sealed record Seeded(int Grand, int Parent, int Child, int ModA, int ModB, int ModBBase, int Inner);
private async Task<Seeded> SeedAsync()
{
var grand = await AddAsync("Grand");
var parent = await AddAsync("Parent", grand.Id);
var child = await AddAsync("Child", parent.Id);
var modBBase = await AddAsync("ModBBase");
var modB = await AddAsync("ModB", modBBase.Id);
var modA = await AddAsync("ModA");
var inner = await AddAsync("Inner");
await ComposeAsync(parent, modA, "m1");
await ComposeAsync(child, modB, "m2");
await ComposeAsync(modA, inner, "inner");
for (var i = 0; i < UnrelatedTemplates; i++)
await AddAsync($"Unrelated{i}");
_context.ChangeTracker.Clear();
_recorder.Reset();
return new Seeded(grand.Id, parent.Id, child.Id, modA.Id, modB.Id, modBBase.Id, inner.Id);
}
/// <summary>
/// The closure is exactly the chain + composition closure, loaded untracked without script
/// bodies, in one query per depth — against 47 templates for the whole-graph load.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Closure_ReadsOnlyChainAndCompositionClosure_OneQueryPerDepth()
{
var ids = await SeedAsync();
var whole = await _repository.GetAllTemplatesForAnalysisAsync();
var wholeQueries = _recorder.Commands.Count;
_recorder.Reset();
var closure = await _repository.GetTemplateAnalysisClosureAsync(ids.Child);
var closureQueries = _recorder.Commands.Count;
// Entity count: 6 templates / 12 attributes instead of 47 / 94.
Assert.Equal(
new[] { ids.Grand, ids.Parent, ids.Child, ids.ModA, ids.ModB, ids.Inner }.OrderBy(i => i),
closure.Select(t => t.Id).OrderBy(i => i));
Assert.Equal(7 + UnrelatedTemplates, whole.Count);
Assert.Equal(12, closure.Sum(t => t.Attributes.Count));
Assert.Equal(2 * (7 + UnrelatedTemplates), whole.Sum(t => t.Attributes.Count));
// Query count: one projection query per depth (Child; Parent+ModB; Grand+ModA; Inner).
Assert.Equal(4 * wholeQueries, closureQueries);
// Same analysis shape as the whole-graph load: untracked, no script bodies, edges present.
Assert.Empty(_context.ChangeTracker.Entries());
Assert.All(closure.SelectMany(t => t.Scripts), s => Assert.Equal(string.Empty, s.Code));
Assert.False(_recorder.AnySelectReads("\"Code\""));
Assert.Single(closure.Single(t => t.Id == ids.Child).Compositions);
// Positive control: the body column IS visible to the recorder when a load reads it.
_recorder.Reset();
await _repository.GetAllTemplatesAsync();
Assert.True(_recorder.AnySelectReads("\"Code\""));
}
/// <summary>An unknown id loads nothing beyond the single probe query.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Closure_UnknownTemplate_IsEmpty()
{
await SeedAsync();
Assert.Empty(await _repository.GetTemplateAnalysisClosureAsync(987654));
}
}
@@ -0,0 +1,40 @@
using Moq;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;
namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests;
/// <summary>
/// Gitea #66: member add/update now asks the repository for a template's analysis closure
/// rather than the whole analysis graph. Test classes keep seeding the whole graph through
/// <c>GetAllTemplatesForAnalysisAsync</c>; this wires <c>GetTemplateAnalysisClosureAsync</c>
/// to serve the real closure walk over that seeded graph, so every existing scenario runs the
/// service against the reduced set — the same set the production repository returns.
/// </summary>
internal static class AnalysisClosureMock
{
/// <summary>Wires the closure method to filter whatever the full-graph setup returns.</summary>
/// <param name="repoMock">The repository mock.</param>
/// <param name="closureRequests">Optional sink recording each requested closure root.</param>
public static void ServeClosureFromFullGraph(
this Mock<ITemplateEngineRepository> repoMock,
List<int>? closureRequests = null)
{
repoMock
.Setup(r => r.GetTemplateAnalysisClosureAsync(It.IsAny<int>(), It.IsAny<CancellationToken>()))
.Returns((int templateId, CancellationToken ct) =>
{
closureRequests?.Add(templateId);
return TemplateAnalysisClosure.CollectAsync(
templateId,
async (ids, token) =>
{
var all = await repoMock.Object.GetAllTemplatesForAnalysisAsync(token)
?? Array.Empty<Template>();
return all.Where(t => ids.Contains(t.Id)).ToList();
},
ct);
});
}
}
@@ -0,0 +1,162 @@
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;
namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests;
/// <summary>
/// Gitea #66: member add/update validation now runs the analysis walkers over a template's
/// inheritance chain + composition closure instead of the whole library. These tests pin that
/// the closure is (a) sufficient — every walker answer is identical to the whole-graph answer,
/// for every template in a graph with chains, nested and diamond compositions, a derived
/// module, cycles, dangling edges and unrelated look-alikes — and (b) actually smaller.
/// </summary>
public class TemplateAnalysisClosureTests
{
// Ids of the fixture graph.
private const int Grand = 1, Parent = 2, Child = 3, ModA = 4, ModB = 5, ModBBase = 6, Inner = 7;
private const int CycX = 8, CycY = 9, Dangling = 10;
private const int Twin = 18, TwinParent = 19, TwinGrand = 20;
private static readonly List<Template> Graph = BuildGraph();
private static List<Template> BuildGraph()
{
var memberId = 1000;
Template T(int id, string name, int? parent = null, params string[] attrs)
{
var t = new Template(name) { Id = id, ParentTemplateId = parent };
foreach (var a in attrs)
t.Attributes.Add(new TemplateAttribute(a) { Id = memberId++, TemplateId = id, IsLocked = a.StartsWith('L') });
return t;
}
void Compose(Template owner, Template module, string slot) =>
owner.Compositions.Add(new TemplateComposition(slot) { Id = memberId++, TemplateId = owner.Id, ComposedTemplateId = module.Id });
var grand = T(Grand, "Grand", null, "LSpeed", "Shared");
var parent = T(Parent, "Parent", Grand, "Flow");
var child = T(Child, "Child", Parent, "Shared", "m1.Level"); // overrides + a would-be collision
var modA = T(ModA, "ModA", null, "Level", "Temp");
var modB = T(ModB, "ModB", ModBBase, "Base1"); // derived module (placeholder row)
var modBBase = T(ModBBase, "ModBBase", null, "Base1", "OnlyInBase");
var inner = T(Inner, "Inner", null, "Deep");
modB.Alarms.Add(new TemplateAlarm("HiHi") { Id = memberId++, TemplateId = ModB });
modA.Scripts.Add(new TemplateScript("Calc", string.Empty) { Id = memberId++, TemplateId = ModA, IsLocked = true });
Compose(parent, modA, "m1");
Compose(child, modB, "m2");
Compose(child, modA, "m1b"); // diamond: ModA reached by two paths
Compose(modA, inner, "inner");
var cycX = T(CycX, "CycX", null, "X");
var cycY = T(CycY, "CycY", null, "Y");
Compose(cycX, cycY, "y");
Compose(cycY, cycX, "x"); // composition cycle
var dangling = T(Dangling, "Dangling", 998, "D");
dangling.Compositions.Add(new TemplateComposition("ghost") { Id = memberId++, TemplateId = Dangling, ComposedTemplateId = 999 });
// A template reached first as a module and then as a chain member: its parent must still load.
var twinGrand = T(TwinGrand, "TwinGrand", null, "G");
var twinParent = T(TwinParent, "TwinParent", TwinGrand, "P");
var twin = T(Twin, "Twin", TwinParent, "S");
Compose(twin, twinParent, "t");
var all = new List<Template> { grand, parent, child, modA, modB, modBBase, inner, cycX, cycY, dangling, twinGrand, twinParent, twin };
// Unrelated library content with colliding member names — must never be loaded.
for (var i = 0; i < 5; i++)
all.Add(T(11 + i, $"Unrelated{i}", i == 0 ? null : 11, "Shared", "Level", "m1.Level"));
return all;
}
private static Task<IReadOnlyList<Template>> InMemoryClosure(int id, List<IReadOnlyCollection<int>>? batches = null) =>
TemplateAnalysisClosure.CollectAsync(id, (ids, _) =>
{
batches?.Add(ids);
return Task.FromResult<IReadOnlyList<Template>>(Graph.Where(t => ids.Contains(t.Id)).ToList());
});
public static IEnumerable<object[]> EveryTemplate() => Graph.Select(t => new object[] { t.Id });
/// <summary>Resolver and collision detector give the same answer from the closure as from the whole graph.</summary>
/// <param name="templateId">Template under analysis.</param>
/// <returns>A task that represents the asynchronous operation.</returns>
[Theory]
[MemberData(nameof(EveryTemplate))]
public async Task Walkers_AnswerIdenticallyFromClosure(int templateId)
{
var closure = await InMemoryClosure(templateId);
var template = Graph.Single(t => t.Id == templateId);
Assert.Equal(
TemplateResolver.ResolveAllMembers(templateId, Graph),
TemplateResolver.ResolveAllMembers(templateId, closure));
Assert.Equal(
CollisionDetector.DetectCollisions(template, Graph),
CollisionDetector.DetectCollisions(template, closure));
foreach (var member in TemplateResolver.ResolveAllMembers(templateId, Graph))
{
Assert.Equal(
TemplateResolver.FindMemberByCanonicalName(member.CanonicalName, templateId, Graph),
TemplateResolver.FindMemberByCanonicalName(member.CanonicalName, templateId, closure));
}
}
/// <summary>The fixture really exercises a collision, so the equality above is not vacuous.</summary>
[Fact]
public void Fixture_ChildHasACompositionCollision()
{
var child = Graph.Single(t => t.Id == Child);
Assert.NotEmpty(CollisionDetector.DetectCollisions(child, Graph));
}
/// <summary>The closure is exactly the chain plus composition closure — no module ancestors, no unrelated templates.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Closure_IsChainPlusCompositionClosureOnly()
{
var closure = await InMemoryClosure(Child);
Assert.Equal(
new[] { Grand, Parent, Child, ModA, ModB, Inner }.OrderBy(i => i),
closure.Select(t => t.Id).OrderBy(i => i));
Assert.DoesNotContain(closure, t => t.Id == ModBBase); // a module's parent chain is never walked
Assert.True(closure.Count < Graph.Count);
}
/// <summary>A template reached as a module before it is reached as a chain member still expands its parent.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Closure_ModuleThatIsAlsoAChainMember_StillClimbsItsParent()
{
var closure = await InMemoryClosure(Twin);
Assert.Equal(new[] { Twin, TwinParent, TwinGrand }.OrderBy(i => i), closure.Select(t => t.Id).OrderBy(i => i));
}
/// <summary>Cycles terminate and dangling ids are skipped.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Closure_TerminatesOnCyclesAndSkipsDanglingEdges()
{
Assert.Equal(new[] { CycX, CycY }, (await InMemoryClosure(CycX)).Select(t => t.Id).OrderBy(i => i));
Assert.Equal(new[] { Dangling }, (await InMemoryClosure(Dangling)).Select(t => t.Id));
Assert.Empty(await InMemoryClosure(424242));
}
/// <summary>One batch per closure depth, and no id is ever requested twice.</summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Closure_LoadsOneBatchPerDepth_NeverRequestsAnIdTwice()
{
var batches = new List<IReadOnlyCollection<int>>();
await InMemoryClosure(Child, batches);
// Depth 0: Child; depth 1: Parent, ModB, ModA; depth 2: Grand, Inner.
Assert.Equal(3, batches.Count);
var requested = batches.SelectMany(b => b).ToList();
Assert.Equal(requested.Count, requested.Distinct().Count());
}
}
@@ -23,6 +23,7 @@ public class TemplateAuditShapeTests
public TemplateAuditShapeTests() public TemplateAuditShapeTests()
{ {
_repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>())).ReturnsAsync(1); _repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>())).ReturnsAsync(1);
_repoMock.ServeClosureFromFullGraph();
_auditMock _auditMock
.Setup(a => a.LogAsync(It.IsAny<string>(), It.IsAny<string>(), It.IsAny<string>(), .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>())) It.IsAny<string>(), It.IsAny<string>(), It.IsAny<object?>(), It.IsAny<CancellationToken>()))
@@ -13,6 +13,7 @@ public class TemplateServiceTests
private readonly Mock<ITemplateEngineRepository> _repoMock; private readonly Mock<ITemplateEngineRepository> _repoMock;
private readonly Mock<IAuditService> _auditMock; private readonly Mock<IAuditService> _auditMock;
private readonly TemplateService _service; private readonly TemplateService _service;
private readonly List<int> _closureRequests = new();
/// <summary>Initializes the test with mocked repositories and the service under test.</summary> /// <summary>Initializes the test with mocked repositories and the service under test.</summary>
public TemplateServiceTests() public TemplateServiceTests()
@@ -20,6 +21,7 @@ public class TemplateServiceTests
_repoMock = new Mock<ITemplateEngineRepository>(); _repoMock = new Mock<ITemplateEngineRepository>();
_auditMock = new Mock<IAuditService>(); _auditMock = new Mock<IAuditService>();
_repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>())).ReturnsAsync(1); _repoMock.Setup(r => r.SaveChangesAsync(It.IsAny<CancellationToken>())).ReturnsAsync(1);
_repoMock.ServeClosureFromFullGraph(_closureRequests);
_service = new TemplateService(_repoMock.Object, _auditMock.Object); _service = new TemplateService(_repoMock.Object, _auditMock.Object);
} }
@@ -2319,4 +2321,88 @@ public class TemplateServiceTests
Assert.True(propagated.IsInherited); Assert.True(propagated.IsInherited);
Assert.True(propagated.IsMesReportable); Assert.True(propagated.IsMesReportable);
} }
// ========================================================================
// Gitea #66: member add/update loads only the analysis closure
// ========================================================================
private void WholeAnalysisGraphMustNotLoad() =>
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
.ThrowsAsync(new InvalidOperationException("member add/update loaded the whole analysis graph"));
/// <summary>
/// Member adds validate collisions against the template's own closure (here: itself plus
/// its composed module) and never touch the whole-library analysis graph.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task MemberAdds_CheckCollisionsAgainstClosure_NeverLoadWholeGraph()
{
var module = new Template("Motor") { Id = 2 };
module.Attributes.Add(new TemplateAttribute("Temp") { Id = 20, TemplateId = 2 });
var template = new Template("Pump") { Id = 1 };
template.Compositions.Add(new TemplateComposition("m") { Id = 5, TemplateId = 1, ComposedTemplateId = 2 });
_repoMock.Setup(r => r.GetTemplateByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(template);
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
.ReturnsAsync(new List<Template> { template, module });
_repoMock.Setup(r => r.GetTemplateAnalysisClosureAsync(1, It.IsAny<CancellationToken>()))
.ReturnsAsync(new List<Template> { template, module });
WholeAnalysisGraphMustNotLoad();
// A canonical-name collision with the composed module is still caught from the closure.
var colliding = await _service.AddAttributeAsync(
1, new TemplateAttribute("m.Temp") { DataType = DataType.Float, Value = "0" }, "admin");
Assert.True(colliding.IsFailure);
Assert.Contains("Naming collision on 'm.Temp'", colliding.Error);
Assert.True((await _service.AddAttributeAsync(
1, new TemplateAttribute("Speed") { DataType = DataType.Float, Value = "0" }, "admin")).IsSuccess);
Assert.True((await _service.AddAlarmAsync(
1, new TemplateAlarm("Hi") { PriorityLevel = 500, TriggerType = AlarmTriggerType.ValueMatch }, "admin")).IsSuccess);
Assert.True((await _service.AddScriptAsync(
1, new TemplateScript("OnStart", "return true;") { TriggerType = "Startup" }, "admin")).IsSuccess);
_repoMock.Verify(r => r.GetTemplateAnalysisClosureAsync(1, It.IsAny<CancellationToken>()), Times.Exactly(4));
_repoMock.Verify(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()), Times.Never);
}
/// <summary>
/// Member updates resolve the parent lock from the PARENT's closure: a member locked on the
/// grandparent is found through the chain without loading the whole graph.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task MemberUpdates_ResolveParentLockFromParentClosure_NeverLoadWholeGraph()
{
var grand = new Template("Grand") { Id = 1 };
grand.Attributes.Add(new TemplateAttribute("Speed") { Id = 10, TemplateId = 1, DataType = DataType.Float, IsLocked = true });
grand.Alarms.Add(new TemplateAlarm("Hi") { Id = 11, TemplateId = 1, IsLocked = true, TriggerType = AlarmTriggerType.ValueMatch });
grand.Scripts.Add(new TemplateScript("Calc", string.Empty) { Id = 12, TemplateId = 1, IsLocked = true });
var parent = new Template("Parent") { Id = 2, ParentTemplateId = 1 };
var child = new Template("Child") { Id = 3, ParentTemplateId = 2 };
var childAttr = new TemplateAttribute("Speed") { Id = 30, TemplateId = 3, DataType = DataType.Float };
var childAlarm = new TemplateAlarm("Hi") { Id = 31, TemplateId = 3, TriggerType = AlarmTriggerType.ValueMatch };
var childScript = new TemplateScript("Calc", "return 1;") { Id = 32, TemplateId = 3 };
_repoMock.Setup(r => r.GetTemplateAttributeByIdAsync(30, It.IsAny<CancellationToken>())).ReturnsAsync(childAttr);
_repoMock.Setup(r => r.GetTemplateAlarmByIdAsync(31, It.IsAny<CancellationToken>())).ReturnsAsync(childAlarm);
_repoMock.Setup(r => r.GetTemplateScriptByIdAsync(32, It.IsAny<CancellationToken>())).ReturnsAsync(childScript);
_repoMock.Setup(r => r.GetTemplateByIdAsync(3, It.IsAny<CancellationToken>())).ReturnsAsync(child);
_repoMock.Setup(r => r.GetTemplateAnalysisClosureAsync(2, It.IsAny<CancellationToken>()))
.ReturnsAsync(new List<Template> { parent, grand });
WholeAnalysisGraphMustNotLoad();
var attr = await _service.UpdateAttributeAsync(
30, new TemplateAttribute("Speed") { DataType = DataType.Float, Value = "1" }, "admin");
var alarm = await _service.UpdateAlarmAsync(
31, new TemplateAlarm("Hi") { TriggerType = AlarmTriggerType.ValueMatch }, "admin");
var script = await _service.UpdateScriptAsync(32, new TemplateScript("Calc", "return 2;"), "admin");
Assert.Contains("locked in parent", attr.Error);
Assert.Contains("locked in parent", alarm.Error);
Assert.Contains("locked in parent", script.Error);
_repoMock.Verify(r => r.GetTemplateAnalysisClosureAsync(2, It.IsAny<CancellationToken>()), Times.Exactly(3));
_repoMock.Verify(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()), Times.Never);
}
} }