dohertj2/ScadaBridge
1
0
Fork 0
Code Issues 2 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(transport): export reads instance children once, site connections once per site, composed templates in batches (review 2026-10-21 04/P5)

Browse Source
This commit is contained in:
Joseph Doherty
2026-10-22 09:24:31 -04:00
parent 56aa386784
commit 04ac4c0491
3 changed files with 214 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/requirements/Component-Transport.md
+2
View File
@@ -159,6 +159,8 @@ A **Sites & Instances** section (T18) adds a flat list of sites, each row expand
The user can toggle "include all dependencies" off (with a warning that the bundle may produce an invalid import).
**Resolver round trips (review 2026-10-21 04/P5).** Instances are read with their override/binding collections in the same query (`GetInstanceByIdAsync` / the template repository's `GetInstancesBySiteIdAsync` both `Include` them — there is no per-collection reload); a site's data connections are read once per export and shared between the site seed and the per-instance closure; and missing templates (instance templates, then each level of the composition closure) are fetched in one batch through `GetTemplatesWithChildrenAsync` keyed by an id→name map from `GetTemplateStubsAsync` — a lone missing template still uses the by-id accessor. Exporting one 500-instance site is a few dozen queries rather than ~5 000.
**Step 3 — Encryption.** Passphrase and confirm. Strength meter. Explicit warning lists every secret field being encrypted. An "Export without encryption" path is logged with the `UnencryptedBundleExport` audit flag.
**Step 4 — Download.** Generated filename pattern: `scadabundle-{sourceEnv}-{yyyy-MM-dd-HHmmss}.scadabundle`. SHA-256 displayed for out-of-band verification.
src/ZB.MOM.WW.ScadaBridge.Transport/Export/DependencyResolver.cs
+98 -47
View File
@@ -136,6 +136,10 @@ public sealed class DependencyResolver
var dataConnections = new Dictionary<int, DataConnection>();
var instances = new Dictionary<int, Instance>();
// Review 2026-10-21 04/P5: one GetDataConnectionsBySiteIdAsync per site per
// export — the site seed and the per-instance closure below share this cache.
var siteConnectionCache = new Dictionary<int, IReadOnlyList<DataConnection>>();
// Selecting a SITE pulls its data connections and all of its instances. The
// instance's overrides/bindings are loaded eagerly so the serializer can read
// them off the entity navigation collections.
@@ -145,16 +149,17 @@ public sealed class DependencyResolver
if (site is null) continue;
sites[site.Id] = site;
foreach (var conn in await _siteRepository.GetDataConnectionsBySiteIdAsync(site.Id, ct).ConfigureAwait(false))
foreach (var conn in await GetSiteConnectionsAsync(site.Id, siteConnectionCache, ct).ConfigureAwait(false))
{
dataConnections[conn.Id] = conn;
}
foreach (var siteInstance in await _siteRepository.GetInstancesBySiteIdAsync(site.Id, ct).ConfigureAwait(false))
// The template repository's GetInstancesBySiteIdAsync Includes the four
// override/binding collections (the site repository's does not), so the
// site-loaded entity is used as-is — no per-instance reload (04/P5).
foreach (var siteInstance in await _templates.GetInstancesBySiteIdAsync(site.Id, ct).ConfigureAwait(false))
{
if (instances.ContainsKey(siteInstance.Id)) continue;
var loaded = await LoadInstanceWithChildrenAsync(siteInstance.Id, ct).ConfigureAwait(false);
if (loaded is not null) instances[loaded.Id] = loaded;
instances.TryAdd(siteInstance.Id, siteInstance);
}
}
@@ -164,7 +169,9 @@ public sealed class DependencyResolver
foreach (var instanceId in selection.InstanceIds.Distinct())
{
if (instances.ContainsKey(instanceId)) continue;
var loaded = await LoadInstanceWithChildrenAsync(instanceId, ct).ConfigureAwait(false);
// GetInstanceByIdAsync Includes the four override/binding collections, so
// no per-collection reload is needed (04/P5).
var loaded = await _templates.GetInstanceByIdAsync(instanceId, ct).ConfigureAwait(false);
if (loaded is not null) instances[loaded.Id] = loaded;
}
@@ -175,9 +182,11 @@ public sealed class DependencyResolver
// binds, and (by feeding instance.TemplateId into the template dictionary)
// its template — so the template/shared-script/external-system closure below
// expands transitively over instance templates too.
await ExpandSiteInstanceClosureAsync(instances, sites, dataConnections, templates, ct).ConfigureAwait(false);
var templateIdByName = new TemplateNameLookup(_templates);
await ExpandSiteInstanceClosureAsync(
instances, sites, dataConnections, templates, siteConnectionCache, templateIdByName, ct).ConfigureAwait(false);
await ExpandTemplateClosureAsync(templates, ct).ConfigureAwait(false);
await ExpandTemplateClosureAsync(templates, templateIdByName, ct).ConfigureAwait(false);
await ExpandSharedScriptsFromTemplatesAsync(templates, sharedScripts, ct).ConfigureAwait(false);
await ExpandSharedScriptsFromApiMethodsAsync(apiMethods, sharedScripts, ct).ConfigureAwait(false);
await ExpandExternalSystemsFromTemplatesAsync(templates, externalSystems, ct).ConfigureAwait(false);
@@ -272,25 +281,71 @@ public sealed class DependencyResolver
};
}
// ---- Instance child loading ----
// Loads an instance and attaches its overrides/bindings onto the entity's own
// navigation collections, so the serializer reads children off the entity (the
// same shape central-config entities use). Returns null if the instance is gone.
private async Task<Instance?> LoadInstanceWithChildrenAsync(int instanceId, CancellationToken ct)
// ---- Batched loads (review 2026-10-21 04/P5) ----
// Reads a site's data connections at most once per ResolveAsync call.
private async Task<IReadOnlyList<DataConnection>> GetSiteConnectionsAsync(
int siteId, Dictionary<int, IReadOnlyList<DataConnection>> cache, CancellationToken ct)
{
var instance = await _templates.GetInstanceByIdAsync(instanceId, ct).ConfigureAwait(false);
if (instance is null) return null;
if (!cache.TryGetValue(siteId, out var connections))
{
connections = await _siteRepository.GetDataConnectionsBySiteIdAsync(siteId, ct).ConfigureAwait(false);
cache[siteId] = connections;
}
return connections;
}
var attributeOverrides = await _templates.GetOverridesByInstanceIdAsync(instance.Id, ct).ConfigureAwait(false);
var alarmOverrides = await _templates.GetAlarmOverridesByInstanceIdAsync(instance.Id, ct).ConfigureAwait(false);
var nativeAlarmOverrides = await _templates.GetNativeAlarmSourceOverridesByInstanceIdAsync(instance.Id, ct).ConfigureAwait(false);
var bindings = await _templates.GetBindingsByInstanceIdAsync(instance.Id, ct).ConfigureAwait(false);
// Lazily-loaded id → name map over the whole template library (a body-free
// projection), so a set of template ids can be fetched with children through the
// by-name bulk accessor in one round trip instead of one by-id load each.
private sealed class TemplateNameLookup
{
private readonly ITemplateEngineRepository _repo;
private Dictionary<int, string>? _nameById;
instance.AttributeOverrides = attributeOverrides.ToList();
instance.AlarmOverrides = alarmOverrides.ToList();
instance.NativeAlarmSourceOverrides = nativeAlarmOverrides.ToList();
instance.ConnectionBindings = bindings.ToList();
return instance;
public TemplateNameLookup(ITemplateEngineRepository repo) => _repo = repo;
public async Task<IReadOnlyDictionary<int, string>> GetAsync(CancellationToken ct)
{
if (_nameById is null)
{
var stubs = await _repo.GetTemplateStubsAsync(ct).ConfigureAwait(false);
_nameById = stubs.ToDictionary(t => t.Id, t => t.Name);
}
return _nameById;
}
}
// Adds every template in `ids` not yet in `templates`, with children. A single
// missing id keeps the by-id accessor (no library projection needed); two or more
// go through one batched by-name load. Returns the newly added templates.
private async Task<List<Template>> LoadMissingTemplatesAsync(
IEnumerable<int> ids,
Dictionary<int, Template> templates,
TemplateNameLookup lookup,
CancellationToken ct)
{
var missing = ids.Where(id => !templates.ContainsKey(id)).Distinct().ToList();
var added = new List<Template>();
if (missing.Count == 0) return added;
if (missing.Count == 1)
{
var single = await _templates.GetTemplateWithChildrenAsync(missing[0], ct).ConfigureAwait(false);
if (single is not null && templates.TryAdd(single.Id, single)) added.Add(single);
return added;
}
var nameById = await lookup.GetAsync(ct).ConfigureAwait(false);
var names = missing.Where(nameById.ContainsKey).Select(id => nameById[id]).ToList();
if (names.Count == 0) return added;
var missingSet = missing.ToHashSet();
foreach (var t in await _templates.GetTemplatesWithChildrenAsync(names, ct).ConfigureAwait(false))
{
if (missingSet.Contains(t.Id) && templates.TryAdd(t.Id, t)) added.Add(t);
}
return added;
}
// ---- Site/instance closure ----
@@ -304,8 +359,15 @@ public sealed class DependencyResolver
Dictionary<int, Site> sites,
Dictionary<int, DataConnection> dataConnections,
Dictionary<int, Template> templates,
Dictionary<int, IReadOnlyList<DataConnection>> siteConnectionCache,
TemplateNameLookup templateLookup,
CancellationToken ct)
{
// 2. Templates → the existing template closure picks them up below. Loaded
// in one batch across every gathered instance (04/P5).
await LoadMissingTemplatesAsync(
instances.Values.Select(i => i.TemplateId), templates, templateLookup, ct).ConfigureAwait(false);
// Snapshot the instances: the loop only grows sites/dataConnections/templates,
// never instances, so iterating a snapshot keeps the dictionary stable.
foreach (var instance in instances.Values.ToList())
@@ -317,13 +379,6 @@ public sealed class DependencyResolver
if (site is not null) sites[site.Id] = site;
}
// 2. Template → the existing template closure picks it up below.
if (!templates.ContainsKey(instance.TemplateId))
{
var template = await _templates.GetTemplateWithChildrenAsync(instance.TemplateId, ct).ConfigureAwait(false);
if (template is not null) templates[template.Id] = template;
}
// 3. Data connections the instance references, resolved within its site.
// Connection bindings reference connections by id; native-alarm-source
// overrides reference them by name — resolve both against the site set.
@@ -341,8 +396,7 @@ public sealed class DependencyResolver
if (bindingConnectionIds.Count == 0 && referencedConnectionNames.Count == 0) continue;
var siteConnections = await _siteRepository
.GetDataConnectionsBySiteIdAsync(instance.SiteId, ct)
var siteConnections = await GetSiteConnectionsAsync(instance.SiteId, siteConnectionCache, ct)
.ConfigureAwait(false);
foreach (var conn in siteConnections)
@@ -360,22 +414,19 @@ public sealed class DependencyResolver
siteIdentifierById.TryGetValue(siteId, out var id) ? id : siteId.ToString();
// ---- Template composition closure ----
private async Task ExpandTemplateClosureAsync(Dictionary<int, Template> templates, CancellationToken ct)
// Breadth-first by level: every module composed by the current frontier is
// loaded in one batch (04/P5), then becomes the next frontier.
private async Task ExpandTemplateClosureAsync(
Dictionary<int, Template> templates, TemplateNameLookup templateLookup, CancellationToken ct)
{
var queue = new Queue<Template>(templates.Values);
while (queue.Count > 0)
IReadOnlyCollection<Template> frontier = templates.Values.ToList();
while (frontier.Count > 0)
{
var current = queue.Dequeue();
foreach (var comp in current.Compositions)
{
if (templates.ContainsKey(comp.ComposedTemplateId)) continue;
var composed = await _templates
.GetTemplateWithChildrenAsync(comp.ComposedTemplateId, ct)
.ConfigureAwait(false);
if (composed is null) continue;
templates[composed.Id] = composed;
queue.Enqueue(composed);
}
frontier = await LoadMissingTemplatesAsync(
frontier.SelectMany(t => t.Compositions).Select(c => c.ComposedTemplateId),
templates,
templateLookup,
ct).ConfigureAwait(false);
}
}
tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Export/DependencyResolverTests.cs
+114 -3
View File
@@ -7,6 +7,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Entities.Scripts;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Sites;
using ZB.MOM.WW.ScadaBridge.Commons.Entities.Templates;
using ZB.MOM.WW.ScadaBridge.Commons.Interfaces.Repositories;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Templates;
using ZB.MOM.WW.ScadaBridge.Commons.Types.Transport;
using ZB.MOM.WW.ScadaBridge.Transport.Export;
@@ -76,9 +77,28 @@ public sealed class DependencyResolverTests
IncludeDependencies: true,
SmsConfigurationIds: ids);
// Templates registered via StubTemplate. Backs both the by-id accessor and the
// batched stub + by-name accessors the resolver uses for closure expansion
// (review 2026-10-21 04/P5).
private readonly List<Template> _stubbedTemplates = new();
private void StubTemplate(Template t)
{
_templates.GetTemplateWithChildrenAsync(t.Id, Arg.Any<CancellationToken>()).Returns(t);
if (_stubbedTemplates.Count == 0)
{
_templates.GetTemplateStubsAsync(Arg.Any<CancellationToken>()).Returns(_ =>
(IReadOnlyList<TemplateStub>)_stubbedTemplates
.Select(x => new TemplateStub(x.Id, x.Name, x.ParentTemplateId, x.FolderId, x.IsDerived, x.OwnerCompositionId))
.ToList());
_templates.GetTemplatesWithChildrenAsync(Arg.Any<IEnumerable<string>>(), Arg.Any<CancellationToken>())
.Returns(ci =>
{
var names = ci.Arg<IEnumerable<string>>().ToHashSet(StringComparer.Ordinal);
return (IReadOnlyList<Template>)_stubbedTemplates.Where(x => names.Contains(x.Name)).ToList();
});
}
_stubbedTemplates.Add(t);
}
private void StubAllSharedScripts(params SharedScript[] scripts)
@@ -114,7 +134,7 @@ public sealed class DependencyResolverTests
private void StubSiteInstances(int siteId, params Instance[] instances)
{
_sites.GetInstancesBySiteIdAsync(siteId, Arg.Any<CancellationToken>()).Returns(instances);
_templates.GetInstancesBySiteIdAsync(siteId, Arg.Any<CancellationToken>()).Returns(instances);
}
// Stubs the per-instance child getters on the template repo so the resolver can
@@ -442,8 +462,9 @@ public sealed class DependencyResolverTests
Assert.Single(result.Instances, i => i.UniqueName == "Mixer01");
Assert.Single(result.Sites, s => s.SiteIdentifier == "site-e");
// The instance was loaded exactly once (site path), not re-loaded via the instance path.
await _templates.Received(1).GetInstanceByIdAsync(instance.Id, Arg.Any<CancellationToken>());
// The instance came from the site load (which already includes its children)
// and was not re-loaded via the instance path (review 2026-10-21 04/P5).
await _templates.DidNotReceive().GetInstanceByIdAsync(instance.Id, Arg.Any<CancellationToken>());
}
/// <summary>Verifies that resolving orders sites, connections and instances deterministically.</summary>
@@ -524,4 +545,94 @@ public sealed class DependencyResolverTests
Assert.Contains("Site:site-n", instEntry.DependsOn);
Assert.Contains("DataConnection:site-n/opc-1", instEntry.DependsOn);
}
// ─────────────────────────────────────────────────────────────────────
// Review 2026-10-21 04/P5 — export round trips.
// ─────────────────────────────────────────────────────────────────────
/// <summary>
/// Verifies that a directly-selected instance is loaded once, with its children
/// from the same query, and the four per-collection child loads are not issued.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Resolve_instance_does_not_reload_its_child_collections()
{
var site = new Site("Plant", "site-p") { Id = 5 };
var instance = new Instance("Pump01") { Id = 70, SiteId = site.Id, TemplateId = 0 };
StubSite(site);
StubInstance(instance);
StubAllSharedScripts();
StubAllExternalSystems();
StubAllFolders();
var result = await Sut().ResolveAsync(SelectInstances(new[] { instance.Id }), CancellationToken.None);
Assert.Single(result.Instances);
await _templates.Received(1).GetInstanceByIdAsync(instance.Id, Arg.Any<CancellationToken>());
await _templates.DidNotReceive().GetOverridesByInstanceIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>());
await _templates.DidNotReceive().GetAlarmOverridesByInstanceIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>());
await _templates.DidNotReceive().GetNativeAlarmSourceOverridesByInstanceIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>());
await _templates.DidNotReceive().GetBindingsByInstanceIdAsync(Arg.Any<int>(), Arg.Any<CancellationToken>());
}
/// <summary>
/// Verifies that a site's data connections are read once per export, not once per
/// instance that binds one.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Resolve_site_reads_its_data_connections_once()
{
var site = new Site("Plant", "site-p") { Id = 6 };
var conn = new DataConnection("opc", "OpcUa", site.Id) { Id = 61 };
var instances = Enumerable.Range(0, 3).Select(i =>
{
var inst = new Instance($"Pump{i}") { Id = 600 + i, SiteId = site.Id, TemplateId = 0 };
inst.ConnectionBindings.Add(new InstanceConnectionBinding("flow") { InstanceId = inst.Id, DataConnectionId = conn.Id });
return inst;
}).ToArray();
StubSite(site);
StubSiteConnections(site.Id, conn);
StubSiteInstances(site.Id, instances);
StubAllSharedScripts();
StubAllExternalSystems();
StubAllFolders();
var result = await Sut().ResolveAsync(SelectSites(new[] { site.Id }), CancellationToken.None);
Assert.Equal(3, result.Instances.Count);
Assert.Single(result.DataConnections);
await _sites.Received(1).GetDataConnectionsBySiteIdAsync(site.Id, Arg.Any<CancellationToken>());
}
/// <summary>
/// Verifies that the modules a template composes are loaded in one batch rather
/// than one by-id load each.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[Fact]
public async Task Resolve_batches_composed_template_loads()
{
var root = new Template("Root") { Id = 1 };
var modules = Enumerable.Range(0, 3).Select(i => new Template($"Module{i}") { Id = 10 + i }).ToArray();
foreach (var m in modules)
{
root.Compositions.Add(new TemplateComposition($"slot{m.Id}") { TemplateId = root.Id, ComposedTemplateId = m.Id });
}
StubTemplate(root);
foreach (var m in modules) StubTemplate(m);
StubAllSharedScripts();
StubAllExternalSystems();
StubAllFolders();
var result = await Sut().ResolveAsync(SelectTemplates(root.Id), CancellationToken.None);
Assert.Equal(4, result.Templates.Count);
foreach (var m in modules)
{
await _templates.DidNotReceive().GetTemplateWithChildrenAsync(m.Id, Arg.Any<CancellationToken>());
}
await _templates.Received(1).GetTemplatesWithChildrenAsync(Arg.Any<IEnumerable<string>>(), Arg.Any<CancellationToken>());
}
}