perf(templateengine): alarm-override, folder-delete and template-delete checks read lean template projections (review 2026-10-21 04/P6, 08/P1)
InstanceService.SetAlarmOverrideAsync and TemplateDeletionService move to the no-tracking, body-free GetAllTemplatesForAnalysisAsync; TemplateFolderService counts folder contents from GetTemplateStubsAsync.
This commit is contained in:
@@ -211,7 +211,15 @@ The design-time checks — acyclicity (`CycleDetector`), naming collisions
|
||||
(`CollisionDetector`) and canonical-name resolution (`TemplateResolver`) — read the
|
||||
whole template graph but write through none of it and read no script bodies. They
|
||||
use `GetAllTemplatesForAnalysisAsync`: `AsNoTracking`, with `TemplateScript.Code`
|
||||
projected away.
|
||||
projected away. The same projection serves the non-analysis lookups that need only
|
||||
names, locks and edges: `InstanceService.SetAlarmOverrideAsync` (resolving one alarm
|
||||
name) and `TemplateDeletionService.CanDeleteTemplateAsync` (inheritor / derived /
|
||||
composer checks). `TemplateFolderService.DeleteFolderAsync`, which only counts
|
||||
templates per `FolderId`, reads the lighter `GetTemplateStubsAsync` identity projection
|
||||
(review 2026-10-21 04/P6, 08/P1). Template member add/update paths still load the whole
|
||||
analysis projection for collision detection; narrowing that to the edited template's
|
||||
chain + composition closure needs a closure-scoped repository read that does not exist
|
||||
yet.
|
||||
|
||||
The tracked, body-bearing `GetAllTemplatesAsync` remains the read for the two paths
|
||||
that genuinely need it — the inheritance reconciler (which compares and copies
|
||||
|
||||
@@ -243,7 +243,9 @@ public class InstanceService
|
||||
// against the template's direct alarms alone would miss them, silently
|
||||
// accepting an override for a non-existent name or bypassing the lock
|
||||
// rule for a composed alarm. Mirrors SetAttributeOverrideAsync.
|
||||
var allTemplates = await _repository.GetAllTemplatesAsync(cancellationToken);
|
||||
// The resolver reads member names and lock flags only, so the no-tracking,
|
||||
// script-body-free analysis projection suffices (review 2026-10-21 04/P6, 08/P1).
|
||||
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken);
|
||||
var resolvedAlarm = TemplateResolver
|
||||
.ResolveAllMembers(instance.TemplateId, allTemplates)
|
||||
.FirstOrDefault(m => m.MemberType == "Alarm" && m.CanonicalName == alarmCanonicalName);
|
||||
|
||||
@@ -53,7 +53,10 @@ public class TemplateDeletionService
|
||||
}
|
||||
|
||||
// Check 2: Child templates reference it as parent. Split derived vs. regular.
|
||||
var allTemplates = await _repository.GetAllTemplatesAsync(cancellationToken);
|
||||
// Reads ids, names, parent/derived/owner edges and composition rows only — the
|
||||
// no-tracking, script-body-free analysis projection carries all of them
|
||||
// (review 2026-10-21 04/P6, 08/P1).
|
||||
var allTemplates = await _repository.GetAllTemplatesForAnalysisAsync(cancellationToken);
|
||||
var inheritors = allTemplates.Where(t => t.ParentTemplateId == templateId).ToList();
|
||||
var regularChildren = inheritors.Where(t => !t.IsDerived).ToList();
|
||||
var derivatives = inheritors.Where(t => t.IsDerived).ToList();
|
||||
@@ -79,7 +82,7 @@ public class TemplateDeletionService
|
||||
}
|
||||
|
||||
// Check 3: Other templates compose it directly.
|
||||
// Read the Compositions navigation already loaded by GetAllTemplatesAsync
|
||||
// Read the Compositions navigation already loaded by GetAllTemplatesForAnalysisAsync
|
||||
// rather than issuing one GetCompositionsByTemplateIdAsync round-trip per
|
||||
// template — this is the same source TemplateService
|
||||
// .DeleteTemplateAsync uses for the equivalent check.
|
||||
|
||||
@@ -232,7 +232,9 @@ public class TemplateFolderService
|
||||
return Result<bool>.Failure($"Folder with ID {folderId} not found.");
|
||||
|
||||
var allFolders = await _repository.GetAllFoldersAsync(cancellationToken);
|
||||
var allTemplates = await _repository.GetAllTemplatesAsync(cancellationToken);
|
||||
// Only FolderId is read — the untracked identity projection, not the full
|
||||
// template graph with every script body (review 2026-10-21 04/P6, 08/P1).
|
||||
var allTemplates = await _repository.GetTemplateStubsAsync(cancellationToken);
|
||||
|
||||
var childFolderCount = allFolders.Count(f => f.ParentFolderId == folderId);
|
||||
var childTemplateCount = allTemplates.Count(t => t.FolderId == folderId);
|
||||
|
||||
@@ -154,7 +154,7 @@ public class InstanceServiceTests
|
||||
var instance = new Instance("Inst1") { Id = 1, TemplateId = 1 };
|
||||
_repoMock.Setup(r => r.GetInstanceByIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(instance);
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
TemplateWithAlarms(1, new TemplateAlarm("HighTemp") { Id = 10 })
|
||||
@@ -166,6 +166,9 @@ public class InstanceServiceTests
|
||||
Assert.Contains("does not exist", result.Error);
|
||||
_repoMock.Verify(r => r.AddInstanceAlarmOverrideAsync(
|
||||
It.IsAny<InstanceAlarmOverride>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
// Review 2026-10-21 04/P6 + 08/P1: resolving one alarm name needs names and lock
|
||||
// flags only — never the tracked, script-bodies-included full template graph.
|
||||
_repoMock.Verify(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that overriding a locked alarm on a composed template returns a failure.</summary>
|
||||
@@ -183,7 +186,7 @@ public class InstanceServiceTests
|
||||
var host = new Template("Host") { Id = 1 };
|
||||
host.Compositions.Add(new TemplateComposition("Pump") { Id = 1, ComposedTemplateId = 2 });
|
||||
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { host, module });
|
||||
|
||||
var result = await _sut.SetAlarmOverrideAsync(1, "Pump.Fault", "{}", null, "admin");
|
||||
@@ -207,7 +210,7 @@ public class InstanceServiceTests
|
||||
var host = new Template("Host") { Id = 1 };
|
||||
host.Compositions.Add(new TemplateComposition("Pump") { Id = 1, ComposedTemplateId = 2 });
|
||||
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { host, module });
|
||||
_repoMock.Setup(r => r.GetAlarmOverrideAsync(1, "Pump.Fault", It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync((InstanceAlarmOverride?)null);
|
||||
@@ -229,7 +232,7 @@ public class InstanceServiceTests
|
||||
var instance = new Instance("Inst1") { Id = 1, TemplateId = 1 };
|
||||
_repoMock.Setup(r => r.GetInstanceByIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(instance);
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
TemplateWithAlarms(1, new TemplateAlarm("HighTemp") { Id = 10, IsLocked = true })
|
||||
@@ -249,7 +252,7 @@ public class InstanceServiceTests
|
||||
var instance = new Instance("Inst1") { Id = 1, TemplateId = 1 };
|
||||
_repoMock.Setup(r => r.GetInstanceByIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(instance);
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
TemplateWithAlarms(1, new TemplateAlarm("HighTemp") { Id = 10, IsLocked = false })
|
||||
@@ -281,7 +284,7 @@ public class InstanceServiceTests
|
||||
var instance = new Instance("Inst1") { Id = 1, TemplateId = 1 };
|
||||
_repoMock.Setup(r => r.GetInstanceByIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(instance);
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
TemplateWithAlarms(1, new TemplateAlarm("HighTemp") { Id = 10, IsLocked = false })
|
||||
@@ -317,7 +320,7 @@ public class InstanceServiceTests
|
||||
var instance = new Instance("Inst1") { Id = 1, TemplateId = 1 };
|
||||
_repoMock.Setup(r => r.GetInstanceByIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(instance);
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
TemplateWithAlarms(1, new TemplateAlarm("HighTemp") { Id = 10, IsLocked = false })
|
||||
|
||||
+11
-8
@@ -26,7 +26,7 @@ public class TemplateDeletionServiceTests
|
||||
.ReturnsAsync(new Template("Orphan") { Id = 1 });
|
||||
_repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Instance>());
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { new("Orphan") { Id = 1 } });
|
||||
_repoMock.Setup(r => r.GetCompositionsByTemplateIdAsync(It.IsAny<int>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateComposition>());
|
||||
@@ -34,6 +34,9 @@ public class TemplateDeletionServiceTests
|
||||
var result = await _sut.CanDeleteTemplateAsync(1);
|
||||
|
||||
Assert.True(result.IsSuccess);
|
||||
// Review 2026-10-21 04/P6 + 08/P1: the reference checks read ids, names and
|
||||
// composition edges only — the no-tracking, body-free analysis projection.
|
||||
_repoMock.Verify(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that a template with instances cannot be deleted.</summary>
|
||||
@@ -49,7 +52,7 @@ public class TemplateDeletionServiceTests
|
||||
new("Inst1") { Id = 1 },
|
||||
new("Inst2") { Id = 2 }
|
||||
});
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { new("Used") { Id = 1 } });
|
||||
_repoMock.Setup(r => r.GetCompositionsByTemplateIdAsync(It.IsAny<int>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateComposition>());
|
||||
@@ -70,7 +73,7 @@ public class TemplateDeletionServiceTests
|
||||
.ReturnsAsync(new Template("Base") { Id = 1 });
|
||||
_repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Instance>());
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
new("Base") { Id = 1 },
|
||||
@@ -98,7 +101,7 @@ public class TemplateDeletionServiceTests
|
||||
.ReturnsAsync(new Template("Module") { Id = 1 });
|
||||
_repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Instance>());
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
new("Module") { Id = 1 },
|
||||
@@ -118,7 +121,7 @@ public class TemplateDeletionServiceTests
|
||||
public async Task CanDeleteTemplate_DoesNotIssuePerTemplateCompositionQuery()
|
||||
{
|
||||
// The composition check must read the Compositions navigation
|
||||
// already loaded by GetAllTemplatesAsync rather than issuing one
|
||||
// already loaded by GetAllTemplatesForAnalysisAsync rather than issuing one
|
||||
// GetCompositionsByTemplateIdAsync round-trip per template.
|
||||
var templates = new List<Template>
|
||||
{
|
||||
@@ -132,7 +135,7 @@ public class TemplateDeletionServiceTests
|
||||
.ReturnsAsync(new Template("Module") { Id = 1 });
|
||||
_repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Instance>());
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(templates);
|
||||
|
||||
var result = await _sut.CanDeleteTemplateAsync(1);
|
||||
@@ -165,7 +168,7 @@ public class TemplateDeletionServiceTests
|
||||
.ReturnsAsync(new Template("Safe") { Id = 1 });
|
||||
_repoMock.Setup(r => r.GetInstancesByTemplateIdAsync(1, It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Instance>());
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { new("Safe") { Id = 1 } });
|
||||
_repoMock.Setup(r => r.GetCompositionsByTemplateIdAsync(It.IsAny<int>(), It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateComposition>());
|
||||
@@ -189,7 +192,7 @@ public class TemplateDeletionServiceTests
|
||||
.ReturnsAsync(new List<Instance> { new("Inst1") { Id = 1 } });
|
||||
var composer = new Template("Composer") { Id = 3 };
|
||||
composer.Compositions.Add(new TemplateComposition("Module") { ComposedTemplateId = 1 });
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
_repoMock.Setup(r => r.GetAllTemplatesForAnalysisAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>
|
||||
{
|
||||
new("Busy") { Id = 1 },
|
||||
|
||||
+13
-9
@@ -3,6 +3,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Services;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums;
|
||||
using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;
|
||||
using ZB.MOM.WW.ScadaBridge.TemplateEngine.Services;
|
||||
|
||||
namespace ZB.MOM.WW.ScadaBridge.TemplateEngine.Tests.Services;
|
||||
@@ -223,8 +224,8 @@ public class TemplateFolderServiceTests
|
||||
_repoMock.Setup(r => r.GetFolderByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(f);
|
||||
_repoMock.Setup(r => r.GetAllFoldersAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateFolder> { f });
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>());
|
||||
_repoMock.Setup(r => r.GetTemplateStubsAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateStub>());
|
||||
|
||||
var result = await _sut.DeleteFolderAsync(1, "admin");
|
||||
|
||||
@@ -242,8 +243,8 @@ public class TemplateFolderServiceTests
|
||||
_repoMock.Setup(r => r.GetFolderByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(parent);
|
||||
_repoMock.Setup(r => r.GetAllFoldersAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateFolder> { parent, child });
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>());
|
||||
_repoMock.Setup(r => r.GetTemplateStubsAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateStub>());
|
||||
|
||||
var result = await _sut.DeleteFolderAsync(1, "admin");
|
||||
|
||||
@@ -257,17 +258,20 @@ public class TemplateFolderServiceTests
|
||||
public async Task DeleteFolder_HasTemplates_ReturnsFailure_WithCounts()
|
||||
{
|
||||
var f = new TemplateFolder("P") { Id = 1 };
|
||||
var t = new Template("X") { Id = 5, FolderId = 1 };
|
||||
var t = new TemplateStub(5, "X", null, 1, false, null);
|
||||
_repoMock.Setup(r => r.GetFolderByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(f);
|
||||
_repoMock.Setup(r => r.GetAllFoldersAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateFolder> { f });
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template> { t });
|
||||
_repoMock.Setup(r => r.GetTemplateStubsAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateStub> { t });
|
||||
|
||||
var result = await _sut.DeleteFolderAsync(1, "admin");
|
||||
|
||||
Assert.True(result.IsFailure);
|
||||
Assert.Contains("1 template", result.Error);
|
||||
// Review 2026-10-21 04/P6 + 08/P1: a folder-content count needs only FolderId —
|
||||
// never the tracked, script-bodies-included full template graph.
|
||||
_repoMock.Verify(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
// ========================================================================
|
||||
@@ -629,8 +633,8 @@ public class TemplateFolderServiceTests
|
||||
_repoMock.Setup(r => r.GetFolderByIdAsync(1, It.IsAny<CancellationToken>())).ReturnsAsync(f);
|
||||
_repoMock.Setup(r => r.GetAllFoldersAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateFolder> { f });
|
||||
_repoMock.Setup(r => r.GetAllTemplatesAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<Template>());
|
||||
_repoMock.Setup(r => r.GetTemplateStubsAsync(It.IsAny<CancellationToken>()))
|
||||
.ReturnsAsync(new List<TemplateStub>());
|
||||
}));
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user